Pack the frequently allocated structs

[jiridanek]

Before

Reading symbols from router/skrouterd...
(gdb) pahole qdr_connection_work_t
/*   56     */ struct qdr_connection_work_t {
/*   0    8 */     struct qdr_connection_work_t * prev
/*   8    8 */     struct qdr_connection_work_t * next
/*  16    4 */     enum {...} work_type
/* XXX    4 */ !!  __32_bit_padding__
/*  24    8 */     qdr_link_t * link
/*  32    8 */     qdr_terminus_t * source
/*  40    8 */     qdr_terminus_t * target
/*  48    4 */     enum {...} ssn_class
/* XXX    4 */ !!  __32_bit_padding__
               } qdr_connection_work_t

After

(gdb) pahole qdr_connection_work_t
/*   48     */ struct qdr_connection_work_t {
/*   0    8 */     struct qdr_connection_work_t * prev
/*   8    8 */     struct qdr_connection_work_t * next
/*  16    8 */     qdr_link_t * link
/*  24    8 */     qdr_terminus_t * source
/*  32    8 */     qdr_terminus_t * target
/*  40    4 */     enum {...} work_type
/*  44    4 */     enum {...} ssn_class
               } qdr_connection_work_t

The qdr_connection_work_t has gotten 8 bytes smaller after reordering, when the work_type enum is declared after all the pointers. This is because the compiler was able to avoid having to add padding. This results in a reduction of total space taken by the instances of this struct as qdrouter runs by 14%.

Obviously, it only makes sense to do for structs that are allocated frequently, where the savings can be meaningful and justify the manual effort required. (Rust reorders structs automatically, the article linked below says.)

Considerations is that fight against padding are that related struct field should be declared together for source code readability, and that some fields should or should not share a cacheline due to multithreading concerns. Meaning that deciding whether to pack or not is a tradeoff.

Ensuring that padding is not added back as the code is eddited is... unresolved question for me at this point.

Theory and Tooling

I have all my info from http://www.catb.org/esr/structure-packing/

There is a package pahole in Fedora, but it crashes reading skrouterd binary. The pahole gdb script which works for me is from https://github.com/PhilArmstrong/pahole-gdb

Clang has a -Wpadded option that reports every time compiler added padding.

[jiridanek]

Btw, the struct from example above was pretty much the only struct that was not tightly packed. Other considerations for laying out structs are of course false sharing and readability of the code. (As I've already mentioned above.)