Thread: pgsql: Move memory management away from writetup() and tuplesort_put*()
pgsql: Move memory management away from writetup() and tuplesort_put*()
From
Alexander Korotkov
Date:
Move memory management away from writetup() and tuplesort_put*() This commit puts some generic work away from sort-variant-specific function. In particular, tuplesort_put*() now doesn't need to decrease available memory and switch to sort context before calling puttuple_common(). writetup() doesn't need to free SortTuple.tuple and increase available memory. Discussion: https://postgr.es/m/CAPpHfdvjix0Ahx-H3Jp1M2R%2B_74P-zKnGGygx4OWr%3DbUQ8BNdw%40mail.gmail.com Author: Alexander Korotkov Reviewed-by: Pavel Borisov, Maxim Orlov, Matthias van de Meent Reviewed-by: Andres Freund, John Naylor Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/097366c45f5dfe142eb232dc6d348ca0705a63a9 Modified Files -------------- src/backend/utils/sort/tuplesort.c | 78 ++++++++++++++++---------------------- 1 file changed, 33 insertions(+), 45 deletions(-)
On Wed, 27 Jul 2022 at 17:29, Alexander Korotkov <akorotkov@postgresql.org> wrote: > src/backend/utils/sort/tuplesort.c | 78 ++++++++++++++++---------------------- I was wondering about the following comment that this commit added: +/* + * Write a stored tuple onto tape.tuple. Unless the slab allocator is + * used, after writing the tuple, pfree() the out-of-line data (not the + * SortTuple struct!), and increase state->availMem by the amount of + * memory space thereby released. + */ static void writetuple(Tuplesortstate *state, LogicalTape *tape, SortTuple *stup) LogicalTable has no field named 'tuple' so I'm thinking the "tape.tuple" is a mistake? If so, maybe the comment should be: /* * Write 'stup' out onto 'tape' and, unless using the slab allocator, pfree stup's * tuple and adjust the memory accounting accordingly. */ David
Re: pgsql: Move memory management away from writetup() and tuplesort_put*()
From
Alexander Korotkov
Date:
Hi, David! On Fri, Aug 26, 2022 at 3:53 AM David Rowley <dgrowleyml@gmail.com> wrote: > > On Wed, 27 Jul 2022 at 17:29, Alexander Korotkov > <akorotkov@postgresql.org> wrote: > > src/backend/utils/sort/tuplesort.c | 78 ++++++++++++++++---------------------- > > I was wondering about the following comment that this commit added: > > +/* > + * Write a stored tuple onto tape.tuple. Unless the slab allocator is > + * used, after writing the tuple, pfree() the out-of-line data (not the > + * SortTuple struct!), and increase state->availMem by the amount of > + * memory space thereby released. > + */ > static void > writetuple(Tuplesortstate *state, LogicalTape *tape, SortTuple *stup) > > LogicalTable has no field named 'tuple' so I'm thinking the > "tape.tuple" is a mistake? Thank you for catching this. Yes, it definitely should be just "tape", not "tape.tuple". > If so, maybe the comment should be: > > /* > * Write 'stup' out onto 'tape' and, unless using the slab allocator, > pfree stup's > * tuple and adjust the memory accounting accordingly. > */ I've just pushed a fix for this. But I didn't change the comment to refer veritable names, just fixed the typo. ------ Regards, Alexander Korotkov