Hi Ashutosh, On Mon, Feb 19, 2024 at 10:01 PM Ashutosh Bapat <ashutosh.bapat....@gmail.com> wrote: > On Sun, Feb 18, 2024 at 10:55 PM Tomas Vondra > <tomas.von...@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
