Matheus Alcantara писал(а) 2025-03-18 21:56:
On Fri, Mar 7, 2025 at 7:43 AM Vladlen Popolitov
<v.popoli...@postgrespro.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?
Yes, I understood. It is possible to make compilation of the newly
created expressions. I tried to avoid additional compilation and chose
the implementation without compilation.
> 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 agree, it requires cleaning of the code. I am going to do it on the
next
step.
> 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?
I am preparing the new version. I got comments with bugs and memory
leaks,
I am fixing and testing the code.
I run TPC-H benchmark and found the degradation of performance with
jit_cached (and with regular jit also in some queries). As I see,
parallel execution of the query creates a parallel worker, that starts,
gets the plan, compile all expressions again. Even if query has cached
jit code, all parallel workers compile code again.
By the way, it is possible to send to the worker compiled jit code
together with plan (shared memory is used), but again arise the problem,
how to find the link between expression addresses in parallel worker
address space and jit code in shared memory.
--
Best regards,
Vladlen Popolitov.
