refactor : Reorder CBlockIndex members to save 8 bytes #36501

bartoli commented at 4:07 PM on October 11, 2026: none

It seems we can save 8 bytes per CBlockIndex instance by reordering one class member. Moving one 8-byte field seems to avoid losing 2x 4 bytes to structure padding on a 64-bit build (on a 32-bit build, there is no gain).

I could not exactly measure the runtime gain from just this (smaller than RAM usage variation between 2 launches), but I confirmed there was about as many instances of CBlockIndex as blocks in the chain (increasing CBlockIndex size by 320 bytes increases RAM usage by ~300MB), so saving 8 bytes per instance should save about 8MB of RAM at current block height.

The gain is small, but it seems like it has no cost/risk, since that structure is not used for serialization, and just reordering here should have no side effects.

I saw a few other types that could gain a few bytes in size by doing something similar (for example, CTxIn), but they have far fewer instances in memory to make the change worth it.

Reorder CBlockIndex members to save 8 bytes

On a windows x64 build, CBlockIndex size is 152 bytes. Moving one field
somewhere else makes the class lose less bytes due to padding to respect
the 8-byte alignment

For a ~1M block height, this saves about 8M of Ram without doing
anything else.

d9387c41c1

DrahtBot commented at 4:07 PM on October 11, 2026: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

External sites

Code coverage & benchmarks: https://corecheck.dev/bitcoin/bitcoin/pulls/36501
Ralph review <img src=https://review.fish.foo/assets/bitcoin/bitcoin/36501.png alt="Ralph review status"> https://git.fish.foo/bitcoin/bitcoin/pulls/36501#author-ralph

Reviews

See the guideline and AI policy for information on the review process.

Type	Reviewers
ACK	l0rinc, maflcko

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

l0rinc commented at 11:10 PM on October 11, 2026: contributor

Godbolt indicates 152 -> 144 bytes on 64-bit (with no change on 32-bit, as mentioned in the PR description). I wouldn't claim a total RAM saving though, as it seems allocator-dependent: glibc gives the map nodes 200 bytes in both cases. But I don't see a downside to the swap, since the fields initialize independently and serialization is unchanged.

ACK d9387c41c1c2076ce035aeb6a3daf7bfd83f7c32

maflcko commented at 11:03 AM on October 12, 2026: member

lgtm ACK d9387c41c1c2076ce035aeb6a3daf7bfd83f7c32

stickies-v commented at 9:59 AM on October 13, 2026: contributor

it seems like it has no cost/risk, since that structure is not used for serialization, and just reordering here should have no side effects.

I disagree with the risk assessment. Also, CDiskBlockIndex derives from CBlockIndex and is serialized. The change is safe because serialization is field by name, not because it does not happen.

CBlockIndex is used everywhere, any reliance on its memory layout can yield a bug, possibly a consensus one. Especially for a refactor change like this, I think the onus is on the PR author to exhaustively list why this change is safe, and how reviewers can verify that. For example, some ways the reorder can cause bugs:

positional / aggregate init
initializer-order dependencies
defaulted comparison operators
serialization
byte-level access: memcpy, reinterpret_cast, etc

I checked these myself, including clang-query over all translation units for casts, mem* calls and sizeof/offsetof on the type, and found only pointer printing and allocator internals. So it looks safe to me. But the PR should let reviewers falsify its assumptions rather than assert "no side effects".

bartoli commented at 2:14 PM on October 13, 2026: none

it seems like it has no cost/risk, since that structure is not used for serialization, and just reordering here should have no side effects.

I disagree with the risk assessment. Also, CDiskBlockIndex derives from CBlockIndex and is serialized. The change is safe because serialization is field by name, not because it does not happen.

CBlockIndex is used everywhere, any reliance on its memory layout can yield a bug, possibly a consensus one. Especially for a refactor change like this, I think the onus is on the PR author to exhaustively list why this change is safe, and how reviewers can verify that. For example, some ways the reorder can cause bugs:

positional / aggregate init

initializer-order dependencies

defaulted comparison operators

serialization

byte-level access: memcpy, reinterpret_cast, etc

I checked these myself, including clang-query over all translation units for casts, mem* calls and sizeof/offsetof on the type, and found only pointer printing and allocator internals. So it looks safe to me. But the PR should let reviewers falsify its assumptions rather than assert "no side effects".

My rationale was that the type is not serialized directly, and it also changes size between built platforms, so type serialization to bytes in memory has little chance to cause issues outside of that binary if they don't already do. But you are definately right about more needed checks on accesses inside the node process.