Hi Matthias, On Fri, Mar 7, 2025 at 4:08 AM Matthias van de Meent <boekewurm+postg...@gmail.com> wrote: > > On Tue, 4 Mar 2025 at 20:50, Tomas Vondra <to...@vondra.me> wrote: > > > > I pushed the two smaller parts today. > > > > Here's the remaining two parts, to keep cfbot happy. I don't expect to > > get these into PG18, though. > > As promised on- and off-list, here's the 0001 patch, polished, split, > and further adapted for performance. > > As seen before, it reduces tempspace requirements by up to 50%. I've > not tested this against HEAD for performance. > > It has been split into: > > 0001: Some API cleanup/changes that creaped into the patch. This > removes manual length-passing from the gin tuplesort APIs, instead > relying on GinTuple's tuplen field. It's not critical for anything, > and could be ignored if so desired. > > 0002: Tuplesort changes to allow TupleSort users to buffer and merge > tuples during the sort operations. > The patch was pulled directly from [0] (which was derived from earlier > work in this thread), is fairly easy to understand, and has no other > moving parts. > > 0003: Deduplication in tuplesort's flush-to-disk actions, utilizing > API introduced with 0002. > This improves temporary disk usage by deduplicating data even further, > for when there's a lot of duplicated data but the data has enough > distinct values to not fit in the available memory. > > 0004: Use a single tuplesort. This removes the worker-local tuplesort > in favor of only storing data in the global one. > > This mainly reduces the code size and complexity of parallel GIN > builds; we already were using that global sort for various tasks. > > Open questions and open items for this: > - I did not yet update the pg_stat_progress systems, nor docs. > - Maybe 0003 needs further splitting up, one for the optimizations in > GinBuffer, one for the tuplesort buffering.
Yes, please. That would simplify the detailed review. > - Maybe we need to trim the buffer in gin's tuplesort flush? I didn't get it. Could you please elaborate more on this? > - Maybe we should grow the GinBuffer->items array superlinearly rather > than to the exact size requirement of the merge operation. +1 for this > Apart from the complexities in 0003, I think the changes are fairly > straightforward. Yes, 0001, 0002 and 0004 are pretty straightforward. Regarding 0003, having separate deformed tuple in GinBuffer and cached tuples looks a bit cumbersome. Could we simplify this? I understand that we need to decompress items array lazily. But could we leave just items-related fields in GinBuffer, but have the rest always in GinBuffer.cached? So, if GinBuffer.items != NULL then we have items decompressed already, otherwise have to decompress them when needed. ------ Regards, Alexander Korotkov Supabase
