Re: Is a clearer memory lifespan for outerTuple and innerTuple useful?
Nikita Malakhov writes: > Hi! > > Maybe, the alternative way is using a separate kind of context, say name it > 'ToastContext' for all custom data related to Toasted values? What do > you think? That should be a candidate. The latest research makes me think the 'detoast_values' should have the same life cycles as tts_values, so the memory should be managed by TupleTuleSlot (rather than ExprContext) and be handled in ExecCopySlot / ExecClearSlot stuff. In TupleTableSlot we already have a tts_mctx MemoryContext, reusing it needs using 'pfree' to free the detoast values and but a dedicated memory context pays more costs on the setup, but a more efficient MemoryContextReset. > > On Sun, Dec 17, 2023 at 4:52 PM Andy Fan wrote: > > Andy Fan writes: > > > Andy Fan writes: > > > >> ..., I attached the 2 MemoryContext in > >> JoinState rather than MergeJoinState, which is for the "shared detoast > >> value"[0] more or less. > >> > > In order to delimit the scope of this discussion, I attached the 2 > MemoryContext to MergeJoinState. Since the code was writen by Tom at > 2005, so add Tom to the cc-list. > However this patch can be discussed seperately. -- Best Regards Andy Fan
Re: Is a clearer memory lifespan for outerTuple and innerTuple useful?
Hi! Maybe, the alternative way is using a separate kind of context, say name it 'ToastContext' for all custom data related to Toasted values? What do you think? On Sun, Dec 17, 2023 at 4:52 PM Andy Fan wrote: > > Andy Fan writes: > > > Andy Fan writes: > > > >> ..., I attached the 2 MemoryContext in > >> JoinState rather than MergeJoinState, which is for the "shared detoast > >> value"[0] more or less. > >> > > In order to delimit the scope of this discussion, I attached the 2 > MemoryContext to MergeJoinState. Since the code was writen by Tom at > 2005, so add Tom to the cc-list. > > > -- > Best Regards > Andy Fan > -- Regards, Nikita Malakhov Postgres Professional The Russian Postgres Company https://postgrespro.ru/
Re: Is a clearer memory lifespan for outerTuple and innerTuple useful?
Andy Fan writes: > Andy Fan writes: > >> ..., I attached the 2 MemoryContext in >> JoinState rather than MergeJoinState, which is for the "shared detoast >> value"[0] more or less. >> In order to delimit the scope of this discussion, I attached the 2 MemoryContext to MergeJoinState. Since the code was writen by Tom at 2005, so add Tom to the cc-list. >From 20068bdda00716da712fb3f1e554401e81924e19 Mon Sep 17 00:00:00 2001 From: "yizhi.fzh" Date: Sun, 17 Dec 2023 21:41:34 +0800 Subject: [PATCH v2 1/1] Use MemoryContext instead of ExprContext for nodeMergejoin.c MergeJoin needs to store the values in MergeJoinClause for a longer lifespan, in the past it uses a dedicate ExprContext, however a MemoryContext should be OK. This patch does this. --- src/backend/executor/nodeMergejoin.c | 29 +--- src/include/nodes/execnodes.h| 5 +++-- 2 files changed, 21 insertions(+), 13 deletions(-) diff --git a/src/backend/executor/nodeMergejoin.c b/src/backend/executor/nodeMergejoin.c index 3cdab77dfc..4d45305482 100644 --- a/src/backend/executor/nodeMergejoin.c +++ b/src/backend/executor/nodeMergejoin.c @@ -294,7 +294,7 @@ MJExamineQuals(List *mergeclauses, static MJEvalResult MJEvalOuterValues(MergeJoinState *mergestate) { - ExprContext *econtext = mergestate->mj_OuterEContext; + ExprContext *econtext = mergestate->js.ps.ps_ExprContext; MJEvalResult result = MJEVAL_MATCHABLE; int i; MemoryContext oldContext; @@ -303,9 +303,9 @@ MJEvalOuterValues(MergeJoinState *mergestate) if (TupIsNull(mergestate->mj_OuterTupleSlot)) return MJEVAL_ENDOFJOIN; - ResetExprContext(econtext); + MemoryContextReset(mergestate->mj_outerTuple_memory); - oldContext = MemoryContextSwitchTo(econtext->ecxt_per_tuple_memory); + oldContext = MemoryContextSwitchTo(mergestate->mj_outerTuple_memory); econtext->ecxt_outertuple = mergestate->mj_OuterTupleSlot; @@ -341,7 +341,7 @@ MJEvalOuterValues(MergeJoinState *mergestate) static MJEvalResult MJEvalInnerValues(MergeJoinState *mergestate, TupleTableSlot *innerslot) { - ExprContext *econtext = mergestate->mj_InnerEContext; + ExprContext *econtext = mergestate->js.ps.ps_ExprContext; MJEvalResult result = MJEVAL_MATCHABLE; int i; MemoryContext oldContext; @@ -350,9 +350,9 @@ MJEvalInnerValues(MergeJoinState *mergestate, TupleTableSlot *innerslot) if (TupIsNull(innerslot)) return MJEVAL_ENDOFJOIN; - ResetExprContext(econtext); + MemoryContextReset(mergestate->mj_innerTuple_memory); - oldContext = MemoryContextSwitchTo(econtext->ecxt_per_tuple_memory); + oldContext = MemoryContextSwitchTo(mergestate->mj_innerTuple_memory); econtext->ecxt_innertuple = innerslot; @@ -1447,6 +1447,7 @@ ExecInitMergeJoin(MergeJoin *node, EState *estate, int eflags) TupleDesc outerDesc, innerDesc; const TupleTableSlotOps *innerOps; + ExprContext *econtext; /* check for unsupported flags */ Assert(!(eflags & (EXEC_FLAG_BACKWARD | EXEC_FLAG_MARK))); @@ -1471,13 +1472,19 @@ ExecInitMergeJoin(MergeJoin *node, EState *estate, int eflags) */ ExecAssignExprContext(estate, >js.ps); + econtext = mergestate->js.ps.ps_ExprContext; + /* - * we need two additional econtexts in which we can compute the join - * expressions from the left and right input tuples. The node's regular - * econtext won't do because it gets reset too often. + * we need two additional contexts in which we can compute the join + * expressions from the left and right input tuples. The econtext's + * regular ecxt_per_tuple_memory won't do because it gets reset too often. */ - mergestate->mj_OuterEContext = CreateExprContext(estate); - mergestate->mj_InnerEContext = CreateExprContext(estate); + mergestate->mj_outerTuple_memory = AllocSetContextCreate(econtext->ecxt_per_query_memory, + "OuterTupleCtx", + ALLOCSET_SMALL_SIZES); + mergestate->mj_innerTuple_memory = AllocSetContextCreate(econtext->ecxt_per_query_memory, + "InnerTupleCtx", + ALLOCSET_SMALL_SIZES); /* * initialize child nodes diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index 5d7f17dee0..e8af959c01 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -2064,8 +2064,9 @@ typedef struct MergeJoinState TupleTableSlot *mj_MarkedTupleSlot; TupleTableSlot *mj_NullOuterTupleSlot; TupleTableSlot *mj_NullInnerTupleSlot; - ExprContext *mj_OuterEContext; - ExprContext *mj_InnerEContext; + + MemoryContext mj_outerTuple_memory; + MemoryContext mj_innerTuple_memory; } MergeJoinState; /* -- 2.34.1 -- Best Regards Andy Fan
Re: Is a clearer memory lifespan for outerTuple and innerTuple useful?
Andy Fan writes: > ..., I attached the 2 MemoryContext in > JoinState rather than MergeJoinState, which is for the "shared detoast > value"[0] more or less. > After thinking more, if it is designed for "shared detoast value" patch (happens on ExecInterpExpr stage), the inner_tuple_memory and outer_tuple_memory should be attached to ExprContext rather than JoinState since it is more natual to access ExprConext (compared with JoinState) in ExecInterpExpr. I didn't attach a new version for this, any feedback will be appreciated. -- Best Regards Andy Fan
Is a clearer memory lifespan for outerTuple and innerTuple useful?
Hi, When I am working on "shared detoast value"[0], where I want to avoid detoast the same datum over and over again, I have to decide which memory context should be used to hold the detoast value. later I found I have to use different MemoryContexts for the OuterTuple and innerTuple since OuterTuple usually have a longer lifespan. I found the following code in nodeMergeJoin.c which has pretty similar situation, just that it uses ExprContext rather than MemoryContext. MergeJoinState * ExecInitMergeJoin(MergeJoin *node, EState *estate, int eflags) /* * we need two additional econtexts in which we can compute the join * expressions from the left and right input tuples. The node's regular * econtext won't do because it gets reset too often. */ mergestate->mj_OuterEContext = CreateExprContext(estate); mergestate->mj_InnerEContext = CreateExprContext(estate); IIUC, we needs a MemoryContext rather than ExprContext in fact. In the attachment, I just use two MemoryContext instead of the two ExprContexts which should be less memory and more precise semantics, and works fine. shall we go in this direction? I attached the 2 MemoryContext in JoinState rather than MergeJoinState, which is for the "shared detoast value"[0] more or less. [0] https://www.postgresql.org/message-id/87ttoyihgm@163.com >From cadd94f8ff2398ca430b43494b3eb71225b017c3 Mon Sep 17 00:00:00 2001 From: "yizhi.fzh" Date: Fri, 15 Dec 2023 15:28:36 +0800 Subject: [PATCH v1] Use MemoryContext instead of ExprContext for nodeMergejoin.c --- src/backend/executor/nodeMergejoin.c | 27 ++- src/backend/executor/nodeNestloop.c | 1 + src/include/nodes/execnodes.h| 4 ++-- 3 files changed, 17 insertions(+), 15 deletions(-) diff --git a/src/backend/executor/nodeMergejoin.c b/src/backend/executor/nodeMergejoin.c index 3cdab77dfc..d330e104f6 100644 --- a/src/backend/executor/nodeMergejoin.c +++ b/src/backend/executor/nodeMergejoin.c @@ -294,7 +294,7 @@ MJExamineQuals(List *mergeclauses, static MJEvalResult MJEvalOuterValues(MergeJoinState *mergestate) { - ExprContext *econtext = mergestate->mj_OuterEContext; + ExprContext *econtext = mergestate->js.ps.ps_ExprContext; MJEvalResult result = MJEVAL_MATCHABLE; int i; MemoryContext oldContext; @@ -303,9 +303,9 @@ MJEvalOuterValues(MergeJoinState *mergestate) if (TupIsNull(mergestate->mj_OuterTupleSlot)) return MJEVAL_ENDOFJOIN; - ResetExprContext(econtext); + MemoryContextReset(mergestate->js.outer_tuple_memory); - oldContext = MemoryContextSwitchTo(econtext->ecxt_per_tuple_memory); + oldContext = MemoryContextSwitchTo(mergestate->js.outer_tuple_memory); econtext->ecxt_outertuple = mergestate->mj_OuterTupleSlot; @@ -341,7 +341,7 @@ MJEvalOuterValues(MergeJoinState *mergestate) static MJEvalResult MJEvalInnerValues(MergeJoinState *mergestate, TupleTableSlot *innerslot) { - ExprContext *econtext = mergestate->mj_InnerEContext; + ExprContext *econtext = mergestate->js.ps.ps_ExprContext; MJEvalResult result = MJEVAL_MATCHABLE; int i; MemoryContext oldContext; @@ -352,7 +352,7 @@ MJEvalInnerValues(MergeJoinState *mergestate, TupleTableSlot *innerslot) ResetExprContext(econtext); - oldContext = MemoryContextSwitchTo(econtext->ecxt_per_tuple_memory); + oldContext = MemoryContextSwitchTo(mergestate->js.inner_tuple_memory); econtext->ecxt_innertuple = innerslot; @@ -1471,14 +1471,6 @@ ExecInitMergeJoin(MergeJoin *node, EState *estate, int eflags) */ ExecAssignExprContext(estate, >js.ps); - /* - * we need two additional econtexts in which we can compute the join - * expressions from the left and right input tuples. The node's regular - * econtext won't do because it gets reset too often. - */ - mergestate->mj_OuterEContext = CreateExprContext(estate); - mergestate->mj_InnerEContext = CreateExprContext(estate); - /* * initialize child nodes * @@ -1497,6 +1489,15 @@ ExecInitMergeJoin(MergeJoin *node, EState *estate, int eflags) (eflags | EXEC_FLAG_MARK)); innerDesc = ExecGetResultType(innerPlanState(mergestate)); + mergestate->js.outer_tuple_memory = AllocSetContextCreate( + mergestate->js.ps.ps_ExprContext->ecxt_per_query_memory, + "OuterTupleCtx", + ALLOCSET_SMALL_SIZES); + mergestate->js.inner_tuple_memory = AllocSetContextCreate( + mergestate->js.ps.ps_ExprContext->ecxt_per_query_memory, + "InnerTupleCtx", + ALLOCSET_SMALL_SIZES); + /* * For certain types of inner child nodes, it is advantageous to issue * MARK every time we advance past an inner tuple we will never return to. diff --git a/src/backend/executor/nodeNestloop.c b/src/backend/executor/nodeNestloop.c index ebd1406843..5e10d01c4e 100644 --- a/src/backend/executor/nodeNestloop.c +++ b/src/backend/executor/nodeNestloop.c @@ -349,6 +349,7 @@ ExecInitNestLoop(NestLoop *node, EState *estate, int eflags) NL1_printf("ExecInitNestLoop: %s\n", "node