Re: BUG #15592: Memory overuse with subquery containing unnest() and set operations (11.x regression)

Andres Freund <andres@anarazel.de>

From: Andres Freund <andres@anarazel.de>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-bugs@lists.postgresql.org, amdmi3@amdmi3.ru, Andrew Gierth <andrew@tao11.riddles.org.uk>, Jakub Janeček <jakub.janecek@comgate.cz>
Date: 2019-02-09T09:40:25Z
Lists: pgsql-bugs
Hi,

On 2019-01-14 14:28:38 -0800, Andres Freund wrote:
> Attached is a series of patches that fix this, and the issue in [1],
> namely that we do not reuse JITed expressions, leading to significant
> overhead when JIT compiling.
> 
> The first patch just moves the ExprContext into the tablecxt of the
> tuple hash table, and uses a standalone expression instead of linked
> into estate. I think that's OK for the the table's purpose, because the
> expression used is tightly restricted because it's built with
> ExecBuildGroupingEqual(). Previously we'd call fmgr functions directly,
> so there can't be any dependency on expression parents here (and it's
> not clear how that'd ever make any sense). Given that, I think it's ok
> to not explicitly shutdown the expr context.  If somebody disagrees, we
> could change this, but freeing individual ExprContexts is
> O(#exprcontexts), so I'd prefer not to unnecessarily go there.
> 
> This is sufficient to fix the memory leak problem (but is slower than
> 10, due to the overhead of creating the comparator expression
> repeatedly).
> 
> The remaining patches add 0002) support for resetting a simplehash
> hashtable, 0003) Add BuildTupleHashTableExt(), which allows to place the
> tuple hashtable into a separate memory context besides tablecxt, 0004)
> changes in-core tuple hash users to reset instead recreate hashtables.
> This has the advantage of being doable without breaking external users
> of the API, while still avoiding recreation of the comparator expression
> (and the slot, and the TupleHashTable itself), which in turn avoids the
> JIT overhead.
> 
> These patches, especially surrounding comments, need a bit of polish,
> but I thought it'd be good to discuss them before investing more time.

I've pushed these now.

Does anybody have an opinion about removing the BuildTupleHashTable /
BuildTupleHashTableExt split in master?

Dmitry, the fix will be included in the next minor release, which is
scheduled for next week.

Greetings,

Andres Freund


Commits

  1. Plug leak in BuildTupleHashTable by creating ExprContext in correct context.

  2. simplehash: Add support for resetting a hashtable's contents.