From: Matthew Fernandez Date: Thu, 22 Dec 2022 03:18:54 +0000 (-0800) Subject: neatogen: push queue allocation into 'bfs' X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=1e560711ff4b65c026e6a231fd49db4c387e97d5;p=graphviz neatogen: push queue allocation into 'bfs' 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. --- diff --git a/lib/neatogen/bfs.c b/lib/neatogen/bfs.c index a6f9d7ea0..7b2ccf657 100644 --- a/lib/neatogen/bfs.c +++ b/lib/neatogen/bfs.c @@ -21,7 +21,7 @@ #include #include -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 diff --git a/lib/neatogen/bfs.h b/lib/neatogen/bfs.h index 61f8ef0e4..8321d41aa 100644 --- a/lib/neatogen/bfs.h +++ b/lib/neatogen/bfs.h @@ -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 diff --git a/lib/neatogen/embed_graph.c b/lib/neatogen/embed_graph.c index 47d56591e..c15024caa 100644 --- a/lib/neatogen/embed_graph.c +++ b/lib/neatogen/embed_graph.c @@ -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); diff --git a/lib/neatogen/kkutils.c b/lib/neatogen/kkutils.c index 8277d71cc..ab2f1afb1 100644 --- a/lib/neatogen/kkutils.c +++ b/lib/neatogen/kkutils.c @@ -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; } diff --git a/lib/neatogen/stress.c b/lib/neatogen/stress.c index dac0db524..490a0d3a0 100644 --- a/lib/neatogen/stress.c +++ b/lib/neatogen/stress.c @@ -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; }