From: | David Rowley <dgrowleyml(at)gmail(dot)com> |
---|---|
To: | Melanie Plageman <melanieplageman(at)gmail(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: heapgettup refactoring |
Date: | 2023-02-07 04:40:13 |
Message-ID: | CAApHDvp-ZnPP=CLt1+NZb4iCKQwB8z-SvUsXcUpS+Wu2azW25g@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Fri, 3 Feb 2023 at 15:26, David Rowley <dgrowleyml(at)gmail(dot)com> wrote:
> I've pushed all but the final 2 patches now.
I just pushed the final patch in the series. I held back on moving
the setting of rs_inited back into the heapgettup_initial_block()
helper function as I wondered if we should even keep that field.
It seems that rs_cblock == InvalidBlockNumber in all cases where
rs_inited == false, so maybe it's better just to use that as a
condition to check if the scan has started or not. I've attached a
patch which does that.
I ended up adjusting HeapScanDescData more than what is minimally
required to remove rs_inited. I wondered if rs_cindex should be closer
to rs_cblock in the struct so that we're more likely to be adjusting
the same cache line than if that field were closer to the end of the
struct. We don't need rs_coffset and rs_cindex at the same time, so I
made it a union. I see that the struct is still 712 bytes before and
after this change. I've not yet tested to see if there are any
performance gains to this change.
David
Attachment | Content-Type | Size |
---|---|---|
remove_HeapScanDescData_rs_inited_field.patch | text/plain | 6.4 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Amit Kapila | 2023-02-07 04:40:15 | pgsql: Use appropriate wait event when sending data in the apply worker |
Previous Message | Kyotaro Horiguchi | 2023-02-07 04:37:06 | Re: Time delayed LR (WAS Re: logical replication restrictions) |