Re: BUG #16707: Memory leak

Hi,

On 2021-04-07 09:31:54 -0700, Andres Freund wrote:
> On Wed, Apr 7, 2021, at 09:28, Jaime Casanova wrote:
> > I was bit by this too while testing something. I thought it could have been
> > a problem caused by the support of llvm 12? but given that this report
> > is for pg12, it seems this is older than that.
>
> I hope to fix it soon - I have two different mostly working
> fixes. I'll get back to it once the dust around the freeze settles.

Quick recap:

The issue is that inlining needs to re-read the modules with the code
that's being inlined, because LLVMs IR linker is destructive. Copying
the modules alone is not sufficient, because the IR linker modifies the
types, and IR types are cross-module entities (just the names are
modified, but that's enough to cause problems). Whenever a module is
read, it does *not* reuse existing struct types, but instead creates
them from scratch (type uniquing would be expensive, and names are not
sufficient identification).

I have evaluated two approaches to fixing the issue:

1) Write a new IR linker module for LLVM that is not destructive. The
pro arguments for that is that that improves inlining performance
substantially. But it's a fair bit of new code, which I think makes
it unsuitable for fixing the bug in the back branches.

2) Occasionally dispose of the current LLVMContext and create a new
one. The LLVMContext is where modules and types live, so disposing
the context releases all the accumulated types.

Unfortunately we can't just create a separate LLVMContext for every
query, as types etc cannot be reused across contexts, and reloading
all the information needed for JITing would be too expensive.

A second disadvantage of this approach is that it's somewhat
invasive, as a fair bit of code needs to be changed not to reference
the "global context" we were using so far. But that's fairly
mechanical, and not too hard to audit.

While I hope to continue to look into 1), it's clearly v15+
material. See the attached patches for a draft of how 2) would look
like.

The last of the attached patches is the main logic change, whereas the
first contains the changes to make all context references explicit. The
last patch also contains a number of TODOs in the commit message.

Greetings,

Andres Freund