On Fri, 28 Jul 2023 at 02:06, Ashutosh Bapat <ashutosh.bapat....@gmail.com> wrote: > Table 1: Join between unpartitioned tables > Number of tables | without patch | with patch | % reduction | > being joined | | | | > -------------------------------------------------------------- > 2 | 29.0 KiB | 28.9 KiB | 0.66% | > 3 | 79.1 KiB | 76.7 KiB | 3.03% | > 4 | 208.6 KiB | 198.2 KiB | 4.97% | > 5 | 561.6 KiB | 526.3 KiB | 6.28% |
I have mostly just random thoughts and some questions... Have you done any analysis of the node types that are consuming all that memory? Are Path and subclasses of Path really that dominant in this? The memory savings aren't that impressive and it makes me wonder if this is worth the trouble. What's the goal of the memory reduction work? If it's for performance, does this increase performance? Tracking refcounts of Paths isn't free. Why do your refcounts start at 1? This seems weird: + /* Free the path if none references it. */ + if (path->ref_count == 1) Does ref_count really need to be an int? There's a 16-bit hole you could use between param_info and parallel_aware. I doubt you'd need 32 bits of space for this. I agree that it would be nice to get rid of the "if (!IsA(old_path, IndexPath))" hack in add_path() and it seems like this idea could allow that. However, I think if we were to have something like this then you'd want all the logic to be neatly contained inside pathnode.c without adding any burden in other areas to track or check Path refcounts. I imagine the patch would be neater if you added some kind of Path traversal function akin to expression_tree_walker() that allows you to visit each subpath recursively and run a callback function on each. David
