Should we move the resowner field from JitContext to LLVMJitContext?

[Andreas Karlsson]

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.

AndreasFrom e3b55e00aee578a46298447463e0984aa3a230f7 Mon Sep 17 00:00:00 2001
From: Andreas Karlsson <andr...@proxel.se>
Date: Wed, 5 Jun 2024 10:13:23 +0200
Subject: [PATCH] Move resowner from common JitContext to LLVM specific

Only the LLVM specific code uses it since resource owners were made
extensible in commit b8bff07daa85c837a2747b4d35cd5a27e73fb7b2.
---
 src/backend/jit/llvm/llvmjit.c | 10 +++++-----
 src/include/jit/jit.h          |  2 --
 src/include/jit/llvmjit.h      |  3 +++
 3 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/src/backend/jit/llvm/llvmjit.c b/src/backend/jit/llvm/llvmjit.c
index 1d439f2455..abfe444c7e 100644
--- a/src/backend/jit/llvm/llvmjit.c
+++ b/src/backend/jit/llvm/llvmjit.c
@@ -249,7 +249,7 @@ llvm_create_context(int jitFlags)
 	context->base.flags = jitFlags;
 
 	/* ensure cleanup */
-	context->base.resowner = CurrentResourceOwner;
+	context->resowner = CurrentResourceOwner;
 	ResourceOwnerRememberJIT(CurrentResourceOwner, context);
 
 	llvm_jit_context_in_use_count++;
@@ -323,8 +323,8 @@ llvm_release_context(JitContext *context)
 
 	llvm_leave_fatal_on_oom();
 
-	if (context->resowner)
-		ResourceOwnerForgetJIT(context->resowner, llvm_jit_context);
+	if (llvm_jit_context->resowner)
+		ResourceOwnerForgetJIT(llvm_jit_context->resowner, llvm_jit_context);
 }
 
 /*
@@ -1372,8 +1372,8 @@ llvm_error_message(LLVMErrorRef error)
 static void
 ResOwnerReleaseJitContext(Datum res)
 {
-	JitContext *context = (JitContext *) DatumGetPointer(res);
+	LLVMJitContext *context = (LLVMJitContext *) DatumGetPointer(res);
 
 	context->resowner = NULL;
-	jit_release_context(context);
+	jit_release_context(&context->base);
 }
diff --git a/src/include/jit/jit.h b/src/include/jit/jit.h
index b7a1eed281..d9a080ce98 100644
--- a/src/include/jit/jit.h
+++ b/src/include/jit/jit.h
@@ -59,8 +59,6 @@ typedef struct JitContext
 	/* see PGJIT_* above */
 	int			flags;
 
-	ResourceOwner resowner;
-
 	JitInstrumentation instr;
 } JitContext;
 
diff --git a/src/include/jit/llvmjit.h b/src/include/jit/llvmjit.h
index 9d9db80662..420775b189 100644
--- a/src/include/jit/llvmjit.h
+++ b/src/include/jit/llvmjit.h
@@ -39,6 +39,9 @@ typedef struct LLVMJitContext
 {
 	JitContext	base;
 
+	/* used to ensure cleanup of context */
+	ResourceOwner resowner;
+
 	/* number of modules created */
 	size_t		module_generation;
 
-- 
2.43.0