On Fri, 28 Jul 2023 at 02:06, Ashutosh Bapat
<ashutosh.bapat.oss@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