Horton

Thread

Commits

  1. Move resowner from common JitContext to LLVM specific

  2. Make ResourceOwners more easily extensible.

  1. Should we move the resowner field from JitContext to LLVMJitContext?

    Andreas Karlsson <andreas@proxel.se> — 2024-06-05T08:19:01Z

    Hi,

I am implementing my own JIT plugin (based on Cranelift) for PostgreSQL 
to learn more about the JIT and noticed an API change in PostgreSQL 17.

When Heikki made the resource owners extensible in commit 
b8bff07daa85c837a2747b4d35cd5a27e73fb7b2 the API for JIT plugins changed 
when ResourceOwnerForgetJIT() was moved from the generic JIT code to the 
LLVM specific JIT code so now the resowner field of the context is only 
used by the code of the LLVM plugin.

Maybe a bit late in the release cycle but should we make the resowner 
field specific to the LLVM code too now that we already are breaking the 
API? I personally do not like having a LLVM JIT specific field in the 
common struct. Code is easier to understand if things are local. Granted 
most JIT engines will likely need similar infrastructure but just 
providing the struct field and nothing else does not seem very helpful.

See the attached patch.

Andreas

  2. Re: Should we move the resowner field from JitContext to LLVMJitContext?

    Daniel Gustafsson <daniel@yesql.se> — 2024-07-01T14:19:55Z

    > On 5 Jun 2024, at 10:19, Andreas Karlsson <andreas@proxel.se> wrote:

> When Heikki made the resource owners extensible in commit b8bff07daa85c837a2747b4d35cd5a27e73fb7b2 the API for JIT plugins changed when ResourceOwnerForgetJIT() was moved from the generic JIT code to the LLVM specific JIT code so now the resowner field of the context is only used by the code of the LLVM plugin.
> 
> Maybe a bit late in the release cycle but should we make the resowner field specific to the LLVM code too now that we already are breaking the API? I personally do not like having a LLVM JIT specific field in the common struct. Code is easier to understand if things are local. Granted most JIT engines will likely need similar infrastructure but just providing the struct field and nothing else does not seem very helpful.

I'm inclined to agree, given that the code for handling the resowner is private
to the LLVM implementation it makes sense for the resowner to be as well.  A
future JIT implementation will likely need a ResourceOwner, but it might just
as well need two or none.

--
Daniel Gustafsson

  3. Re: Should we move the resowner field from JitContext to LLVMJitContext?

    Heikki Linnakangas <hlinnaka@iki.fi> — 2024-07-19T07:46:51Z

    On 01/07/2024 17:19, Daniel Gustafsson wrote:
>> On 5 Jun 2024, at 10:19, Andreas Karlsson <andreas@proxel.se> wrote:
> 
>> When Heikki made the resource owners extensible in commit b8bff07daa85c837a2747b4d35cd5a27e73fb7b2 the API for JIT plugins changed when ResourceOwnerForgetJIT() was moved from the generic JIT code to the LLVM specific JIT code so now the resowner field of the context is only used by the code of the LLVM plugin.
>>
>> Maybe a bit late in the release cycle but should we make the resowner field specific to the LLVM code too now that we already are breaking the API? I personally do not like having a LLVM JIT specific field in the common struct. Code is easier to understand if things are local. Granted most JIT engines will likely need similar infrastructure but just providing the struct field and nothing else does not seem very helpful.
> 
> I'm inclined to agree, given that the code for handling the resowner is private
> to the LLVM implementation it makes sense for the resowner to be as well.  A
> future JIT implementation will likely need a ResourceOwner, but it might just
> as well need two or none.

Committed, thanks!

-- 
Heikki Linnakangas
Neon (https://neon.tech)