Date: Fri, 16 Apr 2021 19:16:02 -0700
From: Andres Freund <andres@anarazel.de>
To: Jaime Casanova <jcasanov@systemguards.com.ec>
Cc: Kurt Roeckx <kurt@roeckx.be>, Tom Lane <tgl@sss.pgh.pa.us>,
	pgsql-bugs@lists.postgresql.org,
	Justin Pryzby <pryzby@telsasoft.com>
Subject: Re: BUG #16707: Memory leak
Message-ID: <20210417021602.7dilihkdc7oblrf7@alap3.anarazel.de>
References: <20201109220329.GC391173@roeckx.be>
 <2002012.1604960437@sss.pgh.pa.us>
 <20201110043127.sfkyvvjqy7x3er5k@alap3.anarazel.de>
 <20201110081120.GF391173@roeckx.be>
 <20201110193517.t5hbqkya67gb23rq@alap3.anarazel.de>
 <20201110195039.GG391173@roeckx.be>
 <20201110224516.GA568918@roeckx.be>
 <20201111013808.4cev55uxq32bd6dd@alap3.anarazel.de>
 <20210407162835.GB2857@ahch-to>
 <7acc8678-df5f-4923-9cf6-e843131ae89d@www.fastmail.com>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="um6u6gsnflasiacx"
Content-Disposition: inline
In-Reply-To: <7acc8678-df5f-4923-9cf6-e843131ae89d@www.fastmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/20210417021602.7dilihkdc7oblrf7%40alap3.anarazel.de>
Precedence: bulk


--um6u6gsnflasiacx
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

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

--um6u6gsnflasiacx
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v1-0001-wip-llvmjit-Use-explicit-LLVMContextRef.patch"