simplify bitarray API to assume arrays are only resized at construction time

The bit array API was written to allow arbitrary growing or shrinking of an
array after its construction, while maintaining its content. After fully rolling
it out, it turns out Graphviz only ever creates such arrays once at a fixed size
and then uses them until free. Carrying a more generic API than was necessary
had a number of negative consequences:

  1. `bitarray_resize` was more complicated than it needed to be, in order to
     cope with an arbitrary initial size of the input array.

  2. `bitarray_resize` used `realloc;memset` instead of the more efficient
     `calloc` because it was assuming the caller needed to maintain the original
     content.

  3. `bitarray_resize` performed a “loose” allocation with the capacity of the
     backing buffer larger than the size of the array itself, assuming that
     amortizing allocation costs across repeated resizes was a relevant concern.
     Dropping this and making the allocation “tight” not only reduces heap
     pressure, but allows dropping the `.capacity` member.

  4. `bitarray_array_resize_or_exit` had a more awkward calling convention than
     necessary, as exemplified by how it is streamlined in this commit.

As such, this change not only simplifies the code but also reduces memory usage.

diff --git a/lib/cgraph/bitarray.h b/lib/cgraph/bitarray.h
index aa584538fdf94e77a97915d48fa07f755e512b13..109d8e172ddd7c1a1925ca6aab4b2a40918c15bb 100644 (file)
--- a/lib/cgraph/bitarray.h
+++ b/lib/cgraph/bitarray.h
@@ -34,52 +34,37 @@
 /// these to zero gives you a valid zero-length bit array.
 typedef struct {
   uint8_t *base; ///< start of the underlying allocated buffer
-  size_t size_bits; ///< used extent in bits
-  size_t capacity; ///< allocated extent in bytes
+  size_t size_bits; ///< extent in bits
 } bitarray_t;
 
-/// increase or decrease the element length of this array
-static inline int bitarray_resize(bitarray_t *self, size_t size_bits) {
+/// create an array of the given element length
+static inline int bitarray_new(bitarray_t *self, size_t size_bits) {
   assert(self != NULL);
+  assert(self->size_bits == 0);
 
-  // if this is less than the current size, just forget the excess elements
-  if (size_bits <= self->size_bits) {
-    self->size_bits = size_bits;
-    return 0;
-  }
-
-  // do we need to expand the backing buffer?
-  if (self->capacity * 8 < self->size_bits) {
-
-    size_t capacity = self->capacity == 0 ? 128 : self->capacity * 2;
-    if (capacity * 8 < self->size_bits)
-      capacity = self->size_bits / 8 + (self->size_bits % 8 == 0 ? 0 : 1);
-
-    uint8_t *base = realloc(self->base, capacity);
-    if (UNLIKELY(base == NULL))
-      return ENOMEM;
-
-    // clear the newly allocated bytes
-    memset(&base[self->capacity], 0, capacity - self->capacity);
-
-    self->base = base;
-    self->capacity = capacity;
-  }
+  size_t capacity = size_bits / 8 + (size_bits % 8 == 0 ? 0 : 1);
+  uint8_t *base = calloc(capacity, sizeof(self->base[0]));
+  if (UNLIKELY(base == NULL))
+    return ENOMEM;
 
+  self->base = base;
   self->size_bits = size_bits;
 
   return 0;
 }
 
-/// `bitarray_resize` for callers who cannot handle failure
-static inline void bitarray_resize_or_exit(bitarray_t *self, size_t size_bits) {
-  assert(self != NULL);
+/// `bitarray_new` for callers who cannot handle failure
+static inline bitarray_t bitarray_new_or_exit(size_t size_bits) {
 
-  int error = bitarray_resize(self, size_bits);
+  bitarray_t ba = {0};
+
+  int error = bitarray_new(&ba, size_bits);
   if (UNLIKELY(error != 0)) {
     fprintf(stderr, "out of memory\n");
     exit(EXIT_FAILURE);
   }
+
+  return ba;
 }
 
 /// get the value of the given element

diff --git a/lib/neatogen/dijkstra.c b/lib/neatogen/dijkstra.c
index 404024a2ede4edfb69902b517492dcc103bac03c..05192c97e66423f8d1e8f89916e1ca934791d9a1 100644 (file)
--- a/lib/neatogen/dijkstra.c
+++ b/lib/neatogen/dijkstra.c
@@ -199,8 +199,7 @@ dijkstra_bounded(int vertex, vtx_data * graph, int n, DistType * dist,
     }
     num_visited_nodes =
        bfs_bounded(vertex, graph, n, dist, &Q, bound, visited_nodes);
-    bitarray_t node_in_neighborhood = {0};
-    bitarray_resize_or_exit(&node_in_neighborhood, n);
+    bitarray_t node_in_neighborhood = bitarray_new_or_exit(n);
     for (i = 0; i < num_visited_nodes; i++) {
        bitarray_set(node_in_neighborhood, visited_nodes[i], true);
     }

diff --git a/lib/neatogen/neatoinit.c b/lib/neatogen/neatoinit.c
index 3c7bf59b8e5eb3707c93134f53f3c5fa08278c73..e56a43b8f6ad89c53d4d39b23e3ebd244120a9eb 100644 (file)
--- a/lib/neatogen/neatoinit.c
+++ b/lib/neatogen/neatoinit.c
@@ -189,8 +189,7 @@ static cluster_data* cluster_map(graph_t *mastergraph, graph_t *g)
      /* array of arrays of node indices in each cluster */
     int **cs,*cn;
     int i,j,nclusters=0;
-    bitarray_t assigned = {0};
-    bitarray_resize_or_exit(&assigned, agnnodes(g));
+    bitarray_t assigned = bitarray_new_or_exit(agnnodes(g));
     cluster_data *cdata = GNEW(cluster_data);
 
     cdata->ntoplevel = agnnodes(g);

diff --git a/lib/neatogen/sgd.c b/lib/neatogen/sgd.c
index ff30427e86bf95386ecd1a4d2db77d10988855fb..b109923519322b00c65ffe0e7321f2d025a41cd2 100644 (file)
--- a/lib/neatogen/sgd.c
+++ b/lib/neatogen/sgd.c
@@ -52,7 +52,7 @@ static graph_sgd * extract_adjacency(graph_t *G, int model) {
     }
     graph_sgd *graph = N_NEW(1, graph_sgd);
     graph->sources = N_NEW(n_nodes + 1, size_t);
-    bitarray_resize_or_exit(&graph->pinneds, n_nodes);
+    graph->pinneds = bitarray_new_or_exit(n_nodes);
     graph->targets = N_NEW(n_edges, size_t);
     graph->weights = N_NEW(n_edges, float);
 
@@ -86,11 +86,9 @@ static graph_sgd * extract_adjacency(graph_t *G, int model) {
         // do nothing
     } else if (model == MODEL_SUBSET) {
         // i,j,k refer to actual node indices, while x,y refer to edge indices in graph->targets
-        bitarray_t neighbours_i = {0};
-        bitarray_t neighbours_j = {0};
         // initialise to no neighbours
-        bitarray_resize_or_exit(&neighbours_i, graph->n);
-        bitarray_resize_or_exit(&neighbours_j, graph->n);
+        bitarray_t neighbours_i = bitarray_new_or_exit(graph->n);
+        bitarray_t neighbours_j = bitarray_new_or_exit(graph->n);
         for (size_t i = 0; i < graph->n; i++) {
             int deg_i = 0;
             for (size_t x = graph->sources[i]; x < graph->sources[i + 1]; x++) {