neatogen: push queue allocation into 'bfs'
authorMatthew Fernandez <matthew.fernandez@gmail.com>
Thu, 22 Dec 2022 03:18:54 +0000 (19:18 -0800)
committerMatthew Fernandez <matthew.fernandez@gmail.com>
Sun, 25 Dec 2022 01:17:08 +0000 (17:17 -0800)
Callers of `bfs` were constructing `Queue` objects and passing them into `bfs`.
But in all of these cases neither the callee nor the caller care about the
contents of the queue. This appears to have been an optimization to hoist
allocation of this object outside loops in callers. This is nowhere close to the
most expensive operation these locations are performing. And replicating
operations like this led to opportunities for errors like that fixed in the
prior commit. It seems a win to undo this for readability.

lib/neatogen/bfs.c
lib/neatogen/bfs.h
lib/neatogen/embed_graph.c
lib/neatogen/kkutils.c
lib/neatogen/stress.c

index a6f9d7ea0c5b4a9942834bd3f5dd35a49b3611df..7b2ccf657a584fec3c6c7c382a29d238f3de2a8b 100644 (file)
@@ -21,7 +21,7 @@
 #include <stdbool.h>
 #include <stdlib.h>
 
-void bfs(int vertex, vtx_data * graph, int n, DistType * dist, Queue * Q)
+void bfs(int vertex, vtx_data *graph, int n, DistType *dist)
  /* compute vector 'dist' of distances of all nodes from 'vertex' */
 {
     int i;
@@ -33,21 +33,23 @@ void bfs(int vertex, vtx_data * graph, int n, DistType * dist, Queue * Q)
        dist[i] = -1;
     dist[vertex] = 0;
 
-    initQueue(Q, vertex);
+    Queue Q;
+    mkQueue(&Q, n);
+    initQueue(&Q, vertex);
 
     if (graph[0].ewgts == NULL) {
-       while (deQueue(Q, &closestVertex)) {
+       while (deQueue(&Q, &closestVertex)) {
            closestDist = dist[closestVertex];
            for (i = 1; i < graph[closestVertex].nedges; i++) {
                neighbor = graph[closestVertex].edges[i];
                if (dist[neighbor] < -0.5) {    /* first time to reach neighbor */
                    dist[neighbor] = closestDist + 1;
-                   enQueue(Q, neighbor);
+                   enQueue(&Q, neighbor);
                }
            }
        }
     } else {
-       while (deQueue(Q, &closestVertex)) {
+       while (deQueue(&Q, &closestVertex)) {
            closestDist = dist[closestVertex];
            for (i = 1; i < graph[closestVertex].nedges; i++) {
                neighbor = graph[closestVertex].edges[i];
@@ -55,7 +57,7 @@ void bfs(int vertex, vtx_data * graph, int n, DistType * dist, Queue * Q)
                    dist[neighbor] =
                        closestDist +
                        (DistType) graph[closestVertex].ewgts[i];
-                   enQueue(Q, neighbor);
+                   enQueue(&Q, neighbor);
                }
            }
        }
@@ -65,6 +67,8 @@ void bfs(int vertex, vtx_data * graph, int n, DistType * dist, Queue * Q)
     for (i = 0; i < n; i++)
        if (dist[i] < -0.5)     /* 'i' is not connected to 'vertex' */
            dist[i] = closestDist + 10;
+
+    freeQueue(&Q);
 }
 
 int
index 61f8ef0e41a91c70a4ec46d096350b24a4131ba1..8321d41aa98c85ecc9f77328993a67401cbd690d 100644 (file)
@@ -31,7 +31,7 @@ extern "C" {
     extern bool deQueue(Queue *, int *);
     extern bool enQueue(Queue *, int);
 
-    extern void bfs(int, vtx_data *, int, DistType *, Queue *);
+    extern void bfs(int, vtx_data*, int, DistType*);
     extern int bfs_bounded(int, vtx_data*, DistType *, Queue*, int, int*);
 
 #ifdef __cplusplus
index 47d56591eb97273b2bd61a281d5d7d877156ceb0..c15024caa17146cbb35e5320c8b76d6c7af3a2c5 100644 (file)
@@ -42,7 +42,6 @@ void embed_graph(vtx_data * graph, int n, int dim, DistType *** Coords,
     DistType *dist = N_GNEW(n, DistType);      /* this vector stores  the distances of
                                                   each nodes to the selected "pivots" */
     float *old_weights = graph[0].ewgts;
-    Queue Q;
     DistType max_dist = 0;
 
     /* this matrix stores the distance between each node and each "pivot" */
@@ -57,11 +56,10 @@ void embed_graph(vtx_data * graph, int n, int dim, DistType *** Coords,
     /* select the first pivot */
     node = rand() % n;
 
-    mkQueue(&Q, n);
     if (reweight_graph) {
        dijkstra(node, graph, n, coords[0]);
     } else {
-       bfs(node, graph, n, coords[0], &Q);
+       bfs(node, graph, n, coords[0]);
     }
 
     for (i = 0; i < n; i++) {
@@ -77,7 +75,7 @@ void embed_graph(vtx_data * graph, int n, int dim, DistType *** Coords,
        if (reweight_graph) {
            dijkstra(node, graph, n, coords[i]);
        } else {
-           bfs(node, graph, n, coords[i], &Q);
+           bfs(node, graph, n, coords[i]);
        }
        max_dist = 0;
        for (j = 0; j < n; j++) {
@@ -91,7 +89,6 @@ void embed_graph(vtx_data * graph, int n, int dim, DistType *** Coords,
     }
 
     free(dist);
-    freeQueue(&Q);
 
     if (reweight_graph) {
        restore_old_weights(graph, n, old_weights);
index 8277d71cc1de92b672d2a4fb40980153735e05f3..ab2f1afb1c2d6ba1399cee2efe957db6c91a51d2 100644 (file)
@@ -74,17 +74,14 @@ static DistType **compute_apsp_simple(vtx_data * graph, int n)
     /* for unweighted graph */
     int i;
     DistType *storage = gv_calloc((size_t)(n * n), sizeof(DistType));
-    Queue Q;
 
     DistType **dij = gv_calloc(n, sizeof(DistType*));
     for (i = 0; i < n; i++) {
        dij[i] = storage + i * n;
     }
-    mkQueue(&Q, n);
     for (i = 0; i < n; i++) {
-       bfs(i, graph, n, dij[i], &Q);
+       bfs(i, graph, n, dij[i]);
     }
-    freeQueue(&Q);
     return dij;
 }
 
index dac0db524409cf8ccbe62d060df16aaf13ba43f8..490a0d3a0ba2541917ff2b55bc95562b26fc9813 100644 (file)
@@ -368,7 +368,7 @@ static int sparse_stress_subspace_majorization_kD(vtx_data * graph, /* Input gra
     if (reweight_graph) {
        dijkstra(node, graph, n, Dij[0]);
     } else {
-       bfs(node, graph, n, Dij[0], &Q);
+       bfs(node, graph, n, Dij[0]);
     }
 
     /* find the most distant node from first pivot */
@@ -387,7 +387,7 @@ static int sparse_stress_subspace_majorization_kD(vtx_data * graph, /* Input gra
        if (reweight_graph) {
            dijkstra(node, graph, n, Dij[i]);
        } else {
-           bfs(node, graph, n, Dij[i], &Q);
+           bfs(node, graph, n, Dij[i]);
        }
        max_dist = 0;
        for (j = 0; j < n; j++) {
@@ -751,19 +751,15 @@ float *compute_apsp_packed(vtx_data * graph, int n)
     float *Dij = N_NEW(n * (n + 1) / 2, float);
 
     DistType *Di = N_NEW(n, DistType);
-    Queue Q;
-
-    mkQueue(&Q, n);
 
     count = 0;
     for (i = 0; i < n; i++) {
-       bfs(i, graph, n, Di, &Q);
+       bfs(i, graph, n, Di);
        for (j = i; j < n; j++) {
            Dij[count++] = (float)Di[j];
        }
     }
     free(Di);
-    freeQueue(&Q);
     return Dij;
 }