CAddress inherits CService which is 28 bytes (on 64 bit machines).
CAddress then adds two member variables - one that requires 4 byte
alignment (nTime) and one that requires 8 byte alignment
(nServices).
Declare the smaller one first so that it fits in bytes 29..32.
On 32 bit machines this change has no effect and CAddress remains 40
bytes.
`CAddress` inherits `CService` which is 28 bytes (on 64 bit machines).
`CAddress` then adds two member variables - one that requires 4 byte
alignment (`nTime`) and one that requires 8 byte alignment
(`nServices`).
Declare the smaller one first so that it fits in bytes 29..32.
On 32 bit machines this change has no effect and `CAddress` remains 40
bytes.
CAddress on all Travis platforms - pushed a patch that prints the sizes of CAddress before and after the change. One result, all results
I guess we store enough addresses in e.g. addrman for this to matter substantially?
I guess this will reduce the memory footprint by a few 100s of KiB. I am not sure if this qualifies for “substantially” but the change is simple enough and not risky.
Might want to add a comment on why this specific ordering of fields is used.
Isn’t that too low level and basic to be mentioned? Like we don’t comment every ++it why it was preferred over it++. Maybe also other structs would warrant such comments? I think that every time the order of data members in a struct is considered, padding should be taken into account. But anyway, this is subjective, I will add a comment about it if you feel it is needed.
Edit: How does this interact with #19628?
No conflicts, either one can go first without disturbing the other one. Size reduced from 56 to 48 bytes.
Isn’t that too low level and basic to be mentioned? Like we don’t comment every ++it why it was preferred over it++. Maybe also other structs would warrant such comments? I think that every time the order of data members in a struct is considered, padding should be taken into account. But anyway, this is subjective, I will add a comment about it if you feel it is needed.
I agree in that case. But I wouldn’t exactly say this is obvious. Normally to create compact structures the preferred ordering is to have larger alignment fields first, then smaller one. The reason it’s different here is because of the inheritance.
No conflicts, either one can go first without disturbing the other one. Size reduced from 56 to 48 bytes.
Thanks!
Nice little memory-saving change. Verified that both on master branch and PR #19628 putting nServices after nTime in class CAddress reduces its size by 8 bytes. Running a x86_64 machine with gcc 5.4.0 here.
ACK https://github.com/bitcoin/bitcoin/commit/767073fb9645f5cb0976a14288c03fe71912b569
Is this really x86-64 specific, doesn’t it reduce CAddress size on other 64-bit archs, like aarch64 or riscv64, too?
I’d expect this is the same for every architecture that requires 64-bit values to be aligned to 64-bit.
So practically for every 64-bit architecture.
rv64:
ACK 767073fb9645f5cb0976a14288c03fe71912b569