Re: Memory consumed by child SpecialJoinInfo in partitionwise join planning

From: Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>
To: Amit Langote <amitlangote09(at)gmail(dot)com>
Cc: Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com>, Andrei Lepikhov <a(dot)lepikhov(at)postgrespro(dot)ru>, vignesh C <vignesh21(at)gmail(dot)com>, Richard Guo <guofenglinux(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Memory consumed by child SpecialJoinInfo in partitionwise join planning
Date: 2024-03-18 11:11:27
Message-ID: CAExHW5v+hS_E+M99bE1zryHY+kQPdgQEHR-t1KZnYtOOeDEmjw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Amit,

On Fri, Mar 15, 2024 at 11:45 AM Amit Langote <amitlangote09(at)gmail(dot)com>
wrote:

> > >
> > > 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.
>

Attached patch-set

0001 - original patch as is
0002 - addresses your first set of comments
0003 - uses palloc and pfree to allocate and deallocate SpecialJoinInfo
structure.

I will squash both 0002 and 0003 into 0001 once you review those changes
and are fine with those.

>
> > > 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?
>
>
Here are they
tables | master | patched
--------+---------+---------
2 | 29 MB | 28 MB
3 | 102 MB | 93 MB
4 | 307 MB | 263 MB
5 | 1076 MB | 843 MB

The numbers look slightly different from my earlier numbers. But they were
quite old. The patch used to measure memory that time is different from the
one that we committed. So there's a slight difference in the way we measure
memory as well.

--
Best Wishes,
Ashutosh Bapat

Attachment Content-Type Size
0002-Address-Amit-s-comments-20240318.patch text/x-patch 2.3 KB
0001-Reduce-memory-used-by-child-SpecialJoinInfo-20240318.patch text/x-patch 12.1 KB
0003-Use-palloc-ed-SpecialJoinInfo-instead-of-on-20240318.patch text/x-patch 5.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2024-03-18 11:15:16 Re: speed up a logical replica setup
Previous Message Peter Eisentraut 2024-03-18 11:06:30 Re: speed up a logical replica setup