Re: PoC. The saving of the compiled jit-code in the plan cache - Mailing list pgsql-hackers

Hi,

I've spent some time learning more about jit in the last few weeks and I
think that this patch could be very useful, thanks for working on this!
I'm new on this subject but I would like to share some thoughts about
it.

> 1. Changes in jit-code generation.
>
> a) the load of the absolute address (as const) changed to the load of this
> address from a struct member:
>
If I understood correctly, this change is required to avoid making the
jit code points to a memory address that was reallocated (making it
invalid)? If that's the case, would be possible to split this change
into a separated patch? It would help a lot to review.

It's not clear to me what's the difference between jit_context being set
to NULL or CACHED_JITCONTEXT_EMPTY and how it changes from NULL to
CACHED_JITCONTEXT_EMPTY. I think some code comments will help on this,
specially on llvm_compile_expr around if/else blocks.

> 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.

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.

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.

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

-- 
Matheus Alcantara