Re: PoC. The saving of the compiled jit-code in the plan cache

On Fri, Mar 7, 2025 at 7:43 AM Vladlen Popolitov
<v(dot)popolitov(at)postgrespro(dot)ru> wrote:
> >
> >> Updated patch rebased to the current master. Also I resolved the
> >> problems
> >> with the lookup of the compiled expressions.
> >> Cached jit compiles only expressions from cached plan - they are
> >> recognized by memory context - if Expr is not NULL and belong to the
> >> same
> >> memory context as cached plan, this expression is compiled to the
> >> function
> >> with expression address in the name (expression address casted to Size
> >> type).
> >> All other expression (generated later than plan, f.e. expressions in
> >> aggregates, hash join, hash aggregates) are not compiled and are
> >> executed
> >> by standard expression interpreter.
> >>
> > What's stopping us from going back to the current code generation
> > without caching on these scenarios? I'm a bit concerned for never jit
> > compile these kind of expressions and have some kind of performance
> > issue.
> >
> I thought about this. The reason to store the jit in cache - avoid
> compiling every time when query is executed. If we anyway compile
> the code even for 1 function, we lose benefits of cached jit code.
> We can choose the standard jit compilation for other functions.
>
I don't think that I get your point here - Why we would lose the
benefits? I think that I may not be clear with my question; My point
was, why we can't jit compile, without caching, these expressions that
are generated later on the plan? Like aggregates, hash join, as you
mention. I think that we may have scenarios that jit compile is worth
even if we will not cache the compiled code. I understand the limitation
of runtime generated ExprState being allocated in another memory
context, but why can't we follow the same path as we have today for
these scenarios?

> > What's the relation of this exec time generated expressions (hash join,
> > hash agg) problem with the function name lookup issue? It's seems
> > different problems to me but I may be missing something.
> This question and next question are correlated.
> > If these problems is not fully correlated I think that it would be
> > better to have some kind of hash map or use the number of the call to
> > connect expressions with functions (as you did on v4) to lookup the
> > cached compiled function. IMHO it sounds a bit strange to me to add the
> > function pointer on the function name and have this memory context
> > validation.
> This function lookup the biggest challenge in this patch, I see
> it correct.
> In current implementation: the function is generated for expression,
> the name of the function is saved in the ExprState,
> then function is compiled (when first expression is executed),
> then the function is called,
> lookup by saved address is done to find compiled code,
> the compiled code address is saved in ExprState as new execution code,
> the compiled code is called.
>
>
> In cached implementation when expression is called the next time,
> we need to find the link among the expression and the compiled code.
> I tried to find reliable way to connect them, and see now only one
> version - make compilation only for expressions saved with the plan -
> they have the same address across the time and the same memory context
> as the saved plan. Expressions generated during execution like
> aggregate expression and Hash join expression are allocated in
> query execution context and always have different addresses, and I did
> not find any way to connect them with cached plan. The have "expr"
> member, but it can be NULL, T_List or T_Expr (or anything, what
> Custom node can assign to it).
> Function name with expression address now looks as reliable way to find
> compiled function, though not elegant. Standard implementation just
> stores
> generated name, uses it and pfree() it.
>
Yeah, I see that this sounds a bit complicated. I'm wondering if this
logic of checking if the expr is already cached or not could be
implemented in another function or layer (maybe jit_compile_expr?). I
think that llvm_compile_expr is already complicated enough and it should
only care about emitting code, so we could have another layer that check
if the expr is already compiled or not and only call llvm_compile_expr
if it's not compiled yet, WYT?

> > I've also executed make check-world and it seems that
> > test_misc/003_check_guc is falling:
> >
> > [11:29:42.995](1.153s) not ok 1 - no parameters missing from
> > postgresql.conf.sample
> > [11:29:42.995](0.000s) # Failed test 'no parameters missing from
> > postgresql.conf.sample'
> > # at src/test/modules/test_misc/t/003_check_guc.pl line 85.
> > [11:29:42.995](0.000s) # got: '1'
> > # expected: '0'
> > [11:29:42.995](0.000s) ok 2 - no parameters missing from guc_tables.c
> > [11:29:42.995](0.000s) ok 3 - no parameters marked as NOT_IN_SAMPLE in
> > postgresql.conf.sample
> > found GUC jit_cached in guc_tables.c, missing from
> > postgresql.conf.sample
>
> I tested in with make check and make installcheck . In v8 I found bugs,
> but not published fix yet.
>
Do you have any intent to work on a new version for this?

--
Matheus Alcantara