| Lists: | pgsql-hackers |
|---|
| From: | Andy Fan <zhihuifan1213(at)163(dot)com> |
|---|---|
| To: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Is a clearer memory lifespan for outerTuple and innerTuple useful? |
| Date: | 2023-12-15 07:35:15 |
| Message-ID: | 87edfnc3nh.fsf@163.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Lists: | pgsql-hackers |
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.fsf@163.com
| Attachment | Content-Type | Size |
|---|---|---|
| v1-0001-Use-MemoryContext-instead-of-ExprContext-for-node.patch | text/x-diff | 4.4 KB |
| unknown_filename | text/plain | 27 bytes |
| From: | Andy Fan <zhihuifan1213(at)163(dot)com> |
|---|---|
| To: | Andy Fan <zhihuifan1213(at)163(dot)com> |
| Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: Is a clearer memory lifespan for outerTuple and innerTuple useful? |
| Date: | 2023-12-15 07:51:10 |
| Message-ID: | 874jgjc2mg.fsf@163.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Lists: | pgsql-hackers |
Andy Fan <zhihuifan1213(at)163(dot)com> 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
| From: | Andy Fan <zhihuifan1213(at)163(dot)com> |
|---|---|
| To: | Andy Fan <zhihuifan1213(at)163(dot)com> |
| Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
| Subject: | Re: Is a clearer memory lifespan for outerTuple and innerTuple useful? |
| Date: | 2023-12-17 13:46:52 |
| Message-ID: | 877cldx7h8.fsf@163.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Lists: | pgsql-hackers |
Andy Fan <zhihuifan1213(at)163(dot)com> writes:
> Andy Fan <zhihuifan1213(at)163(dot)com> 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.
| Attachment | Content-Type | Size |
|---|---|---|
| v2-0001-Use-MemoryContext-instead-of-ExprContext-for-node.patch | text/x-diff | 4.2 KB |
| unknown_filename | text/plain | 27 bytes |
| From: | Nikita Malakhov <hukutoc(at)gmail(dot)com> |
|---|---|
| To: | Andy Fan <zhihuifan1213(at)163(dot)com> |
| Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
| Subject: | Re: Is a clearer memory lifespan for outerTuple and innerTuple useful? |
| Date: | 2023-12-17 21:20:22 |
| Message-ID: | CAN-LCVOvP70kb0hhMptjd-_NYufZScdTsq8fc9LBVadWNV+Rag@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Lists: | pgsql-hackers |
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 <zhihuifan1213(at)163(dot)com> wrote:
>
> Andy Fan <zhihuifan1213(at)163(dot)com> writes:
>
> > Andy Fan <zhihuifan1213(at)163(dot)com> 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/
| From: | Andy Fan <zhihuifan1213(at)163(dot)com> |
|---|---|
| To: | Nikita Malakhov <hukutoc(at)gmail(dot)com> |
| Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers(at)lists(dot)postgresql(dot)org |
| Subject: | Re: Is a clearer memory lifespan for outerTuple and innerTuple useful? |
| Date: | 2023-12-17 23:22:31 |
| Message-ID: | 8734w0xuyc.fsf@163.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Lists: | pgsql-hackers |
Nikita Malakhov <hukutoc(at)gmail(dot)com> 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 <zhihuifan1213(at)163(dot)com> wrote:
>
> Andy Fan <zhihuifan1213(at)163(dot)com> writes:
>
> > Andy Fan <zhihuifan1213(at)163(dot)com> 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