From 1cd635afa5d6646dbc3b5684712a8ca3ed7a0e73 Mon Sep 17 00:00:00 2001 From: Alexey Andreev Date: Tue, 19 Sep 2023 20:15:51 +0200 Subject: [PATCH] Fix node splitting in irreducible CFG --- .../transform/CoroutineTransformation.java | 18 +- .../common/DefaultGraphSplittingBackend.java | 32 ++- .../teavm/common/GraphSplittingBackend.java | 2 +- .../common/IrreducibleGraphSplitter.java | 234 ++++++++++-------- .../model/util/AsyncProgramSplitter.java | 6 +- .../util/ProgramNodeSplittingBackend.java | 12 +- .../test/java/org/teavm/common/GraphTest.java | 77 ++++-- 7 files changed, 216 insertions(+), 165 deletions(-) diff --git a/core/src/main/java/org/teavm/backend/lowlevel/transform/CoroutineTransformation.java b/core/src/main/java/org/teavm/backend/lowlevel/transform/CoroutineTransformation.java index 1be2a83f3..3f70d46b7 100644 --- a/core/src/main/java/org/teavm/backend/lowlevel/transform/CoroutineTransformation.java +++ b/core/src/main/java/org/teavm/backend/lowlevel/transform/CoroutineTransformation.java @@ -484,13 +484,13 @@ public class CoroutineTransformation { class SplittingBackend implements GraphSplittingBackend { @Override - public int[] split(int[] domain, int[] nodes) { - int[] copies = new int[nodes.length]; + public int[] split(int[] remaining, int[] toCopy) { + int[] copies = new int[toCopy.length]; var map = new IntIntHashMap(); - IntSet nodeSet = IntHashSet.from(nodes); + IntSet nodeSet = IntHashSet.from(toCopy); var outputs = ProgramUtils.getPhiOutputs(program); - for (int i = 0; i < nodes.length; ++i) { - int node = nodes[i]; + for (int i = 0; i < toCopy.length; ++i) { + int node = toCopy[i]; BasicBlock block = program.basicBlockAt(node); BasicBlock blockCopy = program.createBasicBlock(); ProgramUtils.copyBasicBlock(block, blockCopy); @@ -505,13 +505,13 @@ public class CoroutineTransformation { for (int copy : copies) { copyBlockMapper.transform(program.basicBlockAt(copy)); } - for (int domainNode : domain) { + for (int domainNode : remaining) { copyBlockMapper.transformWithoutPhis(program.basicBlockAt(domainNode)); } - var domainSet = IntHashSet.from(domain); - for (int i = 0; i < nodes.length; ++i) { - int node = nodes[i]; + var domainSet = IntHashSet.from(remaining); + for (int i = 0; i < toCopy.length; ++i) { + int node = toCopy[i]; BasicBlock blockCopy = program.basicBlockAt(copies[i]); for (Incoming output : outputs.get(node)) { if (!nodeSet.contains(output.getPhi().getBasicBlock().getIndex())) { diff --git a/core/src/main/java/org/teavm/common/DefaultGraphSplittingBackend.java b/core/src/main/java/org/teavm/common/DefaultGraphSplittingBackend.java index 5457b62a0..6c1c6075a 100644 --- a/core/src/main/java/org/teavm/common/DefaultGraphSplittingBackend.java +++ b/core/src/main/java/org/teavm/common/DefaultGraphSplittingBackend.java @@ -16,7 +16,6 @@ package org.teavm.common; import com.carrotsearch.hppc.IntIntHashMap; -import com.carrotsearch.hppc.IntIntMap; public class DefaultGraphSplittingBackend implements GraphSplittingBackend { private MutableDirectedGraph graph; @@ -46,37 +45,36 @@ public class DefaultGraphSplittingBackend implements GraphSplittingBackend { } @Override - public int[] split(int[] domain, int[] nodes) { - int[] copies = new int[nodes.length]; - IntIntMap map = new IntIntHashMap(); - for (int i = 0; i < nodes.length; ++i) { + public int[] split(int[] remaining, int[] toCopy) { + var copies = new int[toCopy.length]; + var map = new IntIntHashMap(); + for (int i = 0; i < toCopy.length; ++i) { copies[i] = index++; - map.put(nodes[i], copies[i] + 1); - int proto = prototypeNodes.get(nodes[i]); + map.put(toCopy[i], copies[i]); + int proto = prototypeNodes.get(toCopy[i]); prototypeNodes.add(proto); copyIndexes.add(++copyCount[proto]); } - for (int i = 0; i < domain.length; ++i) { - int node = domain[i]; + for (int i = 0; i < remaining.length; ++i) { + int node = remaining[i]; for (int succ : graph.outgoingEdges(node)) { - int succCopy = map.get(succ); - if (succCopy == 0) { + int succCopy = map.getOrDefault(succ, -1); + if (succCopy < 0) { continue; } - --succCopy; graph.deleteEdge(node, succ); graph.addEdge(node, succCopy); } } - for (int i = 0; i < nodes.length; ++i) { - int node = nodes[i]; + for (int i = 0; i < toCopy.length; ++i) { + int node = toCopy[i]; int nodeCopy = copies[i]; for (int succ : graph.outgoingEdges(node)) { - int succCopy = map.get(succ); - if (succCopy != 0) { - graph.addEdge(nodeCopy, succCopy - 1); + int succCopy = map.getOrDefault(succ, -1); + if (succCopy >= 0) { + graph.addEdge(nodeCopy, succCopy); } else { graph.addEdge(nodeCopy, succ); } diff --git a/core/src/main/java/org/teavm/common/GraphSplittingBackend.java b/core/src/main/java/org/teavm/common/GraphSplittingBackend.java index 42fe9d91d..7ed321c5a 100644 --- a/core/src/main/java/org/teavm/common/GraphSplittingBackend.java +++ b/core/src/main/java/org/teavm/common/GraphSplittingBackend.java @@ -16,5 +16,5 @@ package org.teavm.common; public interface GraphSplittingBackend { - int[] split(int[] domain, int[] nodes); + int[] split(int[] remaining, int[] toCopy); } diff --git a/core/src/main/java/org/teavm/common/IrreducibleGraphSplitter.java b/core/src/main/java/org/teavm/common/IrreducibleGraphSplitter.java index 07732b6c3..52fe0a78b 100644 --- a/core/src/main/java/org/teavm/common/IrreducibleGraphSplitter.java +++ b/core/src/main/java/org/teavm/common/IrreducibleGraphSplitter.java @@ -29,14 +29,14 @@ import java.util.Arrays; */ class IrreducibleGraphSplitter { private GraphSplittingBackend backend; - private int[] idom; + private DominatorTree dom; private int[][] domNodes; private MutableDirectedGraph cfg; private int[] weights; private IntArrayList[] realNodes; private int[][] spBackEdges; - private int[] levels; private int[] tmpArray; + private int[] tmpArray2; private IntArrayList copiedRealNodes = new IntArrayList(); private int additionalWeight; private int[] collapseMap; @@ -61,18 +61,14 @@ class IrreducibleGraphSplitter { } this.backend = backend; tmpArray = new int[src.size()]; + tmpArray2 = new int[src.size()]; cfg = new MutableDirectedGraph(src); - DominatorTree domTree = GraphUtils.buildDominatorTree(src); - idom = new int[size]; - for (int i = 0; i < size; ++i) { - idom[i] = domTree.immediateDominatorOf(i); - } + dom = GraphUtils.buildDominatorTree(src); collapseMap = new int[size]; for (int i = 0; i < size; ++i) { collapseMap[i] = i; } buildDomGraph(); - buildLevels(); dfs(); this.realNodes = new IntArrayList[realNodes.length]; for (int i = 0; i < cfg.size(); ++i) { @@ -84,20 +80,20 @@ class IrreducibleGraphSplitter { // n-th element of output array (domGraph) will contain nodes, directly dominated by node n. private void buildDomGraph() { int size = cfg.size(); - int[] domGraphCount = new int[size]; + var domGraphCount = new int[size]; for (int i = 0; i < size; ++i) { - int j = idom[i]; + int j = dom.immediateDominatorOf(i); if (j >= 0) { domGraphCount[j]++; } } - int[][] domGraph = new int[size][]; + var domGraph = new int[size][]; for (int i = 0; i < size; ++i) { domGraph[i] = new int[domGraphCount[i]]; domGraphCount[i] = 0; } for (int i = 0; i < size; ++i) { - int j = idom[i]; + int j = dom.immediateDominatorOf(i); if (j >= 0) { domGraph[j][domGraphCount[j]++] = i; } @@ -105,34 +101,6 @@ class IrreducibleGraphSplitter { this.domNodes = domGraph; } - // n-th element of output array (levels) will contain length of the path from root to node N - // (paper calls this 'level'). - private void buildLevels() { - int size = cfg.size(); - levels = new int[size]; - Arrays.fill(levels, -1); - levels[0] = 0; - for (int i = 1; i < size; ++i) { - if (levels[i] >= 0 || idom[i] < 0) { - continue; - } - - int node = i; - int depth = 0; - while (levels[node] < 0) { - node = idom[node]; - depth++; - } - - int level = depth + levels[node]; - node = i; - while (levels[node] < 0) { - levels[node] = level--; - node = idom[node]; - } - } - } - // Find back edges. // The n-th element of output array (sbBackEdges) will contain null if there is no back edges leading to n, // or array of nodes m_i, where each edge m_i -> n is a back edge in spanning tree @@ -190,13 +158,13 @@ class IrreducibleGraphSplitter { // This is an implementation of 'split_loop' function from the paper. // It does not take 'top' and 'set' parameter. - // Instead, it always starts with 0 node as top and assumes that all of the 'set' nodes are in the graph + // Instead, it always starts with 0 node as top and assumes that all the 'set' nodes are in the graph // We rewrote this method to use stack instead of recursion. The only place where we need recursion // is handleScc. We build a new instance of this class with corresponding subgraph. void splitLoops() { int size = cfg.size(); - boolean[] cross = new boolean[size]; - int[] stack = new int[size * 4]; + var cross = new boolean[size]; + var stack = new int[size * 4]; int head = 0; stack[head++] = 0; @@ -214,28 +182,28 @@ class IrreducibleGraphSplitter { } } else { if (cross[node]) { - for (int successor : domNodes[node]) { - collapse(successor); - } handleIrreducibleChildren(node); } int[] back = spBackEdges[node]; - int parent = idom[node]; + int parent = dom.immediateDominatorOf(node); if (back != null && parent >= 0) { for (int predecessor : back) { - if (!dominates(node, predecessor)) { + if (!dom.dominates(node, predecessor)) { cross[parent] = true; break; } } } + if (domNodes[node].length > 1) { + collapse(node); + } } } } private void handleIrreducibleChildren(int top) { - Graph levelSubgraph = GraphUtils.subgraph(cfg, node -> node == top || idom[node] == top); - int[][] sccs = GraphUtils.findStronglyConnectedComponents(levelSubgraph); + var levelSubgraph = GraphUtils.subgraph(cfg, node -> node != top && dom.dominates(top, node)); + var sccs = GraphUtils.findStronglyConnectedComponents(levelSubgraph); for (int[] scc : sccs) { if (scc.length > 1) { handleStronglyConnectedComponent(top, scc); @@ -244,75 +212,100 @@ class IrreducibleGraphSplitter { } private void handleStronglyConnectedComponent(int top, int[] scc) { + var domainCount = 0; + for (var node : scc) { + if (dom.immediateDominatorOf(node) == top) { + ++domainCount; + } + } + if (domainCount < 2) { + return; + } + // Find header node - int domain = scc[0]; - int maxWeight = weights[domain]; - for (int i = 1; i < scc.length; ++i) { - int node = scc[i]; - if (weights[node] > maxWeight) { - maxWeight = weights[node]; - domain = node; + var domains = fillDomains(top, scc); + var localWeights = fillWeights(domains, scc); + + int remaining = -1; + int maxWeight = 0; + for (var node : scc) { + if (domains[node] != node) { + continue; + } + if (remaining < 0 || localWeights[node] > maxWeight) { + maxWeight = localWeights[node]; + remaining = node; } } - int[] realDomainNodes = realNodes[domain].toArray(); + var realRemainingNodesCount = 0; int realNodesToCopyCount = 0; + int copyWeight = 0; + int remainingNodeCount = 0; for (int node : scc) { - if (node != domain) { + if (domains[node] != remaining) { realNodesToCopyCount += realNodes[node].size(); + copyWeight += weights[node]; + } else { + remainingNodeCount++; + realRemainingNodesCount += realNodes[node].size(); } } - int[] realNodesToCopy = new int[realNodesToCopyCount]; + var realNodesToCopy = new int[realNodesToCopyCount]; + var realRemainingNodes = new int[realRemainingNodesCount]; realNodesToCopyCount = 0; + realRemainingNodesCount = 0; for (int node : scc) { - if (node != domain) { - int[] nodes = realNodes[node].toArray(); + var nodes = realNodes[node].toArray(); + if (domains[node] != remaining) { System.arraycopy(nodes, 0, realNodesToCopy, realNodesToCopyCount, nodes.length); realNodesToCopyCount += nodes.length; + } else { + System.arraycopy(nodes, 0, realRemainingNodes, realRemainingNodesCount, nodes.length); + realRemainingNodesCount += nodes.length; } } - int[] realNodesCopies = backend.split(realDomainNodes, realNodesToCopy); + var realNodesCopies = backend.split(realRemainingNodes, realNodesToCopy); copiedRealNodes.add(realNodesCopies); - realNodes[top].add(realNodesCopies); - int copyWeight = 0; - for (int node : scc) { - if (node != domain) { - copyWeight += weights[node]; - } - } - this.additionalWeight += copyWeight; - weights[top] += copyWeight; - int subgraphSize = scc.length * 2; - GraphBuilder subgraph = new GraphBuilder(subgraphSize); - int[][] subgraphRealNodes = new int[subgraphSize][]; - int[] subgraphWeights = new int[subgraphSize]; - int[] map = new int[cfg.size()]; - int[] copyMap = new int[cfg.size()]; + int subgraphSize = scc.length * 2 + 1 - remainingNodeCount; + var subgraph = new GraphBuilder(subgraphSize); + var subgraphRealNodes = new int[subgraphSize][]; + var subgraphWeights = new int[subgraphSize]; + var map = new int[cfg.size()]; + var copyMap = new int[cfg.size()]; Arrays.fill(map, -1); Arrays.fill(copyMap, -1); - map[top] = 0; - subgraphRealNodes[0] = realNodes[top].toArray(); - subgraphWeights[0] = weights[top]; + subgraphRealNodes[0] = new int[0]; + subgraphWeights[0] = 0; for (int i = 0; i < scc.length; ++i) { int node = scc[i]; map[node] = i + 1; subgraphRealNodes[i + 1] = realNodes[node].toArray(); subgraphWeights[i + 1] = weights[node]; + if (domains[node] == node) { + var hasPredecessorsOutsideLoop = false; + for (var pred : cfg.incomingEdges(node)) { + if (domains[pred] < 0) { + hasPredecessorsOutsideLoop = true; + break; + } + } + if (hasPredecessorsOutsideLoop) { + subgraph.addEdge(0, i + 1); + } + } } int copyIndex = scc.length + 1; int realNodeCopiesIndex = 0; for (int node : scc) { - if (node == domain) { + if (domains[node] == remaining) { continue; } copyMap[node] = copyIndex; int realNodeCount = realNodes[node].size(); - if (node == top) { - realNodeCount -= realNodesCopies.length; - } subgraphRealNodes[copyIndex] = Arrays.copyOfRange(realNodesCopies, realNodeCopiesIndex, realNodeCopiesIndex + realNodeCount); realNodeCopiesIndex += realNodeCount; @@ -320,13 +313,10 @@ class IrreducibleGraphSplitter { copyIndex++; } - for (int i = 0; i < scc.length; ++i) { - subgraph.addEdge(0, i + 1); - } for (int node : scc) { int subgraphNode = map[node]; int subgraphNodeCopy = copyMap[node]; - int[] successors = cfg.outgoingEdges(node); + var successors = cfg.outgoingEdges(node); for (int successor : successors) { // (x, y) = (node, successor) int subgraphSuccessor = map[successor]; @@ -354,22 +344,60 @@ class IrreducibleGraphSplitter { } } - IrreducibleGraphSplitter subgraphSplitter = new IrreducibleGraphSplitter(backend, subgraph.build(), + var subgraphSplitter = new IrreducibleGraphSplitter(backend, subgraph.build(), subgraphWeights, subgraphRealNodes); subgraphSplitter.splitLoops(); - copiedRealNodes.addAll(subgraphSplitter.copiedRealNodes); realNodes[top].addAll(subgraphSplitter.copiedRealNodes); - additionalWeight += subgraphSplitter.additionalWeight; - weights[top] += subgraphSplitter.additionalWeight; + realNodes[top].add(realNodesCopies); + weights[top] += subgraphSplitter.additionalWeight + copyWeight; + copiedRealNodes.addAll(subgraphSplitter.copiedRealNodes); + additionalWeight += subgraphSplitter.additionalWeight + copyWeight; } - private boolean dominates(int dominator, int node) { - int targetLevel = levels[dominator]; - int level = levels[node]; - while (level-- > targetLevel) { - node = idom[node]; + private int[] fillDomains(int top, int[] nodes) { + var domains = tmpArray; + Arrays.fill(domains, -1); + for (var node : nodes) { + var domain = domains[node]; + if (domain < 0) { + while (true) { + var parent = dom.immediateDominatorOf(node); + if (parent == top) { + domain = node; + break; + } + domain = domains[parent]; + if (domain >= 0) { + break; + } + domains[parent] = node; + node = parent; + } + while (true) { + var next = domains[node]; + domains[node] = domain; + if (next == -1) { + break; + } + node = next; + } + } } - return node == dominator; + + return domains; + } + + private int[] fillWeights(int[] domains, int[] nodes) { + var localWeights = tmpArray2; + for (var node : nodes) { + if (domains[node] == node) { + localWeights[node] = 0; + } + } + for (var node : nodes) { + localWeights[domains[node]] += weights[node]; + } + return localWeights; } private void collapse(int top) { @@ -377,9 +405,9 @@ class IrreducibleGraphSplitter { return; } int count = findAllDominatedNodes(top); - int[] nodes = tmpArray; + var nodes = tmpArray; - IntArrayList topRealNodes = realNodes[top]; + var topRealNodes = realNodes[top]; for (int i = 1; i < count; ++i) { int node = nodes[i]; topRealNodes.addAll(realNodes[node]); @@ -410,13 +438,13 @@ class IrreducibleGraphSplitter { } private int findAllDominatedNodes(int top) { - int[] result = tmpArray; + var result = tmpArray; int count = 0; int head = 0; result[count++] = top; while (head < count) { - int[] successors = domNodes[result[head]]; + var successors = domNodes[result[head]]; if (successors != null && successors.length > 0) { System.arraycopy(successors, 0, result, count, successors.length); count += successors.length; diff --git a/core/src/main/java/org/teavm/model/util/AsyncProgramSplitter.java b/core/src/main/java/org/teavm/model/util/AsyncProgramSplitter.java index 051147d1a..1db9ac808 100644 --- a/core/src/main/java/org/teavm/model/util/AsyncProgramSplitter.java +++ b/core/src/main/java/org/teavm/model/util/AsyncProgramSplitter.java @@ -338,11 +338,11 @@ public class AsyncProgramSplitter { } @Override - public int[] split(int[] domain, int[] nodes) { - int[] copies = inner.split(domain, nodes); + public int[] split(int[] remaining, int[] toCopy) { + int[] copies = inner.split(remaining, toCopy); for (int i = 0; i < copies.length; ++i) { int copy = copies[i]; - int node = nodes[i]; + int node = toCopy[i]; if (blockSuccessors.size() <= copy) { blockSuccessors.add(-1); splitPoints.add(null); diff --git a/core/src/main/java/org/teavm/model/util/ProgramNodeSplittingBackend.java b/core/src/main/java/org/teavm/model/util/ProgramNodeSplittingBackend.java index 237bbc6ce..151250bd7 100644 --- a/core/src/main/java/org/teavm/model/util/ProgramNodeSplittingBackend.java +++ b/core/src/main/java/org/teavm/model/util/ProgramNodeSplittingBackend.java @@ -29,16 +29,16 @@ public class ProgramNodeSplittingBackend implements GraphSplittingBackend { } @Override - public int[] split(int[] domain, int[] nodes) { - int[] copies = new int[nodes.length]; + public int[] split(int[] remaining, int[] toCopy) { + int[] copies = new int[toCopy.length]; IntIntMap map = new IntIntHashMap(); - for (int i = 0; i < nodes.length; ++i) { - int node = nodes[i]; + for (int i = 0; i < toCopy.length; ++i) { + int node = toCopy[i]; BasicBlock block = program.basicBlockAt(node); BasicBlock blockCopy = program.createBasicBlock(); ProgramUtils.copyBasicBlock(block, blockCopy); copies[i] = blockCopy.getIndex(); - map.put(nodes[i], copies[i] + 1); + map.put(toCopy[i], copies[i] + 1); } BasicBlockMapper copyBlockMapper = new BasicBlockMapper((int block) -> { int mappedIndex = map.get(block); @@ -47,7 +47,7 @@ public class ProgramNodeSplittingBackend implements GraphSplittingBackend { for (int copy : copies) { copyBlockMapper.transform(program.basicBlockAt(copy)); } - for (int domainNode : domain) { + for (int domainNode : remaining) { copyBlockMapper.transform(program.basicBlockAt(domainNode)); } return copies; diff --git a/core/src/test/java/org/teavm/common/GraphTest.java b/core/src/test/java/org/teavm/common/GraphTest.java index 55d1a4a6d..59e5eb25f 100644 --- a/core/src/test/java/org/teavm/common/GraphTest.java +++ b/core/src/test/java/org/teavm/common/GraphTest.java @@ -144,14 +144,12 @@ public class GraphTest { builder.addEdge(5, 3); Graph graph = builder.build(); - DefaultGraphSplittingBackend backend = new DefaultGraphSplittingBackend(graph); + var backend = new DefaultGraphSplittingBackend(graph); int[] weights = { 1, 4, 1, 10, 1, 1 }; GraphUtils.splitIrreducibleGraph(graph, weights, backend); Graph result = backend.getGraph(); - assertTrue("Should be irreducible", GraphUtils.isIrreducible(graph)); - assertFalse("Should be reducible", GraphUtils.isIrreducible(result)); - assertTrue("Should be equivalent", isEquivalent(backend, graph)); + check(graph, result, backend); } @Test @@ -163,15 +161,13 @@ public class GraphTest { builder.addEdge(2, 1); Graph graph = builder.build(); - DefaultGraphSplittingBackend backend = new DefaultGraphSplittingBackend(graph); + var backend = new DefaultGraphSplittingBackend(graph); int[] weights = new int[graph.size()]; Arrays.fill(weights, 1); GraphUtils.splitIrreducibleGraph(graph, weights, backend); Graph result = backend.getGraph(); - assertTrue("Should be irreducible", GraphUtils.isIrreducible(graph)); - assertFalse("Should be reducible", GraphUtils.isIrreducible(result)); - assertTrue("Should be equivalent", isEquivalent(backend, graph)); + check(graph, result, backend); } @Test @@ -195,15 +191,13 @@ public class GraphTest { builder.addEdge(7, 8); builder.addEdge(8, 7); Graph graph = builder.build(); - DefaultGraphSplittingBackend backend = new DefaultGraphSplittingBackend(graph); + var backend = new DefaultGraphSplittingBackend(graph); int[] weights = new int[graph.size()]; Arrays.fill(weights, 1); GraphUtils.splitIrreducibleGraph(graph, weights, backend); Graph result = backend.getGraph(); - assertTrue("Should be irreducible", GraphUtils.isIrreducible(graph)); - assertFalse("Should be reducible", GraphUtils.isIrreducible(result)); - assertTrue("Should be equivalent", isEquivalent(backend, graph)); + check(graph, result, backend); } @Test @@ -219,15 +213,13 @@ public class GraphTest { builder.addEdge(4, 1); Graph graph = builder.build(); - DefaultGraphSplittingBackend backend = new DefaultGraphSplittingBackend(graph); + var backend = new DefaultGraphSplittingBackend(graph); int[] weights = new int[graph.size()]; Arrays.fill(weights, 1); GraphUtils.splitIrreducibleGraph(graph, weights, backend); Graph result = backend.getGraph(); - assertTrue("Should be irreducible", GraphUtils.isIrreducible(graph)); - assertFalse("Should be reducible", GraphUtils.isIrreducible(result)); - assertTrue("Should be equivalent", isEquivalent(backend, graph)); + check(graph, result, backend); } @Test @@ -287,15 +279,13 @@ public class GraphTest { addEdges(builder, 69, 23); Graph graph = builder.build(); - DefaultGraphSplittingBackend backend = new DefaultGraphSplittingBackend(graph); + var backend = new DefaultGraphSplittingBackend(graph); int[] weights = new int[graph.size()]; Arrays.fill(weights, 1); GraphUtils.splitIrreducibleGraph(graph, weights, backend); Graph result = backend.getGraph(); - assertTrue("Should be irreducible", GraphUtils.isIrreducible(graph)); - assertFalse("Should be reducible", GraphUtils.isIrreducible(result)); - assertTrue("Should be equivalent", isEquivalent(backend, graph)); + check(graph, result, backend); } @Test @@ -316,15 +306,13 @@ public class GraphTest { addEdges(builder, 12, 1); Graph graph = builder.build(); - DefaultGraphSplittingBackend backend = new DefaultGraphSplittingBackend(graph); + var backend = new DefaultGraphSplittingBackend(graph); int[] weights = new int[graph.size()]; Arrays.fill(weights, 1); GraphUtils.splitIrreducibleGraph(graph, weights, backend); Graph result = backend.getGraph(); - assertTrue("Should be irreducible", GraphUtils.isIrreducible(graph)); - assertFalse("Should be reducible", GraphUtils.isIrreducible(result)); - assertTrue("Should be equivalent", isEquivalent(backend, graph)); + check(graph, result, backend); } @Test @@ -371,9 +359,46 @@ public class GraphTest { GraphUtils.splitIrreducibleGraph(graph, weights, backend); var result = backend.getGraph(); - assertTrue("Should be irreducible", GraphUtils.isIrreducible(graph)); + check(graph, result, backend); + } + + @Test + public void irreducibleGraphSplit8() { + var builder = new GraphBuilder(); + addEdges(builder, 0, 2, 1); + addEdges(builder, 1, 2, 3); + addEdges(builder, 2, 4); + addEdges(builder, 3, 9); + addEdges(builder, 4, 5); + addEdges(builder, 5, 6, 7); + addEdges(builder, 6, 4); + addEdges(builder, 7, 8, 10); + addEdges(builder, 8, 7, 10, 9); + addEdges(builder, 9, 8, 4); + + var graph = builder.build(); + var backend = new DefaultGraphSplittingBackend(graph); + var weights = new int[] { 3, 2, 1, 9, 34, 6, 1, 6, 8, 24, 3 }; + GraphUtils.splitIrreducibleGraph(graph, weights, backend); + var result = backend.getGraph(); + + check(graph, result, backend); + } + + private void check(Graph original, Graph result, DefaultGraphSplittingBackend backend) { + assertTrue("Should be irreducible", GraphUtils.isIrreducible(original)); assertFalse("Should be reducible", GraphUtils.isIrreducible(result)); - assertTrue("Should be equivalent", isEquivalent(backend, graph)); + assertTrue("Should be equivalent", isEquivalent(backend, original)); + assertFalse("Should not have disconnected nodes", hasDisconnectedNodes(result)); + } + + private boolean hasDisconnectedNodes(Graph graph) { + for (var i = 1; i < graph.size(); ++i) { + if (graph.incomingEdgesCount(i) == 0) { + return true; + } + } + return false; } private static void addEdges(GraphBuilder builder, int from, int... to) {