Re: heapgettup refactoring

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

In response to

Responses

Browse pgsql-hackers by date

  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)