sd-bus: drop bytefield annontations

It's the same old story: 'struct sd_bus' is generally instantiated once, so
bitfields, for which we pay with more complicated code in all users of this
struct, are counterproductive. In some progs the structure may be instantiated
a few times, but it's still not worth it because we save a few bytes of memory
in one place and pay for this with many more bytes in the code.

$ size build/libsystemd.so.0.39.0{.orig,}
   text	   data	    bss	    dec	    hex	filename
2452757	  65376	   3768	2521901	 267b2d	build/libsystemd.so.0.39.0.orig
2451669	  65376	   3768	2520813	 2676ed	build/libsystemd.so.0.39.0

$ diff -u <(pahole build/libsystemd.so.0.39.0.orig) <(pahole build/libsystemd.so.0.39.0)
...
-       /* size: 1960, cachelines: 31, members: 105 */
-       /* sum members: 1944, holes: 3, sum holes: 9 */
-       /* sum bitfield members: 25 bits, bit holes: 2, sum bit holes: 31 bits */
+       /* size: 1984, cachelines: 31, members: 105 */
+       /* sum members: 1971, holes: 4, sum holes: 13 */
        /* member types with holes: 1, total: 1 */

i.e. 2452757 - 2451669 = 1088 extra bytes of code and slower execution, to save
24 bytes of memory per instance of the struct. (But the number of cachelines
doesn't change, so the smaller struct most likely has no effect on memory
access, and the alignment of the struct most likely means that the memory
saving is illusory too, we just end up with a few bytes of padding after the
struct.)

In the other structs, the alignment prevent the bitfield for having any effect
on memory use, but the compiler would still generate more complicated code,
i.e. we pay something for nothing.

For example:

$ diff -u <(pahole build/libsystemd.so.0.39.0.orig) <(pahole build/libsystemd.so.0.39.0)
...
 struct node_callback {
        struct node *              node;                 /*     0     8 */
-       _Bool                      is_fallback:1;        /*     8: 0  1 */
+       _Bool                      is_fallback;          /*     8     1 */

-       /* XXX 7 bits hole, try to pack */
        /* XXX 3 bytes hole, try to pack */

        unsigned int               last_iteration;       /*    12     4 */
@@ -455,15 +448,13 @@
        struct node_callback *     callbacks_prev;       /*    32     8 */

        /* size: 40, cachelines: 1, members: 6 */
-       /* sum members: 36, holes: 1, sum holes: 3 */
-       /* sum bitfield members: 1 bits, bit holes: 1, sum bit holes: 7 bits */
+       /* sum members: 37, holes: 1, sum holes: 3 */
        /* last cacheline: 40 bytes */
 };

I kept the bitfield in sd_bus_slot because it prevents the struct from growing
from 112 to 120 bytes by reducing the alignment requirement for subsequent
fields, and we potentially can have this instantiated many times.

diff --git a/src/libsystemd/sd-bus/bus-internal.h b/src/libsystemd/sd-bus/bus-internal.h
index 0d99d2d654a80a73e76ad54b05e537277a32d227..5b3cae759b190d1e46ad50d11c8fbc97f9bf1bc8 100644 (file)
--- a/src/libsystemd/sd-bus/bus-internal.h
+++ b/src/libsystemd/sd-bus/bus-internal.h
@@ -69,7 +69,7 @@ struct node {
 struct node_callback {
         struct node *node;
 
-        bool is_fallback:1;
+        bool is_fallback;
         unsigned last_iteration;
 
         sd_bus_message_handler_t callback;
@@ -96,7 +96,7 @@ struct node_object_manager {
 struct node_vtable {
         struct node *node;
 
-        bool is_fallback:1;
+        bool is_fallback;
         unsigned last_iteration;
 
         char *interface;
@@ -190,33 +190,33 @@ struct sd_bus {
         int message_version;
         int message_endian;
 
-        bool can_fds:1;
-        bool bus_client:1;
-        bool ucred_valid:1;
-        bool is_server:1;
-        bool anonymous_auth:1;
-        bool prefer_readv:1;
-        bool prefer_writev:1;
-        bool match_callbacks_modified:1;
-        bool filter_callbacks_modified:1;
-        bool nodes_modified:1;
-        bool trusted:1;
-        bool manual_peer_interface:1;
-        bool allow_interactive_authorization:1;
-        bool exit_on_disconnect:1;
-        bool exited:1;
-        bool exit_triggered:1;
-        bool is_local:1;
-        bool watch_bind:1;
-        bool is_monitor:1;
-        bool accept_fd:1;
-        bool attach_timestamp:1;
-        bool connected_signal:1;
-        bool close_on_exit:1;
+        bool can_fds;
+        bool bus_client;
+        bool ucred_valid;
+        bool is_server;
+        bool anonymous_auth;
+        bool prefer_readv;
+        bool prefer_writev;
+        bool match_callbacks_modified;
+        bool filter_callbacks_modified;
+        bool nodes_modified;
+        bool trusted;
+        bool manual_peer_interface;
+        bool allow_interactive_authorization;
+        bool exit_on_disconnect;
+        bool exited;
+        bool exit_triggered;
+        bool is_local;
+        bool watch_bind;
+        bool is_monitor;
+        bool accept_fd;
+        bool attach_timestamp;
+        bool connected_signal;
+        bool close_on_exit;
 
         RuntimeScope runtime_scope;
 
-        signed int use_memfd:2;
+        int use_memfd;
 
         void *rbuffer;
         size_t rbuffer_size;

diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c
index 738afe0e94888f06a0b1d46cf861b28848eb3e40..2683959db57425a1c5519ae3d071974281b8537d 100644 (file)
--- a/src/libsystemd/sd-bus/sd-bus.c
+++ b/src/libsystemd/sd-bus/sd-bus.c
@@ -3673,7 +3673,7 @@ static int io_callback(sd_event_source *s, int fd, uint32_t revents, void *userd
         sd_bus *bus = ASSERT_PTR(userdata);
         int r;
 
-        /* Note that this is called both on input_fd, output_fd as well as inotify_fd events */
+        /* Note that this is called both on input_fd, output_fd, as well as inotify_fd events */
 
         r = sd_bus_process(bus, NULL);
         if (r < 0) {