Horton

Re: A potential memory leak on Merge Join when Sort node is not below Materialize node

Ronan Dunklau <ronan.dunklau@aiven.io>

View thread View on postgr.es
From: Ronan Dunklau <ronan.dunklau@aiven.io>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: David Rowley <dgrowleyml@gmail.com>, Önder Kalacı <onderkalaci@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2022-09-29T14:15:25Z
Lists: pgsql-hackers 
Le jeudi 29 septembre 2022, 16:10:03 CEST Tom Lane a écrit :
> Ronan Dunklau <ronan.dunklau@aiven.io> writes:
> >> Yeah, I think the same rules around scope apply as
> >> tuplesort_gettupleslot() with copy==false.  We could do it by adding a
> >> copy flag to the existing function, but I'd rather not add the
> >> branching to that function. It's probably just better to duplicate it
> >> and adjust.
> > 
> > For the record, I tried to see if gcc would optimize the function by
> > generating two different versions when copy is true or false, thus getting 
rid
> > of the branching while still having only one function to deal with.
> 
> TBH, I think this is completely ridiculous over-optimization.
> There's exactly zero evidence that a second copy of the function
> would improve performance, or do anything but contribute to code
> bloat (which does have a distributed performance cost).

I wasn't commenting on the merit of the optimization, but just that I tried to 
get gcc to apply it itself, which it doesn't.

Regards,

-- 
Ronan Dunklau

Commits

  1. Allow nodeSort to perform Datum sorts for byref types

  2. Restrict Datum sort optimization to byval types only

  3. Make nodeSort.c use Datum sorts for single column sorts

  4. Fix actual and potential double-frees around tuplesort usage.

  5. Allow avoiding tuple copy within tuplesort_gettupleslot().