Skip to content
This repository was archived by the owner on Jun 18, 2026. It is now read-only.

Commit faa63d8

Browse files
refactor: eliminate redundant Kahn's pass in TopologicalSortAnalyzer
- Compute choicePoints during analyze()'s single Kahn's BFS pass instead of re-running the entire algorithm in countChoicePoints() — saves O(V+E) - Add choicePoints field to TopologicalSortResult, delegate countChoicePoints() - Replace O(E) linear-scan findEdge() with O(1) HashMap lookup in cycle detection — build edge index once, use per-cycle-edge - Fix type mismatch: dfsCycleDetect parameter was LinkedList but callers passed ArrayDeque — unified to ArrayDeque
1 parent 42d1355 commit faa63d8

1 file changed

Lines changed: 38 additions & 63 deletions

File tree

Gvisual/src/gvisual/TopologicalSortAnalyzer.java

Lines changed: 38 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -68,13 +68,15 @@ public static class TopologicalSortResult {
6868
private final List<String> leaves;
6969
private final int longestPathLength;
7070
private final List<String> criticalPath;
71+
private final int choicePoints;
7172

7273
public TopologicalSortResult(boolean isDAG, List<String> sortedOrder,
7374
List<CycleInfo> cycles, Map<String, Integer> depthMap,
7475
Map<String, Integer> dependencyCount,
7576
Map<String, Integer> dependentCount,
7677
List<String> roots, List<String> leaves,
77-
int longestPathLength, List<String> criticalPath) {
78+
int longestPathLength, List<String> criticalPath,
79+
int choicePoints) {
7880
this.isDAG = isDAG;
7981
this.sortedOrder = Collections.unmodifiableList(new ArrayList<String>(sortedOrder));
8082
this.cycles = Collections.unmodifiableList(new ArrayList<CycleInfo>(cycles));
@@ -85,6 +87,7 @@ public TopologicalSortResult(boolean isDAG, List<String> sortedOrder,
8587
this.leaves = Collections.unmodifiableList(new ArrayList<String>(leaves));
8688
this.longestPathLength = longestPathLength;
8789
this.criticalPath = Collections.unmodifiableList(new ArrayList<String>(criticalPath));
90+
this.choicePoints = choicePoints;
8891
}
8992

9093
/** True if the graph is a DAG (no cycles). */
@@ -116,6 +119,9 @@ public TopologicalSortResult(boolean isDAG, List<String> sortedOrder,
116119

117120
/** Vertices along the critical (longest) path. */
118121
public List<String> getCriticalPath() { return criticalPath; }
122+
123+
/** Number of scheduling choice points (positions where multiple vertices are ready). -1 if cyclic. */
124+
public int getChoicePoints() { return choicePoints; }
119125
}
120126

121127
/**
@@ -237,13 +243,14 @@ public TopologicalSortResult analyze() {
237243
roots.add(v);
238244
}
239245
}
240-
// PriorityQueue maintains heap order; no manual sort needed
241246

242247
List<String> sortedOrder = new ArrayList<String>();
248+
int choicePoints = 0;
243249
while (!queue.isEmpty()) {
244-
// PriorityQueue.poll() returns lexicographically smallest in O(log V)
250+
if (queue.size() > 1) {
251+
choicePoints++;
252+
}
245253
String v = queue.poll();
246-
247254
sortedOrder.add(v);
248255

249256
List<String> succs = new ArrayList<String>(successors.get(v));
@@ -315,7 +322,8 @@ public TopologicalSortResult analyze() {
315322

316323
cachedResult = new TopologicalSortResult(isDAG, sortedOrder, cycles, depthMap,
317324
dependencyCount, dependentCount, roots, leaves,
318-
longestPathLength, criticalPath);
325+
longestPathLength, criticalPath,
326+
isDAG ? choicePoints : -1);
319327
return cachedResult;
320328
}
321329

@@ -385,48 +393,14 @@ public VertexDependencyInfo analyzeDependencies(String vertex) {
385393
* of "choice points" — positions where multiple vertices have in-degree
386394
* 0 simultaneously. A high number indicates flexible scheduling.</p>
387395
*
396+
* <p>Now delegates to the cached result from {@link #analyze()}, which
397+
* computes choice points during the single Kahn's pass — eliminating
398+
* a redundant O(V + E) traversal.</p>
399+
*
388400
* @return number of scheduling choice points, or -1 if graph has cycles
389401
*/
390402
public int countChoicePoints() {
391-
TopologicalSortResult result = analyze();
392-
if (!result.isDAG()) {
393-
return -1;
394-
}
395-
396-
// Re-use shared adjacency builder and count points where
397-
// multiple vertices are ready simultaneously
398-
GraphUtils.DirectedAdj adj = buildDirectedAdj();
399-
Map<String, Set<String>> successors = adj.successors;
400-
Map<String, Integer> inDegree = new HashMap<String, Integer>();
401-
402-
for (String v : adj.vertices) {
403-
inDegree.put(v, adj.predecessors.get(v).size());
404-
}
405-
406-
List<String> ready = new ArrayList<String>();
407-
for (Map.Entry<String, Integer> entry : inDegree.entrySet()) {
408-
if (entry.getValue() == 0) {
409-
ready.add(entry.getKey());
410-
}
411-
}
412-
413-
int choicePoints = 0;
414-
while (!ready.isEmpty()) {
415-
if (ready.size() > 1) {
416-
choicePoints++;
417-
}
418-
Collections.sort(ready);
419-
String v = ready.remove(0);
420-
for (String w : successors.get(v)) {
421-
int newDeg = inDegree.get(w) - 1;
422-
inDegree.put(w, newDeg);
423-
if (newDeg == 0) {
424-
ready.add(w);
425-
}
426-
}
427-
}
428-
429-
return choicePoints;
403+
return analyze().getChoicePoints();
430404
}
431405

432406
/**
@@ -464,8 +438,7 @@ public String generateSummary() {
464438
sb.append(" Critical path length: ").append(result.getLongestPathLength()).append("\n");
465439
sb.append(" Critical path: ").append(result.getCriticalPath()).append("\n");
466440

467-
int choicePoints = countChoicePoints();
468-
sb.append(" Scheduling flexibility: ").append(choicePoints)
441+
sb.append(" Scheduling flexibility: ").append(result.getChoicePoints())
469442
.append(" choice point(s)\n");
470443
} else {
471444
sb.append("───────────────────────────────────────────────────\n");
@@ -502,6 +475,17 @@ private List<CycleInfo> detectCycles(Set<String> vertices,
502475
Map<String, Integer> color = new HashMap<String, Integer>(); // 0=white, 1=gray, 2=black
503476
Map<String, String> parent = new HashMap<String, String>();
504477

478+
// Build edge lookup map once — O(E) — instead of O(E) per findEdge call
479+
Map<String, Map<String, Edge>> edgeLookup = new HashMap<String, Map<String, Edge>>();
480+
for (Edge e : graph.getEdges()) {
481+
Map<String, Edge> targets = edgeLookup.get(e.getVertex1());
482+
if (targets == null) {
483+
targets = new HashMap<String, Edge>();
484+
edgeLookup.put(e.getVertex1(), targets);
485+
}
486+
targets.put(e.getVertex2(), e);
487+
}
488+
505489
for (String v : vertices) {
506490
color.put(v, 0);
507491
}
@@ -511,7 +495,8 @@ private List<CycleInfo> detectCycles(Set<String> vertices,
511495

512496
for (String v : sorted) {
513497
if (color.get(v) == 0) {
514-
dfsCycleDetect(v, successors, color, parent, cycles, new ArrayDeque<String>());
498+
dfsCycleDetect(v, successors, color, parent, cycles,
499+
new ArrayDeque<String>(), edgeLookup);
515500
}
516501
}
517502

@@ -523,7 +508,8 @@ private List<CycleInfo> detectCycles(Set<String> vertices,
523508
*/
524509
private void dfsCycleDetect(String v, Map<String, Set<String>> successors,
525510
Map<String, Integer> color, Map<String, String> parent,
526-
List<CycleInfo> cycles, LinkedList<String> path) {
511+
List<CycleInfo> cycles, ArrayDeque<String> path,
512+
Map<String, Map<String, Edge>> edgeLookup) {
527513
color.put(v, 1); // GRAY
528514
path.addLast(v);
529515

@@ -543,11 +529,12 @@ private void dfsCycleDetect(String v, Map<String, Set<String>> successors,
543529
if (found) cycleVertices.add(p);
544530
}
545531

546-
// Find edges for the cycle
532+
// Find edges for the cycle via O(1) lookup
547533
for (int i = 0; i < cycleVertices.size(); i++) {
548534
String from = cycleVertices.get(i);
549535
String to = cycleVertices.get((i + 1) % cycleVertices.size());
550-
Edge e = findEdge(from, to);
536+
Map<String, Edge> targets = edgeLookup.get(from);
537+
Edge e = (targets != null) ? targets.get(to) : null;
551538
if (e != null) cycleEdges.add(e);
552539
}
553540

@@ -557,7 +544,7 @@ private void dfsCycleDetect(String v, Map<String, Set<String>> successors,
557544
}
558545
} else if (color.get(w) == 0) {
559546
parent.put(w, v);
560-
dfsCycleDetect(w, successors, color, parent, cycles, path);
547+
dfsCycleDetect(w, successors, color, parent, cycles, path, edgeLookup);
561548
}
562549
}
563550

@@ -578,18 +565,6 @@ private boolean isDuplicateCycle(List<CycleInfo> existing, List<String> newCycle
578565
return false;
579566
}
580567

581-
/**
582-
* Find an Edge from vertex1 to vertex2 in the graph.
583-
*/
584-
private Edge findEdge(String from, String to) {
585-
for (Edge e : graph.getEdges()) {
586-
if (from.equals(e.getVertex1()) && to.equals(e.getVertex2())) {
587-
return e;
588-
}
589-
}
590-
return null;
591-
}
592-
593568
/**
594569
* Reconstruct the critical (longest) path by backtracking from the
595570
* deepest vertex through predecessors with depth = current - 1.

0 commit comments

Comments
 (0)