pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feed[PATCH v3 4/4] wip: llvmjit: Plug memory leak when performing inlining.
2+ messages / 2 participants
[nested] [flat]
* [PATCH v3 4/4] wip: llvmjit: Plug memory leak when performing inlining.
@ 2022-11-01 05:35 Andres Freund <andres@anarazel.de>
0 siblings, 0 replies; 2+ messages in thread
From: Andres Freund @ 2022-11-01 05:35 UTC (permalink / raw)
When performing inlining LLVM unfortunately "leaks" types (the types
survive and are usable, but a new round of inlining will recreate new
types that are structurally equivalent). I could not come up with a
backpatchable fix for that, so address the problem by dropping &
recreating the LLVMContextRef that contains all IR related data -
which releases all the types.
That incurs some overhead, so only do so after 10000 JITed queries.
Needs a good bit more work:
- confusing naming between different types of contexts (PG side
LLVMJitContext, and LLVM side LLVMContextRef)
- add detailed explanation of leak issue
- consider only incrementing the counter triggering the recreation of
the memory context during inlining.
- several FIXMEs in code
Reported-By: Justin Pryzby <pryzby@telsasoft.com>
Reported-By: Kurt Roeckx <kurt@roeckx.be>
Reported-By: Jaime Casanova <jcasanov@systemguards.com.ec>
Author: Andres Freund
Discussion: https://postgr.es/m/7acc8678-df5f-4923-9cf6-e843131ae89d@www.fastmail.com
Discussion: https://postgr.es/m/20201218235607.GC30237@telsasoft.com
---
src/include/jit/llvmjit.h | 8 +++
src/backend/jit/llvm/llvmjit.c | 76 ++++++++++++++++++++++++-
src/backend/jit/llvm/llvmjit_inline.cpp | 12 ++++
3 files changed, 94 insertions(+), 2 deletions(-)
inline patch 1: src/include/jit/llvmjit.h, src/backend/jit/llvm/llvmjit.c, src/backend/jit/llvm/llvmjit_inline.cpp (+94/-2 lines) download inline-1.patch
diff --git a/src/include/jit/llvmjit.h b/src/include/jit/llvmjit.h
index f4c01c28672..d90d7b2e347 100644
--- a/src/include/jit/llvmjit.h
+++ b/src/include/jit/llvmjit.h
@@ -42,6 +42,13 @@ typedef struct LLVMJitContext
/* number of modules created */
size_t module_generation;
+ /*
+ * The LLVM Context used by this JIT context. An LLVM context is reused
+ * across many compilations, but occasionally reset to prevent it using
+ * too much memory due to more and more types accumulating.
+ */
+ LLVMContextRef llvm_context;
+
/* current, "open for write", module */
LLVMModuleRef module;
@@ -99,6 +106,7 @@ extern LLVMValueRef llvm_function_reference(LLVMJitContext *context,
LLVMModuleRef mod,
FunctionCallInfo fcinfo);
+extern void llvm_inline_reset_caches(void);
extern void llvm_inline(LLVMModuleRef mod);
/*
diff --git a/src/backend/jit/llvm/llvmjit.c b/src/backend/jit/llvm/llvmjit.c
index 3625edceb3e..a7e19a69a8b 100644
--- a/src/backend/jit/llvm/llvmjit.c
+++ b/src/backend/jit/llvm/llvmjit.c
@@ -42,6 +42,10 @@
#include "utils/memutils.h"
#include "utils/resowner_private.h"
+
+#define LLVMJIT_LLVM_CONTEXT_REUSE_MAX 100
+
+
/* Handle of a module emitted via ORC JIT */
typedef struct LLVMJitHandle
{
@@ -81,6 +85,10 @@ static LLVMModuleRef llvm_types_module = NULL;
static bool llvm_session_initialized = false;
static size_t llvm_generation = 0;
+/* number of LLVMJitContexts that currently are in use */
+static size_t llvm_jit_context_in_use_count = 0;
+/* how many times has the current LLVMContextRef been used */
+static size_t llvm_llvm_context_reuse_count = 0;
static const char *llvm_triple = NULL;
static const char *llvm_layout = NULL;
static LLVMContextRef llvm_context;
@@ -141,6 +149,55 @@ llvm_create_context(int jitFlags)
llvm_session_initialize();
+ /*
+ * Every now and then create a new LLVMContextRef. Unfortunately, during
+ * every round of inlining, types may "leak" (they can still be found/used
+ * via the context, but new types will be created the next time in
+ * inlining is performed). To prevent that from slowly accumulating
+ * problematic amounts of memory, recreate the LLVMContextRef we use. We
+ * don't want to do so too often, as that implies some overhead
+ * (particularly re-loading the module summaries / modules is fairly
+ * expensive).
+ *
+ * We can only safely recreate the LLVM context if no other code is being
+ * JITed, otherwise we'd release the types in use for that.
+ *
+ * FIXME: Extract into helper function.
+ */
+ if (llvm_jit_context_in_use_count == 0 &&
+ llvm_llvm_context_reuse_count > LLVMJIT_LLVM_CONTEXT_REUSE_MAX)
+ {
+ llvm_llvm_context_reuse_count = 0;
+
+ Assert(llvm_context != NULL);
+
+ /*
+ * Need to reset the modules that the inlining code caches before
+ * disposing of the context. LLVM modules exist within a specific LLVM
+ * context, therefore disposing of the context before resetting the
+ * cache would lead to dangling pointers to modules.
+ */
+ llvm_inline_reset_caches();
+
+ if (llvm_context != NULL)
+ LLVMContextDispose(llvm_context);
+ llvm_context = LLVMContextCreate();
+
+ /*
+ * Re-build cached type information, so code generation code can rely
+ * on that information to be present (also prevents the variables to
+ * be dangling references).
+ *
+ * FIXME: should split the handling of llvm_triple / llvm_layout out
+ * of llvm_create_types() - that doesn't need to be redone.
+ */
+ llvm_create_types();
+ }
+ else
+ {
+ llvm_llvm_context_reuse_count++;
+ }
+
ResourceOwnerEnlargeJIT(CurrentResourceOwner);
context = MemoryContextAllocZero(TopMemoryContext,
@@ -151,6 +208,8 @@ llvm_create_context(int jitFlags)
context->base.resowner = CurrentResourceOwner;
ResourceOwnerRememberJIT(CurrentResourceOwner, PointerGetDatum(context));
+ llvm_jit_context_in_use_count++;
+
return context;
}
@@ -163,6 +222,12 @@ llvm_release_context(JitContext *context)
LLVMJitContext *llvm_context = (LLVMJitContext *) context;
ListCell *lc;
+ /*
+ * Consider as cleaned up even if we skip doing so below, that way we can
+ * verify the tracking is correct (see llvm_shutdown()).
+ */
+ llvm_jit_context_in_use_count--;
+
/*
* When this backend is exiting, don't clean up LLVM. As an error might
* have occurred from within LLVM, we do not want to risk reentering. All
@@ -901,6 +966,10 @@ llvm_shutdown(int code, Datum arg)
return;
}
+ if (llvm_jit_context_in_use_count != 0)
+ elog(PANIC, "LLVMJitContext in use count not 0 at exit (is %zu)",
+ llvm_jit_context_in_use_count);
+
#if LLVM_VERSION_MAJOR > 11
{
if (llvm_opt3_orc)
@@ -1004,8 +1073,11 @@ llvm_create_types(void)
* Load triple & layout from clang emitted file so we're guaranteed to be
* compatible.
*/
- llvm_triple = pstrdup(LLVMGetTarget(llvm_types_module));
- llvm_layout = pstrdup(LLVMGetDataLayoutStr(llvm_types_module));
+ if (llvm_triple == NULL)
+ {
+ llvm_triple = pstrdup(LLVMGetTarget(llvm_types_module));
+ llvm_layout = pstrdup(LLVMGetDataLayoutStr(llvm_types_module));
+ }
TypeSizeT = llvm_pg_var_type("TypeSizeT");
TypeParamBool = load_return_type(llvm_types_module, "FunctionReturningBool");
diff --git a/src/backend/jit/llvm/llvmjit_inline.cpp b/src/backend/jit/llvm/llvmjit_inline.cpp
index 73fe253f2ec..6b46e141fcb 100644
--- a/src/backend/jit/llvm/llvmjit_inline.cpp
+++ b/src/backend/jit/llvm/llvmjit_inline.cpp
@@ -152,6 +152,18 @@ summaries_for_guid(const InlineSearchPath& path, llvm::GlobalValue::GUID guid);
#define ilog(...) (void) 0
#endif
+/*
+ * Reset inlining related state. This needs to be called before the currently
+ * used LLVMContextRef is disposed (and a new one create), otherwise we would
+ * have dangling references to deleted modules.
+ */
+void
+llvm_inline_reset_caches(void)
+{
+ module_cache->clear();
+ summary_cache->clear();
+}
+
/*
* Perform inlining of external function references in M based on a simple
* cost based analysis.
--
2.38.0
* BUG #18325: Possible bug with plpgsql function + ALTER TABLE DROP COLUMN
@ 2024-02-02 18:39 PG Bug reporting form <noreply@postgresql.org>
0 siblings, 0 replies; 2+ messages in thread
From: PG Bug reporting form @ 2024-02-02 18:39 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org; +Cc: antti@kiinnost.us
The following bug has been logged on the website:
Bug reference: 18325
Logged by: Antti Risteli
Email address: antti@kiinnost.us
PostgreSQL version: 16.1
Operating system: Debian Linux
Description:
Hi,
we encountered a production problem after dropping a column that was used in
a
plpgsql function. While I am not 100% certain that this is a bug it was at
least
not mentioned in any documentation I found. While I'm not very familiar
with
PostgreSQL internals I believe the cause might be related to a plan being
cached
and not properly invalidated as the function call works from another
connection
initiated after the ALTER TABLE.
We initially encountered this on version 14.10 but it also happens on
16.1.
Here's a reproduction (which still might not be minimal but it is heavily
simplified from our actual case) when ran from a single connection:
DROP TABLE IF EXISTS abc;
CREATE TABLE abc (pk integer, test_id text);
INSERT INTO abc VALUES (122,'ejife');
CREATE OR REPLACE FUNCTION test_function() RETURNS integer
AS $body$
DECLARE
abc_record abc;
abc_records abc[] = array[]::abc[];
abc_record2 abc;
BEGIN
FOR abc_record IN SELECT * FROM abc
LOOP
abc_records := abc_records || abc_record;
FOR abc_record2 IN SELECT * FROM unnest(abc_records)
LOOP
RETURN abc_record2.pk;
END LOOP;
END LOOP;
RETURN 1;
END
$body$
LANGUAGE plpgsql;
-- this prints 122
SELECT test_function();
ALTER TABLE abc DROP COLUMN test_id;
-- this should print 122
SELECT test_function();
-- but it actually prints
ERROR: attribute 2 of type record has been dropped
CONTEXT: PL/pgSQL function test_function() line 10 at FOR over SELECT
rows
At this point SELECT test_function() works normally on connections opened
after
ALTER TABLE.
Br,
antti
^ permalink raw reply [nested|flat] 2+ messages in thread
end of thread, other threads:[~2024-02-02 18:39 UTC | newest]
Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2022-11-01 05:35 [PATCH v3 4/4] wip: llvmjit: Plug memory leak when performing inlining. Andres Freund <andres@anarazel.de>
2024-02-02 18:39 BUG #18325: Possible bug with plpgsql function + ALTER TABLE DROP COLUMN PG Bug reporting form <noreply@postgresql.org>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox