Re: Memory consumed by child SpecialJoinInfo in partitionwise join planning

Amit Langote <amitlangote09@gmail.com>

From: Amit Langote <amitlangote09@gmail.com>
To: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Cc: Tomas Vondra <tomas.vondra@enterprisedb.com>, Andrei Lepikhov <a.lepikhov@postgrespro.ru>, vignesh C <vignesh21@gmail.com>, Richard Guo <guofenglinux@gmail.com>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2024-03-15T06:15:07Z
Lists: pgsql-hackers
Hi Ashutosh,

On Mon, Feb 19, 2024 at 10:01 PM Ashutosh Bapat
<ashutosh.bapat.oss@gmail.com> wrote:
> On Sun, Feb 18, 2024 at 10:55 PM Tomas Vondra
> <tomas.vondra@enterprisedb.com> wrote:
> >
> > Hi,
> >
> > I took a quick look at this patch today. I certainly agree with the
> > intent to reduce the amount of memory during planning, assuming it's not
> > overly disruptive. And I think this patch is fairly localized and looks
> > sensible.
>
> Thanks for looking at the patch-set.
> >
> > That being said I'm a big fan of using a local variable on stack and
> > filling it. I'd probably go with the usual palloc/pfree, because that
> > makes it much easier to use - the callers would not be responsible for
> > allocating the SpecialJoinInfo struct. Sure, it's a little bit of
> > overhead, but with the AllocSet caching I doubt it's measurable.
>
> You are suggesting that instead of declaring a local variable of type
> SpecialJoinInfo, build_child_join_sjinfo() should palloc() and return
> SpecialJoinInfo which will be freed by free_child_sjinfo()
> (free_child_sjinfo_members in the patch). I am fine with that.

Agree with Tomas about using makeNode() / pfree().  Having the pfree()
kind of makes it extra-evident that those SpecialJoinInfos are
transitory.

> > I did put this through check-world on amd64/arm64, with valgrind,
> > without any issue. I also tried the scripts shared by Ashutosh in his
> > initial message (with some minor fixes, adding MEMORY to explain etc).
> >
> > The results with the 20240130 patches are like this:
> >
> >    tables    master    patched
> >   -----------------------------
> >         2      40.8       39.9
> >         3     151.7      142.6
> >         4     464.0      418.5
> >         5    1663.9     1419.5

Could you please post the numbers with the palloc() / pfree() version?

-- 
Thanks, Amit Langote



Commits

  1. Code review for 6190d828cd2

  2. Do not translate dummy SpecialJoinInfos for child joins

  3. Reduce memory used by partitionwise joins

  4. Remove dummy_spinlock