Skip to content

Commit c797b76

Browse files
author
Ben Romberg
committed
performance optimizations
1 parent 002e49a commit c797b76

File tree

8 files changed

+180
-73
lines changed

8 files changed

+180
-73
lines changed

pom.xml

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
<?xml version="1.0" encoding="UTF-8"?>
2-
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
2+
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
3+
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
34
<modelVersion>4.0.0</modelVersion>
45

56
<parent>
@@ -128,6 +129,11 @@
128129
<artifactId>jdepend</artifactId>
129130
<version>2.9.1</version>
130131
</dependency>
132+
<dependency>
133+
<groupId>net.sf.jgrapht</groupId>
134+
<artifactId>jgrapht</artifactId>
135+
<version>0.8.3</version>
136+
</dependency>
131137

132138
<dependency>
133139
<groupId>junit</groupId>
Lines changed: 32 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,57 +1,57 @@
11
package de.andrena.tools.nopackagecycles;
22

3-
import java.util.ArrayList;
43
import java.util.Collection;
5-
import java.util.HashSet;
4+
import java.util.Iterator;
65
import java.util.List;
76
import java.util.Set;
87

98
import jdepend.framework.JavaPackage;
109

10+
import org.jgrapht.DirectedGraph;
11+
import org.jgrapht.alg.StrongConnectivityInspector;
12+
import org.jgrapht.graph.DefaultDirectedGraph;
13+
import org.jgrapht.graph.DefaultEdge;
14+
1115
public class PackageCycleCollector {
1216

13-
public Set<JavaPackage> collectCycles(JavaPackage javaPackage) {
14-
return collectCycles(javaPackage, new ArrayList<JavaPackage>());
17+
public List<Set<JavaPackage>> collectCycles(List<JavaPackage> packages) {
18+
DirectedGraph<JavaPackage, DefaultEdge> graph = new DefaultDirectedGraph<JavaPackage, DefaultEdge>(
19+
DefaultEdge.class);
20+
addVerticesToGraph(packages, graph);
21+
addEdgesToGraph(packages, graph);
22+
return collectCycles(graph);
1523
}
1624

17-
private Set<JavaPackage> collectCycles(JavaPackage javaPackage, List<JavaPackage> visitedPackages) {
18-
if (visitedPackages.contains(javaPackage)) {
19-
return getPackageCycle(javaPackage, visitedPackages);
20-
}
21-
visitedPackages.add(javaPackage);
22-
return collectAllDependencyCycles(javaPackage, visitedPackages);
25+
private List<Set<JavaPackage>> collectCycles(DirectedGraph<JavaPackage, DefaultEdge> graph) {
26+
List<Set<JavaPackage>> stronglyConnectedSets = new StrongConnectivityInspector<JavaPackage, DefaultEdge>(graph)
27+
.stronglyConnectedSets();
28+
removeSingletonSets(stronglyConnectedSets);
29+
return stronglyConnectedSets;
2330
}
2431

25-
private Set<JavaPackage> collectAllDependencyCycles(JavaPackage javaPackage, List<JavaPackage> visitedPackages) {
26-
Set<JavaPackage> allCycles = new HashSet<JavaPackage>();
27-
for (JavaPackage dependencyPackage : getEfferents(javaPackage)) {
28-
Set<JavaPackage> dependencyPackageCycle = collectCycles(dependencyPackage, new ArrayList<JavaPackage>(
29-
visitedPackages));
30-
if (dependencyPackageCycle.contains(javaPackage)) {
31-
allCycles.addAll(dependencyPackageCycle);
32-
visitedPackages.addAll(dependencyPackageCycle);
32+
private void removeSingletonSets(List<Set<JavaPackage>> stronglyConnectedSets) {
33+
Iterator<Set<JavaPackage>> iterator = stronglyConnectedSets.iterator();
34+
while (iterator.hasNext()) {
35+
Set<JavaPackage> stronglyConnectedSet = iterator.next();
36+
if (stronglyConnectedSet.size() == 1) {
37+
iterator.remove();
3338
}
3439
}
35-
return allCycles;
3640
}
3741

3842
@SuppressWarnings("unchecked")
39-
private Collection<JavaPackage> getEfferents(JavaPackage javaPackage) {
40-
return javaPackage.getEfferents();
43+
private void addEdgesToGraph(List<JavaPackage> packages, DirectedGraph<JavaPackage, DefaultEdge> graph) {
44+
for (JavaPackage javaPackage : packages) {
45+
for (JavaPackage efferent : (Collection<JavaPackage>) javaPackage.getEfferents()) {
46+
graph.addEdge(javaPackage, efferent);
47+
}
48+
}
4149
}
4250

43-
private Set<JavaPackage> getPackageCycle(JavaPackage javaPackage, List<JavaPackage> visitedPackages) {
44-
Set<JavaPackage> packageCycle = new HashSet<JavaPackage>();
45-
boolean includeFollowingPackages = false;
46-
for (JavaPackage visitedPackage : visitedPackages) {
47-
if (visitedPackage.equals(javaPackage)) {
48-
includeFollowingPackages = true;
49-
}
50-
if (includeFollowingPackages) {
51-
packageCycle.add(visitedPackage);
52-
}
51+
private void addVerticesToGraph(List<JavaPackage> packages, DirectedGraph<JavaPackage, DefaultEdge> graph) {
52+
for (JavaPackage javaPackage : packages) {
53+
graph.addVertex(javaPackage);
5354
}
54-
return packageCycle;
5555
}
5656

5757
}

src/main/java/de/andrena/tools/nopackagecycles/PackageCycleOutput.java

Lines changed: 21 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -5,38 +5,48 @@
55
import java.util.ArrayList;
66
import java.util.Collection;
77
import java.util.Collections;
8-
import java.util.Comparator;
98
import java.util.List;
9+
import java.util.Set;
1010

1111
import jdepend.framework.JavaClass;
1212
import jdepend.framework.JavaPackage;
1313
import de.andrena.tools.nopackagecycles.CollectionOutput.Appender;
1414
import de.andrena.tools.nopackagecycles.CollectionOutput.StringProvider;
15+
import de.andrena.tools.nopackagecycles.comparator.JavaClassNameComparator;
16+
import de.andrena.tools.nopackagecycles.comparator.JavaPackageListComparator;
17+
import de.andrena.tools.nopackagecycles.comparator.JavaPackageNameComparator;
1518

1619
public class PackageCycleOutput {
1720

1821
private List<JavaPackage> packages;
1922
private StringBuilder output;
20-
private PackageCycleCollector packageCycleCollector = new PackageCycleCollector();
23+
private PackageCycleCollector packageCycleCollector;
2124

2225
public PackageCycleOutput(List<JavaPackage> packages) {
2326
this.packages = packages;
27+
packageCycleCollector = new PackageCycleCollector();
2428
}
2529

2630
public String getOutput() {
2731
output = new StringBuilder();
28-
orderByPackageName(packages);
29-
while (!packages.isEmpty()) {
30-
JavaPackage javaPackage = packages.get(0);
31-
packages.remove(javaPackage);
32-
List<JavaPackage> cyclicPackages = getCyclicPackages(javaPackage);
33-
if (!cyclicPackages.isEmpty()) {
34-
appendOutputForPackageCycle(cyclicPackages);
35-
}
32+
for (List<JavaPackage> cycle : collectAndSortCycles()) {
33+
appendOutputForPackageCycle(cycle);
3634
}
3735
return output.toString();
3836
}
3937

38+
private List<List<JavaPackage>> collectAndSortCycles() {
39+
List<Set<JavaPackage>> cycles = packageCycleCollector.collectCycles(packages);
40+
List<List<JavaPackage>> orderedCycles = new ArrayList<List<JavaPackage>>();
41+
for (Set<JavaPackage> cycle : cycles) {
42+
ArrayList<JavaPackage> cycleAsList = new ArrayList<JavaPackage>(cycle);
43+
Collections.sort(cycleAsList, JavaPackageNameComparator.INSTANCE);
44+
orderedCycles.add(cycleAsList);
45+
}
46+
Collections.sort(orderedCycles, JavaPackageListComparator.INSTANCE);
47+
return orderedCycles;
48+
}
49+
4050
private void appendOutputForPackageCycle(List<JavaPackage> cyclicPackages) {
4151
packages.removeAll(cyclicPackages);
4252
appendHeaderForPackageCycle(cyclicPackages);
@@ -48,7 +58,6 @@ private void appendOutputForPackageCycle(List<JavaPackage> cyclicPackages) {
4858
private void appendHeaderForPackageCycle(List<JavaPackage> cyclicPackages) {
4959
output.append("\n\n").append("Package-cycle found involving ");
5060
output.append(joinCollection(cyclicPackages, new StringProvider<JavaPackage>() {
51-
5261
public String provide(JavaPackage javaPackage) {
5362
return javaPackage.getName();
5463
}
@@ -82,7 +91,7 @@ private List<JavaClass> getOrderedDependentClasses(JavaPackage javaPackage, Java
8291
dependentClasses.add(javaClass);
8392
}
8493
}
85-
orderByClassName(dependentClasses);
94+
Collections.sort(dependentClasses, JavaClassNameComparator.INSTANCE);
8695
return dependentClasses;
8796
}
8897

@@ -102,26 +111,4 @@ public void append(JavaClass packageClass) {
102111
}, ", ");
103112
}
104113

105-
private void orderByClassName(List<JavaClass> dependentClasses) {
106-
Collections.sort(dependentClasses, new Comparator<JavaClass>() {
107-
public int compare(JavaClass class1, JavaClass class2) {
108-
return class1.getName().compareTo(class2.getName());
109-
}
110-
});
111-
}
112-
113-
private List<JavaPackage> getCyclicPackages(JavaPackage javaPackage) {
114-
List<JavaPackage> cyclicPackages = new ArrayList<JavaPackage>(packageCycleCollector.collectCycles(javaPackage));
115-
orderByPackageName(cyclicPackages);
116-
return cyclicPackages;
117-
}
118-
119-
private void orderByPackageName(List<JavaPackage> cyclicPackages) {
120-
Collections.sort(cyclicPackages, new Comparator<JavaPackage>() {
121-
public int compare(JavaPackage package1, JavaPackage package2) {
122-
return package1.getName().compareTo(package2.getName());
123-
}
124-
});
125-
}
126-
127114
}
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
package de.andrena.tools.nopackagecycles.comparator;
2+
3+
import java.util.Comparator;
4+
5+
import jdepend.framework.JavaClass;
6+
7+
public enum JavaClassNameComparator implements Comparator<JavaClass> {
8+
INSTANCE;
9+
10+
public int compare(JavaClass class1, JavaClass class2) {
11+
return class1.getName().compareTo(class2.getName());
12+
}
13+
}
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
package de.andrena.tools.nopackagecycles.comparator;
2+
3+
import java.util.Comparator;
4+
import java.util.List;
5+
6+
import jdepend.framework.JavaPackage;
7+
8+
public enum JavaPackageListComparator implements Comparator<List<JavaPackage>> {
9+
INSTANCE;
10+
11+
public int compare(List<JavaPackage> package1, List<JavaPackage> package2) {
12+
return package1.get(0).getName().compareTo(package2.get(0).getName());
13+
}
14+
}
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
package de.andrena.tools.nopackagecycles.comparator;
2+
3+
import java.util.Comparator;
4+
5+
import jdepend.framework.JavaPackage;
6+
7+
public enum JavaPackageNameComparator implements Comparator<JavaPackage> {
8+
INSTANCE;
9+
10+
public int compare(JavaPackage package1, JavaPackage package2) {
11+
return package1.getName().compareTo(package2.getName());
12+
}
13+
}
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
package de.andrena.tools.nopackagecycles;
2+
3+
import static org.hamcrest.MatcherAssert.assertThat;
4+
import static org.hamcrest.Matchers.empty;
5+
import static org.hamcrest.Matchers.is;
6+
7+
import java.util.ArrayList;
8+
import java.util.Collections;
9+
import java.util.HashSet;
10+
import java.util.List;
11+
import java.util.Set;
12+
13+
import jdepend.framework.JavaPackage;
14+
15+
import org.junit.Before;
16+
import org.junit.Test;
17+
18+
public class PackageCycleCollectorPerformanceTest {
19+
private static final int PACKAGE_MATRIX_SIZE = 20;
20+
private List<JavaPackage> allPackages;
21+
22+
@Before
23+
public void setUp() {
24+
allPackages = new ArrayList<JavaPackage>();
25+
}
26+
27+
@Test
28+
public void collectCycles_PerformanceTest() throws Exception {
29+
initializePackages();
30+
List<Set<JavaPackage>> cycles = new PackageCycleCollector().collectCycles(allPackages);
31+
assertThat(cycles, is(empty()));
32+
}
33+
34+
private void initializePackages() {
35+
JavaPackage rootPackage = createPackage("root");
36+
Set<JavaPackage> lastPackageLayer = Collections.singleton(rootPackage);
37+
for (int row = 0; row < PACKAGE_MATRIX_SIZE; row++) {
38+
lastPackageLayer = createNextPackageLayer(lastPackageLayer, row);
39+
}
40+
}
41+
42+
private Set<JavaPackage> createNextPackageLayer(Set<JavaPackage> lastPackageLayer, int row) {
43+
Set<JavaPackage> nextPackageLayer = new HashSet<JavaPackage>();
44+
for (int column = 0; column < PACKAGE_MATRIX_SIZE; column++) {
45+
JavaPackage layerPackage = createPackage("layer." + row + "." + column);
46+
nextPackageLayer.add(layerPackage);
47+
for (JavaPackage javaPackage : lastPackageLayer) {
48+
javaPackage.dependsUpon(layerPackage);
49+
}
50+
}
51+
return nextPackageLayer;
52+
}
53+
54+
private JavaPackage createPackage(String packageName) {
55+
JavaPackage javaPackage = new JavaPackage(packageName);
56+
allPackages.add(javaPackage);
57+
return javaPackage;
58+
}
59+
}

0 commit comments

Comments
 (0)