On Sun, Sep 14, 2025 at 04:40:54PM +0300, Konstantin Knizhnik wrote: > On 13/09/2025 10:10 PM, Peter Geoghegan wrote: >> I think that _bt_split could easily be switched over to using 2 local >> PGAlignedBlock variables, removing its use of PageGetTempPage. I don't >> think that it is necessary to consider other PageGetTempPage callers.
Hmm. I didn't really agree with this last statement, especially if these code paths don't need to manipulate Page pointers across the stack. So I have taken this opportunity to double-check all the existing calls of PageGetTempPage() which is an API old enough to vote (105409746499?), while 44cac9346479 has introduced PGAlignedBlock much later, seeing if there are some opportunities here: - dataSplitPageInternal() and entrySplitPage() expect the contents of the pages to be returned to the caller. We could use PGAlignedBlock with pre-prepared pages that don't get allocated, but this requires the callers of the internal routines to take responsibility for that, like dataBeginPlaceToPage(). The complexity is not appealing in these case. - The same argument applies to the call in ginPlaceToPage(), passing down the page to fillRoot(). - gistplacetopage(), with a copy of a page filled its special area, manipulated across other calls. - An optimization exists for btree_xlog_split() where a temporary page is not manipulated, but the fact that we are in redo does not really matter much for the extra allocation, I guess. At the end, I don't feel excited about switching these cases, where the gains are not obvious. > Attached please find updated version of the patch which uses PGAlignedBlock > instead of PageGetTempPage. + * high key. To prevent confision of VACUUM with orphan page, we also + * first fill in-mempory copy oif this page. Three typos in two lines here (not sure about the best wording that fits with VACUUM, but I like the suggested simplification): s/in-mempory/in-memory/ s/confision/confusion/ s/oif/if/ Saying that, this patch sounds like a good idea, making these code paths a bit more reliable for VACUUM, without paying the cost of an allocation. At least for HEAD, that's nice to have. Peter, what do you think? -- Michael
signature.asc
Description: PGP signature
