Re: Reducing relcache memory usage 2: shrink sizeof(RelationData)

On Fri, 16 Oct 2026 at 15:02, David Geier <geidav(dot)pg(at)gmail(dot)com> wrote:
> > 0004:
> > I think this needs to introduce a field at the top level of
> > RelationData that indicates the variant of Relation that we're dealing
> > with. Continuously dereferencing rel->rd_rel->relkind seems rather
> > expensive compared to "just" keeping track of another boolean (or
> > enum, char, or whatever type) at the front of the struct that
> > distinguishes the exact type of data contained, if we can afford to.
> >
> > (It'll also make it easier to verify we're dealing with the right type
> > of Relation, as we won't have to Assert() two layers of indirection
> > deep)
>
> I would prefer to not do that. I like that now we have a single source
> of truth and adding such a field would be a bunch of code churn.

What specifically would need a bunch of code churn? The paths we're
dealing with shouldn't need much (if any) changes in that regard. One
reason to do this is to make it simpler to add another variant to the
union, if/when we figure out how to split the larger of [table-like,
index-like] relations further.

> Performance-wise the extra indirection doesn't matter. It's not accessed
> anywhere in way that it's expensive relative to all the other stuff
> happening. The pgbench I ran support that claim.

Sure, but for debugging and for interpretability I do think that it
makes more sense to have an indicator which of the two variants
should/can be accessed that is stored in the Relation itself. It'll
also simplify the condition (from two OR-ed options to just one) and
will make adding future variants (if any) simpler by allowing using
switch() blocks when we need to handle generic Relations.

> > I think this'll be better if it splits out the Table and Index parts> from RelationBuildDesc.
>
> I'm not sure what you're exactly after here. Can you share some more
> details please?

I meant that the code of RelationBuildDesc would be easier to read and
understand if we split the code that fills in the shared fields from
the code that fills in the index variant of the union and from the
code that fills in the non-index parts of the union, instead of mixing
those. That doesn't need to be in a different function per se, but
could also just be a better vertical grouping with a singular if/else
block.

> > 0005:
> > 0006:
> >
> > In RelationGetPartitionDesc(), and various other functions, it may
> > make sense to store rd_partdesc in a local variable, it'll save line
> > width whenever we're frequently checking its data.
>
> rd_partdesc got moved into RelationPartitionInfo and is now called
> partdesc. I'm assuming you meant RelationData::rd_partinfo?

Yes

> I went through all places where it's used frequently and stored it in a
> local variable.

Thanks.

Kind regards,

Matthias van de Meent
Databricks (https://www.databricks.com)