engineering/java-topology
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

liquibase-0001: fix patch correctness + add unit test (4/4 PASS)

Browse source 
- Corrected fix in patch: use persistent `seen` set (no backtrack remove)
  so diamond shared-nodes are visited once, not 2^D times
- Previous patch used DFS path-stack (cycle guard) which did not prevent
  the 2^D blowup for convergent diamonds
- Add unit test: chained-diamond D=12 shows 334× visit reduction (16381→49)
- Performance test: ArrayList→HashSet for evaluatedNodes gives 51× speedup at N=500
This commit is contained in:
russell@unturf.com 2026-03-29 18:25:38 -04:00
parent 52a8d535a2
commit bca10f42a3
2 changed files with 324 additions and 29 deletions
Download patch file Download diff file Expand all files Collapse all files

72
defects/liquibase/patch/liquibase-0001-dependencygraph-recursive-size-depth-diamond.md
View file

@ -1,5 +1,4 @@
# UNDF: UNDF-2026-000000578
# UNDF: (pending)
# liquibase-0001: DependencyUtil.DependencyGraph.recursiveSizeDepth — O(2^D) diamond re-traversal + O(N²) evaluatedNodes list scan
## CWE-407 — Algorithmic Complexity: O(2^D) recursive diamond re-traversal; O(N²) list scan in evaluated-node guard
@ -40,7 +39,7 @@ private int recursiveSizeDepth(GraphNode<T> node, int safetyCounter) {
int sum = 0;
safetyCounter++;
for (GraphNode<T> n : node.getGoingOutNodes()) {
int depth = recursiveSizeDepth(n, safetyCounter); // recurse — no current-path guard
int depth = recursiveSizeDepth(n, safetyCounter); // recurse — no visited guard
if (depth < 0) return -1;
sum += depth;
}
@ -54,9 +53,10 @@ private boolean isAlreadyEvaluated(GraphNode<T> node) {
Two distinct defects:
1. **Diamond re-traversal:** `recursiveSizeDepth` has no guard for nodes currently being
traversed in the recursion stack. On a diamond graph it visits shared nodes 2^D times.
Also causes incorrect depth estimates (double-counts shared nodes).
1. **Diamond re-traversal:** `recursiveSizeDepth` has no visited set. On a diamond graph
it visits shared nodes 2^D times. The existing `safetyCounter > 1000` guard is an
emergency brake, not a fix — it truncates the estimate incorrectly and fires only
when recursion nesting exceeds 1000, not when nodes are revisited.
2. **O(N) evaluatedNodes scan:** `evaluatedNodes` is an `ArrayList`. Each call to
`isAlreadyEvaluated` or `areAlreadyEvaluated` is an O(N) scan.
@ -65,32 +65,44 @@ Two distinct defects:
## Fix
Replace `evaluatedNodes: List<GraphNode<T>>` with a `Set<GraphNode<T>>` for O(1) membership,
and add a `Set<GraphNode<T>> currentPath` parameter to `recursiveSizeDepth` to guard against
diamond re-traversal:
and add a persistent `Set<GraphNode<T>> seen` parameter to `recursiveSizeDepth`. The `seen`
set is **not** cleared on backtrack — it accumulates all visited nodes across the entire
call tree, preventing any node from being counted twice regardless of how many paths lead
to it (diamond, fan-out, or any DAG shape).
```java
// AFTER — O(N+E) total
private final Set<GraphNode<T>> evaluatedNodes = new LinkedHashSet<>(); // O(1) contains()
private int recursiveSizeDepth(GraphNode<T> node, int safetyCounter,
Set<GraphNode<T>> currentPath) {
if (safetyCounter > 1000) { return -1; }
if (evaluatedNodes.contains(node)) { return 0; } // O(1)
if (!currentPath.add(node)) { return 0; } // diamond guard: O(1), prevents 2^D
try {
if (node.getGoingOutNodes() == null || node.getGoingOutNodes().isEmpty()) {
return 1;
}
int sum = 0;
for (GraphNode<T> n : node.getGoingOutNodes()) {
int depth = recursiveSizeDepth(n, safetyCounter + 1, currentPath);
if (depth < 0) return -1;
sum += depth;
}
return node.getGoingOutNodes().size() + sum;
} finally {
currentPath.remove(node);
// Public entry point: allocate seen set once per depth-check call
private int recursiveSizeDepth(List<GraphNode<T>> nodes) {
if (nodes == null) return 0;
Set<GraphNode<T>> seen = new HashSet<>();
int sum = 0;
for (GraphNode<T> node : nodes) {
int depth = recursiveSizeDepth(node, 0, seen);
if (depth < 0) return -1;
sum += depth;
}
return sum;
}
private int recursiveSizeDepth(GraphNode<T> node, int safetyCounter,
Set<GraphNode<T>> seen) {
if (safetyCounter > 1000) { return -1; }
if (evaluatedNodes.contains(node)) { return 0; } // O(1) — already emitted
if (!seen.add(node)) { return 0; } // O(1) — already counted this call
// NOTE: do NOT remove from `seen` on return — persistent across all branches
if (node.getGoingOutNodes() == null || node.getGoingOutNodes().isEmpty()) {
return 1;
}
int sum = 0;
for (GraphNode<T> n : node.getGoingOutNodes()) {
int depth = recursiveSizeDepth(n, safetyCounter + 1, seen);
if (depth < 0) return -1;
sum += depth;
}
return node.getGoingOutNodes().size() + sum;
}
```
@ -105,8 +117,10 @@ private boolean areAlreadyEvaluated(List<GraphNode<T>> nodes) {
| Diamond depth (D) | Before (visits) | After (visits) | Speedup |
|------------------|----------------|----------------|---------|
| 10 | 1,023 | 10 | 102× |
| 15 | 32,767 | 15 | 2,184× |
| 20 | 1,048,575 | 20 | 52,428× |
| 10 | 4,093 | 41 | 100× |
| 12 | 16,381 | 49 | 334× |
| 15 | 131,069 | 61 | 2,148× |
Growth before: O(2^D). Growth after: O(D).
Measured by unit test: `defects/liquibase/unit/unit/test_liquibase_0001.py`
Growth before: O(2^D). Growth after: O(D+nodes) linear.

281
defects/liquibase/unit/unit/test_liquibase_0001.py Normal file
View file

@ -0,0 +1,281 @@
"""
Unit test for liquibase-0001:
DependencyUtil.DependencyGraph.recursiveSizeDepth — O(2^D) diamond re-traversal
+ O(N²) evaluatedNodes ArrayList scan.
Two defects modelled here:
1. recursiveSizeDepth recurses over goingOutNodes without any visited set,
so chained diamond graphs are traversed O(2^D) times (exponential blowup).
2. isAlreadyEvaluated calls evaluatedNodes.contains() on an ArrayList — O(N) scan.
Python reimplements the BEFORE and AFTER logic faithfully.
The fix for the diamond blowup is a persistent `seen` set (not removed on backtrack)
passed through the entire recursiveSizeDepth call tree.
"""
import time
# ---- BEFORE: defective implementation (mirrors DependencyUtil.java) ----
class GraphNodeBefore:
def __init__(self, value):
self.value = value
self.coming_in = []
self.going_out = []
def add_going_out(self, node):
self.going_out.append(node)
def add_coming_in(self, node):
self.coming_in.append(node)
class DependencyGraphBefore:
"""Mirrors DependencyUtil.DependencyGraph with the defects intact."""
def __init__(self):
self.nodes = {}
self.evaluated_nodes = [] # ArrayList — O(N) contains()
self.total_visits = 0 # instrumentation
def add(self, eval_first, eval_after):
if eval_first not in self.nodes:
self.nodes[eval_first] = GraphNodeBefore(eval_first)
if eval_after not in self.nodes:
self.nodes[eval_after] = GraphNodeBefore(eval_after)
first = self.nodes[eval_first]
after = self.nodes[eval_after]
first.add_going_out(after)
after.add_coming_in(first)
def _is_already_evaluated(self, node):
return node in self.evaluated_nodes # O(N) list scan
def _are_already_evaluated(self, nodes):
return all(self._is_already_evaluated(n) for n in nodes)
def _recursive_size_depth(self, node, safety_counter):
self.total_visits += 1
if safety_counter > 1000:
return -1
if self._is_already_evaluated(node): # O(N), but misses unevaluated diamonds
return 0
if not node.going_out:
return 1
total = 0
safety_counter += 1
for n in node.going_out:
depth = self._recursive_size_depth(n, safety_counter) # no diamond guard
if depth < 0:
return -1
total += depth
return len(node.going_out) + total
# ---- AFTER: fixed implementation ----
class GraphNodeAfter:
def __init__(self, value):
self.value = value
self.coming_in = []
self.going_out = []
def add_going_out(self, node):
self.going_out.append(node)
def add_coming_in(self, node):
self.coming_in.append(node)
class DependencyGraphAfter:
"""Fix:
1. evaluatedNodes is a set — O(1) contains().
2. recursiveSizeDepth takes a persistent `seen` set shared across the entire
call tree — each node is visited at most once regardless of diamond structure.
"""
def __init__(self):
self.nodes = {}
self.evaluated_nodes = set() # O(1) contains()
self.total_visits = 0
def add(self, eval_first, eval_after):
if eval_first not in self.nodes:
self.nodes[eval_first] = GraphNodeAfter(eval_first)
if eval_after not in self.nodes:
self.nodes[eval_after] = GraphNodeAfter(eval_after)
first = self.nodes[eval_first]
after = self.nodes[eval_after]
first.add_going_out(after)
after.add_coming_in(first)
def _is_already_evaluated(self, node):
return node in self.evaluated_nodes # O(1) set lookup
def _are_already_evaluated(self, nodes):
return all(self._is_already_evaluated(n) for n in nodes)
def _recursive_size_depth(self, node, safety_counter, seen):
"""
`seen` is a persistent set for the entire call tree — nodes are never
removed from it, so each shared (diamond) node is visited at most once.
"""
self.total_visits += 1
if safety_counter > 1000:
return -1
if node in self.evaluated_nodes: # O(1) — already emitted
return 0
if node in seen: # O(1) diamond guard — already counted
return 0
seen.add(node)
if not node.going_out:
return 1
total = 0
for n in node.going_out:
depth = self._recursive_size_depth(n, safety_counter + 1, seen)
if depth < 0:
return -1
total += depth
return len(node.going_out) + total
# ---- Graph builders ----
def make_chained_diamond_graph(GraphClass, depth):
"""
Build a chain of `depth` diamonds using the provided graph class.
Structure (depth=2):
root -> left_0, right_0
left_0, right_0 -> shared_0
shared_0 -> left_1, right_1
left_1, right_1 -> shared_1
At depth D, BEFORE visits O(2^D) nodes; AFTER visits O(4*D) nodes.
"""
g = GraphClass()
g.add("root", "left_0")
g.add("root", "right_0")
g.add("left_0", "shared_0")
g.add("right_0", "shared_0")
for i in range(1, depth):
g.add(f"shared_{i-1}", f"left_{i}")
g.add(f"shared_{i-1}", f"right_{i}")
g.add(f"left_{i}", f"shared_{i}")
g.add(f"right_{i}", f"shared_{i}")
return g
# ---- Tests ----
def test_chained_diamond_before_blowup():
"""
BEFORE: chained diamonds cause superlinear (exponential) node visits.
At D=10, BEFORE visits far more nodes than AFTER.
"""
D = 10
g = make_chained_diamond_graph(DependencyGraphBefore, D)
root = g.nodes["root"]
g.total_visits = 0
g._recursive_size_depth(root, 0)
visits = g.total_visits
# There are only 4*D+1 unique nodes; BEFORE visits O(2^D) due to diamond re-traversal
unique_nodes = 4 * D + 1
print(f" BEFORE D={D}: {visits} visits for {unique_nodes} unique nodes")
assert visits > unique_nodes * 10, (
f"Expected BEFORE to visit >10x unique node count ({unique_nodes}), got {visits}"
)
def test_chained_diamond_after_linear():
"""
AFTER: each node visited at most once — linear in graph size.
"""
D = 10
g = make_chained_diamond_graph(DependencyGraphAfter, D)
root = g.nodes["root"]
g.total_visits = 0
g._recursive_size_depth(root, 0, set())
visits = g.total_visits
unique_nodes = 4 * D + 1
print(f" AFTER D={D}: {visits} visits for {unique_nodes} unique nodes")
# Each unique node visited at most once
assert visits <= unique_nodes + 5, (
f"Expected AFTER to visit ~{unique_nodes} unique nodes, got {visits}"
)
def test_exponential_blowup_ratio():
"""
BEFORE/AFTER visit count ratio grows exponentially with D.
At D=12 the ratio should be >= 100x.
"""
D = 12
g_before = make_chained_diamond_graph(DependencyGraphBefore, D)
g_after = make_chained_diamond_graph(DependencyGraphAfter, D)
g_before.total_visits = 0
root_before = g_before.nodes["root"]
g_before._recursive_size_depth(root_before, 0)
visits_before = g_before.total_visits
g_after.total_visits = 0
root_after = g_after.nodes["root"]
g_after._recursive_size_depth(root_after, 0, set())
visits_after = g_after.total_visits
ratio = visits_before / max(visits_after, 1)
print(f" D={D}: BEFORE={visits_before} visits, AFTER={visits_after} visits, ratio={ratio:.0f}x")
assert ratio >= 100, f"Expected >=100x ratio at D={D}, got {ratio:.1f}x"
assert visits_after <= 4 * D + 10, (
f"Expected AFTER <=linear at D={D}, got {visits_after}"
)
def test_performance_isAlreadyEvaluated_list_vs_set():
"""
AFTER (set) is significantly faster for isAlreadyEvaluated than BEFORE (list).
Build N evaluated nodes and measure time for a contains() check on the last element.
"""
N = 500
g_before = DependencyGraphBefore()
g_after = DependencyGraphAfter()
nodes_before = [GraphNodeBefore(i) for i in range(N)]
nodes_after = [GraphNodeAfter(i) for i in range(N)]
g_before.evaluated_nodes = list(nodes_before) # ArrayList
g_after.evaluated_nodes = set(nodes_after) # HashSet
RUNS = 5000
target_before = nodes_before[-1]
target_after = nodes_after[-1]
t0 = time.perf_counter()
for _ in range(RUNS):
g_before._is_already_evaluated(target_before)
t_before = (time.perf_counter() - t0) / RUNS * 1e6 # microseconds
t1 = time.perf_counter()
for _ in range(RUNS):
g_after._is_already_evaluated(target_after)
t_after = (time.perf_counter() - t1) / RUNS * 1e6
ratio = t_before / max(t_after, 1e-9)
print(f" N={N}: BEFORE={t_before:.2f}us, AFTER={t_after:.2f}us, ratio={ratio:.1f}x")
assert ratio >= 5.0, f"Expected >=5x speedup at N={N}, got {ratio:.1f}x"
if __name__ == "__main__":
tests = [
test_chained_diamond_before_blowup,
test_chained_diamond_after_linear,
test_exponential_blowup_ratio,
test_performance_isAlreadyEvaluated_list_vs_set,
]
for t in tests:
print(f"=== {t.__name__} ===")
t()
print(" PASS")
print("\nAll tests passed.")