cgraph agxbuf: expand usable inline storage

As indicated by the changes to the layout diagram in this diff, previously we
were essentially wasting a number of bytes (3 on x86, 7 on x86-64) on structure
padding. Through some rearrangement and packing, we can make these bytes usable
for the inline storage area.

The definition of the struct is not quite what you might expect to see. The
`located` field is in the first member of the union but is actually used
regardless of what mode an `agxbuf` is in (heap, stack, inline). This is
necessary to get the desired packing. It does not overlap with other struct
fields, though reading and writing a struct through different union members is
well defined in C99.

This optimization might seem a little niche. But `agxbuf` objects are
extensively used. Once inline `agxbuf` objects become possible (an upcoming
commit), this saves us heap allocations in numerous scenarios.

Gitlab: #2302

diff --git a/lib/cgraph/agxbuf.h b/lib/cgraph/agxbuf.h
index 6970add63dbd9ffd0a3b614b4c7aa57321168088..4c812d1a51934ecc34c29d770be5cfab61e87506 100644 (file)
--- a/lib/cgraph/agxbuf.h
+++ b/lib/cgraph/agxbuf.h
@@ -12,6 +12,7 @@
 
 #include <assert.h>
 #include <cgraph/alloc.h>
+#include <limits.h>
 #include <stdarg.h>
 #include <stdbool.h>
 #include <stddef.h>
@@ -33,11 +34,13 @@ typedef enum {
 ///
 /// This has the following layout assuming x86-64.
 ///
-///   ┌───────────────┬───────────────┬───────────────┬───────────────┐
-///   │      buf      │     size      │   capacity    │               │
-///   ├───────────────┴───────────────┴───────────────┤    located    │
-///   │                     store                     │               │
-///   └───────────────────────────────────────────────┴───────────────┘
+///                                                               located
+///                                                                  ↓
+///   ┌───────────────┬───────────────┬───────────────┬─────────────┬─┐
+///   │      buf      │     size      │   capacity    │   padding   │ │
+///   ├───────────────┴───────────────┴───────────────┴─────────────┼─┤
+///   │                             store                           │ │
+///   └─────────────────────────────────────────────────────────────┴─┘
 ///   0               8               16              24              32
 ///
 /// \p buf, \p size, and \p capacity are in use when \p located is
@@ -46,15 +49,17 @@ typedef enum {
 typedef struct {
   union {
     struct {
-      char *buf;       ///< start of buffer
-      size_t size;     ///< number of characters in the buffer
-      size_t capacity; ///< available bytes in the buffer
+      char *buf;                        ///< start of buffer
+      size_t size;                      ///< number of characters in the buffer
+      size_t capacity;                  ///< available bytes in the buffer
+      char padding[sizeof(size_t) - 1]; ///< unused; for alignment
+      unsigned char
+          located; ///< where does the backing memory for this buffer live?
     };
-    char store[sizeof(char *) +
-               sizeof(size_t) * 2]; ///< inline storage used when \p located is
-                                    ///< > \p AGXBUF_ON_STACK
+    char store[sizeof(char *) + sizeof(size_t) * 3 -
+               1]; ///< inline storage used when \p located is
+                   ///< > \p AGXBUF_ON_STACK
   };
-  size_t located; ///< where does the backing memory for this buffer live?
 } agxbuf;
 
 static inline bool agxbuf_is_inline(const agxbuf *xb) {
@@ -228,7 +233,8 @@ static inline PRINTF_LIKE(2, 3) int agxbprint(agxbuf *xb, const char *fmt,
   assert(result == (int)(size - 1) || result < 0);
   if (result > 0) {
     if (agxbuf_is_inline(xb)) {
-      xb->located += (size_t)result;
+      assert(result <= (int)UCHAR_MAX);
+      xb->located += (unsigned char)result;
       assert(agxblen(xb) <= sizeof(xb->store) && "agxbuf corruption");
     } else {
       xb->size += (size_t)result;
@@ -253,7 +259,8 @@ static inline size_t agxbput_n(agxbuf *xb, const char *s, size_t ssz) {
   size_t len = agxblen(xb);
   if (agxbuf_is_inline(xb)) {
     memcpy(&xb->store[len], s, ssz);
-    xb->located += ssz;
+    assert(ssz <= UCHAR_MAX);
+    xb->located += (unsigned char)ssz;
     assert(agxblen(xb) <= sizeof(xb->store) && "agxbuf corruption");
   } else {
     memcpy(&xb->buf[len], s, ssz);