Re: Memory consumed by child SpecialJoinInfo in partitionwise join planning

On Mon, Mar 18, 2024 at 20:11 Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> wrote:

Hi Amit,

On Fri, Mar 15, 2024 at 11:45 AM Amit Langote <amitlangote09@gmail.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.

Thanks for the new patches.

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

Sorry, I should’ve mentioned that I was interested in seeing cpu times to compare the two approaches. Specifically, to see if the palloc / frees add noticeable overhead.