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. > > 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 > > That's certainly a nice improvement, and it even reduces the amount of > time for planning (the 5-table join goes from 18s to 17s on my laptop). > That's nice, although 17 seconds for planning is not ... great. > > That being said, the amount of remaining memory needed by planning is > still pretty high - we save ~240MB for a join of 5 tables, but we still > need ~1.4GB. Yes, this is a bit extreme example, and it probably is not > very common to join 5 tables with 1000 partitions each ... > Yes. > Do we know what are the other places consuming the 1.4GB of memory? Please find the breakdown at [1]. Investigating further, I found that memory consumed by Bitmapsets is not explicitly visible through that breakdown. But Bitmapsets consume a lot of memory in this case. I shared google slides with raw numbers in [2]. The Gslides can be found at https://docs.google.com/presentation/d/1S9BiAADhX-Fv9tDbx5R5Izq4blAofhZMhHcO1c-wzfI/edit#slide=id.p. According to that measurement, Bitmapset objects created while planning a 5-way join between partitioned tables with 1000 partitions each consumed 769.795 MiB as compared to 19.728 KiB consumed by Bitmapsets when planning a 5-way non-partitioned join. See slide 3 for detailed numbers. > Considering my recent thread about scalability, where malloc() turned > out to be one of the main culprits, I wonder if maybe there's a lot to > gain by reducing the memory usage ... Our attitude to memory usage is > that it doesn't really matter if we keep it allocated for a bit, because > we'll free it shortly. And that may be true for "modest" memory usage, > but with 1.4GB that doesn't seem great, and the malloc overhead can be > pretty bad. > That probably explains why we see some modest speedups because of my memory saving patches. Thanks for sharing it. [1] https://www.postgresql.org/message-id/caexhw5stmouobe55pmt83r8uxvfcph+pvo5dnpdrvcsbgxe...@mail.gmail.com [2] https://www.postgresql.org/message-id/CAExHW5t0cPNjJRPRtt%3D%2Bc5SiTeBPHvx%3DSd2n8EO%2B7XdVuE8_YQ%40mail.gmail.com -- Best Wishes, Ashutosh Bapat