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
