Reducing relcache memory usage 2: shrink sizeof(RelationData)

On Thu, 10 Sept 2026 at 11:43, David Geier <geidav.pg@gmail.com> wrote:

To further reduce relcache memory usage we can additionally shrink the
size consumed by struct RelationData:

[...]

All RelationData objects are allocated in CacheMemoryContext. The size
of CacheMemoryContext can be inspected via pg_backend_memory_contexts:

branch | used_bytes
--------|---------------------------------
master | 617,984,696 bytes = ~589.36 MiB
patched | 562,507,232 bytes = ~536.45 MiB

We save ~53 MiB or ~10% of CacheMemoryContext per backend!

That's quite nice!

The attached patch set passes tests and consists of the following
individual patches:

[0001-0003]
I Haven't looked at these in detail. Yes, they provide the largest
savings, but that also makes them most complicated to review in
detail. I'll stave that off for now.

- 0004: Removes the rd_lockinfo member. RelationGetLockRelId() now
computes it when needed. This also removes RelationInitLockInfo() and
the initialization work associated with it.
=> sizeof(RelationData) == 304 bytes

Can we really rely on rd_rel always being valid when we need the
LockInfo? I'm OK with avoiding duplicating
rd_id/rd_lockinfo.lockRelId.relId, but I'm not sure dbId can always be
derived with rd_rel->relisshared whenever we need it.

- 0005: Replaces the embedded partition key, descriptors, partition
qual, validity flag, and memory contexts with one lazily allocated
RelationPartitionInfo pointer.
=> sizeof(RelationData) == 264 bytes

I haven't worked on partitioning, so I'm not fully confident that this
has sufficiently low additional overhead to be worth applying.

- 0006: Removes rd_fkeyvalid by using RELCACHE_FKEYLIST_NOT_LOADED as
the initial state of rd_fkeylist. NIL continues to mean that the list
was computed and no foreign keys were found. This patch is not strictly
needed because it currently doesn't further reduce the size. The same we
could with RelationPartitionInfo::partcheckvalid.
=> sizeof(RelationData) == 264 bytes

I'm not a fan of this change. Sentinel values *can* have their place,
but I really don't like non-NULL values being used to signal "invalid"
states, and seeing that it doesn't actually save any bytes I'd prefer
to not add this complication.

- 0007: Removes rd_index member that always pointed inside
rd_indextuple. Callers now use RelationGetIndex(), which applies
GETSTRUCT() to that tuple.
=> sizeof(RelationData) == 256 bytes

This seems fine to me.

- 0008: Adds a static assertion that sizeof(RelationData) <= 256.
=> sizeof(RelationData) == 256 bytes

I'm not a fan of this, because we generally don't add assertions on
struct sizes unless it's critically important a struct remains the
asserted size (or, in this case, doesn't become larger than that).
In this case, I don't think it's critically important that the struct
fits in the 256-byte aset bucket, given that relcache memory usage has
never been capped, and a slab context would similarly do the trick for
avoiding memory usage cliffs when the size of the struct is increased.

Note that the necessity to arrive at 256 bytes stems from ASET's
allocation granularity being powers of two. To profit from further size
reductions of RelationData, we would need to use a SLAB memory context
because it's unlikely that we'll get to 128 bytes.

Let's start with palloc-ing all RelationData into Slab contexts (like
attached, tagged with .nocfbot to avoid your patch's CI). This allows
us to start saving bytes immediately, and immediately improves memory
usage for every reduction in size so that the merits of each later
patch can be evaluated separately. This will be very useful if we
can't agree on some of the changes that are necessary to reduce the
size down to the next smaller aset size bucket.

Kind regards,

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

Hi Matthias,

Thanks for the review.

[0001-0003]
I Haven't looked at these in detail. Yes, they provide the largest
savings, but that also makes them most complicated to review in
detail. I'll stave that off for now.

While the diff is relatively big, the changes are quite mechanical. The
biggest challenge is knowing that all code has been adapted. Apart from
grepping, I relied on the tests for that.

- 0004: Removes the rd_lockinfo member. RelationGetLockRelId() now
computes it when needed. This also removes RelationInitLockInfo() and
the initialization work associated with it.
=> sizeof(RelationData) == 304 bytes

Can we really rely on rd_rel always being valid when we need the
LockInfo? I'm OK with avoiding duplicating
rd_id/rd_lockinfo.lockRelId.relId, but I'm not sure dbId can always be
derived with rd_rel->relisshared whenever we need it.

The information is derived exactly the same way, just deferred to when
it's needed. The old RelationInitLockInfo() did:

if (relation->rd_rel->relisshared)
relation->rd_lockInfo.lockRelId.dbId = InvalidOid;
else
relation->rd_lockInfo.lockRelId.dbId = MyDatabaseId;

Beyond that, every Relation that can reach RelationGetLockRelId() has
rd_rel populated. There's no code that constructs a Relation with rd_rel
== NULL and then locks it.

There's also one subtle improvement: the deferring version is more
correct than before in the load_relcache_init_file() case. The old code
had to recompute lockRelId after loading pg_internal.init because the
file might have been copied from another database by CREATE DATABASE —
baking in the wrong dbId. Computing MyDatabaseId on the fly makes that
recomputation unnecessary.

- 0005: Replaces the embedded partition key, descriptors, partition
qual, validity flag, and memory contexts with one lazily allocated
RelationPartitionInfo pointer.
=> sizeof(RelationData) == 264 bytes

I haven't worked on partitioning, so I'm not fully confident that this
has sufficiently low additional overhead to be worth applying.

I'll run a partitioned pgbench and report if I could measure any
regressions.

- 0006: Removes rd_fkeyvalid by using RELCACHE_FKEYLIST_NOT_LOADED as
the initial state of rd_fkeylist. NIL continues to mean that the list
was computed and no foreign keys were found. This patch is not strictly
needed because it currently doesn't further reduce the size. The same we
could with RelationPartitionInfo::partcheckvalid.
=> sizeof(RelationData) == 264 bytes

I'm not a fan of this change. Sentinel values *can* have their place,
but I really don't like non-NULL values being used to signal "invalid"
states, and seeing that it doesn't actually save any bytes I'd prefer
to not add this complication.

The pointer itself has no invalid state. NULL is also a valid state.
This is why the extra flag is needed.

I'm in slight favor of keeping it because "is valid" booleans always
have the tendency to cause padding and I'm hopeful that we can further
strip down the struct so that the change is actually yielding a real saving.

I'm keeping the commit in the patch set for now, waiting for other
reviewers to chime in.

- 0008: Adds a static assertion that sizeof(RelationData) <= 256.
=> sizeof(RelationData) == 256 bytes

I'm not a fan of this, because we generally don't add assertions on
struct sizes unless it's critically important a struct remains the
asserted size (or, in this case, doesn't become larger than that).
In this case, I don't think it's critically important that the struct
fits in the 256-byte aset bucket, given that relcache memory usage has
never been capped, and a slab context would similarly do the trick for
avoiding memory usage cliffs when the size of the struct is increased.

Note that the necessity to arrive at 256 bytes stems from ASET's
allocation granularity being powers of two. To profit from further size
reductions of RelationData, we would need to use a SLAB memory context
because it's unlikely that we'll get to 128 bytes.

I've removed the patch because of changing to a slab memory context. See
comment below.

Let's start with palloc-ing all RelationData into Slab contexts (like
attached, tagged with .nocfbot to avoid your patch's CI). This allows
us to start saving bytes immediately, and immediately improves memory
usage for every reduction in size so that the merits of each later
patch can be evaluated separately. This will be very useful if we
can't agree on some of the changes that are necessary to reduce the
size down to the next smaller aset size bucket.

Attached is patch 0002 that allocates all RelationData objects inside a
slab memory context. It's passing all tests together with all the other
patches.

I've also attached patch 0001 that removes two unnecessary checks for if
CacheMemoryContext is already initialized. This is no longer necessary
since #1aebc3618a0, #46287bd6602 and #d4d1885e42e. I wanted to get rid
of these checks so that I don't have to create the slab context in these
places as well.

Attached is the latest patch set rebased also on latest master. While
looking more closely at RelationData, I've realized that we can also
move rd_indexlist, rd_statlist, rd_pkindex, rd_toastoid and
rd_toastchunkidtype to the table union leg. As the index union leg is
currently bigger, that should yield some more savings. I'll give this a
try the next days.

--
David Geier

- 0005: Replaces the embedded partition key, descriptors, partition
qual, validity flag, and memory contexts with one lazily allocated
RelationPartitionInfo pointer.
=> sizeof(RelationData) == 264 bytes

Since patch set v2, this happens in patch 0006. All patches moved by 1
because I added a new patch 0001 (use SLAB context for all RelationData
objects).

I haven't worked on partitioning, so I'm not fully confident that this
has sufficiently low additional overhead to be worth applying.

I'll run a partitioned pgbench and report if I could measure any
regressions.

I ran pgbench with 1000 partitions and scale factor 1 to put maximum
pressure on planning which I assumed uses the relcache most:

pgbench -i -s 1 --partitions=1000 postgres
pgbench -c 1 -j 1 -T 60 -S -M simple postgres

I ran against master and my patch, while having turbo boost, frequency
scaling, CPU idle states, ASLR and hyper threading disabled. I used a
single client to further stabilize results. The runs are still a little
bit noisy but FWICS don't show a regression.

TPS master | TPS patched
------------+------------
46,633 | 46,488
45,560 | 46,437
46,445 | 45,866

I've also attached v3 which is v2 rebased on latest master. No other
changes.

--
David Geier

On Mon, 5 Oct 2026 at 13:48, David Geier <geidav.pg@gmail.com> wrote:

I've also attached v3 which is v2 rebased on latest master. No other
changes.

Some high-level comments:

0003-0008: These need more commit description than they currently have.

0001:
Could you replace the conditional initializations with Assert()s that
the context is initialized? That'll save any code that calls into
these functions too early.

0002: LGTM, though maybe an AllocRelationData() helper macro would
reduce the verbosity of each allocation, given that we're always
allocating into that one context.

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)

+++ relcache.c

I think this'll be better if it splits out the Table and Index parts
from RelationBuildDesc.

+/*
+ * Field order within RelationData (and within each branch of the union
+ * further down) is chosen to minimize struct padding: pointer-sized
+ * (8-byte) fields are grouped first, followed by 4-byte fields, followed
+ * by 1-byte bool fields.  Don't reorder fields without re-checking that
+ * this packing is preserved.
+ */

I don't think this packing is necessarily better than "just" making
sure we order our fields in a way that introduces the fewest padding
bytes possible. Also note that the patch doesn't actually group all
fields in that order, so the comment is already outdated.

0005:

The following change snippet:

+ * RelationGetLockRelId() derives the LockRelId to use for locking the
+ * relation from rd_id and rd_rel->relisshared, in case anything tries
+ * to lock the dummy relation.  We don't have a real OID for this fake
+ * entry, so use relNumber instead.  Note that this is fairly bogus
+ * since relNumber may be different from the relation's real OID.  It
+ * shouldn't really matter though.  In recovery, we are running by
+ * ourselves and can't have any lock conflicts.  While syncing, we
+ * already hold AccessExclusiveLock.  Set relisshared to match rlocator
+ * so that RelationGetLockRelId() derives the correct dbId for shared
+ * relations (InvalidOid); for non-shared relations it'll derive
+ * MyDatabaseId, which is correct whenever this is called from a real
+ * backend (e.g. while syncing) and simply irrelevant during recovery.
*/
- rel->rd_lockInfo.lockRelId.dbId = rlocator.dbOid;
- rel->rd_lockInfo.lockRelId.relId = rlocator.relNumber;
+ rel->rd_id = rlocator.relNumber;
+ rel->rd_rel->relisshared = (rlocator.spcOid == GLOBALTABLESPACE_OID);

Shouldn't this test on rlocator.dbOid, or Assert() that the dbOid is
invalid when spcOid == GLOBALTABLESPACE_OID? Asking, because the lock
code uses dbOid.

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.

0007: -1 on this patch; I don't think the cost/benefit of this change
in ergonomics is worth it.

0008: -1 on this patch, too: I don't think the byte savings are worth
these changes; especially given that this adds additional indirections
with more data dependencies on common paths.

It also adds a 2k line propgraphcmds.c without adding it to compile
targets for it in Makefile nor meson.build, was this a typo?

Kind regards,

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

On Tue, Oct 13, 2026 at 11:09 AM Matthias van de Meent
<boekewurm+postgres@gmail.com> wrote:

On Mon, 5 Oct 2026 at 13:48, David Geier <geidav.pg@gmail.com> wrote:

I've also attached v3 which is v2 rebased on latest master. No other
changes.

Some high-level comments:

0003-0008: These need more commit description than they currently have.

0001:
Could you replace the conditional initializations with Assert()s that
the context is initialized? That'll save any code that calls into
these functions too early.

0002: LGTM, though maybe an AllocRelationData() helper macro would
reduce the verbosity of each allocation, given that we're always
allocating into that one context.

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)

+++ relcache.c

I think this'll be better if it splits out the Table and Index parts
from RelationBuildDesc.

+/*
+ * Field order within RelationData (and within each branch of the union
+ * further down) is chosen to minimize struct padding: pointer-sized
+ * (8-byte) fields are grouped first, followed by 4-byte fields, followed
+ * by 1-byte bool fields.  Don't reorder fields without re-checking that
+ * this packing is preserved.
+ */

I don't think this packing is necessarily better than "just" making
sure we order our fields in a way that introduces the fewest padding
bytes possible. Also note that the patch doesn't actually group all
fields in that order, so the comment is already outdated.

0005:

The following change snippet:

+ * RelationGetLockRelId() derives the LockRelId to use for locking the
+ * relation from rd_id and rd_rel->relisshared, in case anything tries
+ * to lock the dummy relation.  We don't have a real OID for this fake
+ * entry, so use relNumber instead.  Note that this is fairly bogus
+ * since relNumber may be different from the relation's real OID.  It
+ * shouldn't really matter though.  In recovery, we are running by
+ * ourselves and can't have any lock conflicts.  While syncing, we
+ * already hold AccessExclusiveLock.  Set relisshared to match rlocator
+ * so that RelationGetLockRelId() derives the correct dbId for shared
+ * relations (InvalidOid); for non-shared relations it'll derive
+ * MyDatabaseId, which is correct whenever this is called from a real
+ * backend (e.g. while syncing) and simply irrelevant during recovery.
*/
- rel->rd_lockInfo.lockRelId.dbId = rlocator.dbOid;
- rel->rd_lockInfo.lockRelId.relId = rlocator.relNumber;
+ rel->rd_id = rlocator.relNumber;
+ rel->rd_rel->relisshared = (rlocator.spcOid == GLOBALTABLESPACE_OID);

Shouldn't this test on rlocator.dbOid, or Assert() that the dbOid is
invalid when spcOid == GLOBALTABLESPACE_OID? Asking, because the lock
code uses dbOid.

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.

0007: -1 on this patch; I don't think the cost/benefit of this change
in ergonomics is worth it.

0008: -1 on this patch, too: I don't think the byte savings are worth
these changes; especially given that this adds additional indirections
with more data dependencies on common paths.

It also adds a 2k line propgraphcmds.c without adding it to compile
targets for it in Makefile nor meson.build, was this a typo?

Kind regards,

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

Hi David,
I tested v3 independently on Linux x86_64, based on 9dbc53591a2. All
eight attachments apply there without context changes. I concentrated
on v3 0001-0006, following the cost/benefit concerns already raised by
Matthias about 0007 and 0008.
I found a predicate-locking crash introduced by the union in 0004. The
predicate code uses rd_index (and, with 0008, RelationGetIndex()) to
decide whether a Relation is an index. For a rule-bearing table,
rd_rules occupies the same union storage, so that test can interpret a
RuleLock as index metadata.
This reproduces with 0001-0006; 0008 is not required. A minimal case is:

CREATE TABLE t (a int PRIMARY KEY);
CREATE RULE r AS ON INSERT TO t DO ALSO SELECT 1;
BEGIN ISOLATION LEVEL SERIALIZABLE;
INSERT INTO t VALUES (1);
TRUNCATE t;
In a cassert build, TRUNCATE aborts at
CheckTableForSerializableConflictIn(). At that point relkind is 'r',
and rd_rules and the overlaid rd_index contain the same non-NULL address.
The server process exits with SIGABRT. The unpatched baseline passes the
same pg_regress test.
The first attached patch adds RelationIsIndex(), based on relkind, and
uses it at the three predicate-locking classification sites. The second
adds the pg_regress test. Both ordinary and partitioned indexes remain
covered by the predicate.

I also reproduced an ERROR-path problem from 0006 using an injection
point at its new RelationPartitionInfo allocation inside
RelationRebuildRelation(). After an ALTER TABLE added a column to a
partitioned table, the injected ERROR occurred after the descriptor swap.
Rollback restored the two-column catalog state, but the same backend kept
the three-column descriptor and then reported:

could not find inherited attribute "key_added"

An equivalent general swap window exists without this series. The v3
regression is specifically that 0006 adds an allocation failure point to
the region. I did not observe or claim a real OOM.

The third patch allocates destination partition metadata before the swap
and before reparenting a completed partition-key context. It also avoids
reading table-side partition metadata for index relations. With the
allocation moved before the swap, an ERROR leaves the old descriptor intact
and marked invalid; the next lookup rebuilds it normally after rollback.
Ten repeated subtransaction failures, partitioned indexes, and
debug_discard_caches=1 also passed without an external invalidation.

Finally, the init-file loader reconstructs index access fields but does
not reset rd_indexprs, rd_indpred, rd_exclops, rd_exclprocs, or
rd_exclstrats. Those are serialized process pointers whose contents are
supposed to be initialized lazily. The fourth patch resets the five
fields without changing the init-file format. I did not find a reachable
failure with current catalog indexes, so I regard this as an
initialization correctness hazard rather than a demonstrated SQL crash.

On Linux, sizeof(RelationData) is 496 bytes at the baseline, 272 through
0006 and 0007, and 264 through 0008. Under the same 8 KiB Slab setup,
0006 and 0007 both used 302 blocks and 2,474,160 total bytes for a workload
loading 8,731 relations. This supports the limited-benefit point already
made in the review discussion.

One additional issue: original v3 0008 does not build on this base because
execReplication.c still has two rd_index references. I did not repair it
silently or use that configuration for runtime results.

Validation was on Linux 6.17 x86_64 with GCC 13.3 and Clang 18.1. GCC
cassert and release check-world passed (240 core regression and 133
isolation tests). Clang ASan/UBSan core and isolation suites also passed.

Attachments:

0001: classify indexes by relkind in predicate locking
0002: add the rule-bearing SERIALIZABLE regression test
0003: keep partition-info allocations outside descriptor swaps
0004: reset lazy index pointers after init-file loading

Regards,
Haibo