heapgettup refactoring

Melanie Plageman <melanieplageman@gmail.com>

View thread View on postgr.es

From: Melanie Plageman <melanieplageman@gmail.com>

To: Pg Hackers <pgsql-hackers@postgresql.org>

Cc: Andres Freund <andres@anarazel.de>

Date: 2022-10-31T18:37:39Z

Lists: pgsql-hackers

Attachments

v1-0002-Turn-HeapKeyTest-macro-into-function.patch (application/octet-stream) patch v1-0002
v1-0001-Remove-breaks-in-HeapTupleSatisfiesVisibility.patch (application/octet-stream) patch v1-0001
v1-0003-Refactor-heapgettup-and-heapgetpage.patch (application/octet-stream) patch v1-0003

Hi,

Attached is a patchset to refactor heapgettup(), heapgettup_pagemode(),
and heapgetpage(). heapgettup() and heapgettup_pagemode() have a lot of
duplicated code, confusingly nested if statements, and unnecessary local
variables. While working on a feature for the AIO/DIO patchset, I
noticed that it was difficult to add new code to heapgettup() and
heapgettup_pagemode() because of how the functions are written.

I've taken a stab at refactoring them -- without generating less
efficient code or causing regressions. I'm interested if people find it
more readable and if those with more assembly expertise see issues (new
branches added which are not highly predictable, etc). I took a look at
the assembly for those symbols compiled at O2 but am not experienced
enough at analysis to come to any conclusions.

- Melanie

Commits

Remove stray duplicated comment in heapam.h
- 9ed50ab3496f 16.0 landed
More refactoring of heapgettup() and heapgettup_pagemode()
- cfcf56f92398 16.0 landed
Run pgindent on heapam.c
- 009dbdea02d7 16.0 landed
Push lpp variable closer to usage in heapgetpage()
- e351f8541831 16.0 landed
Variable renaming in preparation for refactoring
- 8e1db29cdbbd 16.0 landed
Turn HeapKeyTest macro into inline function
- 4eb3b1120001 16.0 landed
Remove unused include
- c0f1e51ac79e 16.0 landed
Remove redundant breaks in HeapTupleSatisfiesVisibility
- c3652cd84ac8 16.0 landed