From feea8ef3823978861d46dffbfc203f3992257063 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=93scar=20Gonz=C3=A1lez=20Fern=C3=A1ndez?= Date: Sat, 14 May 2011 19:47:49 +0200 Subject: [PATCH] [Bug #1070] Fix problem in GanttDiagramGraph getRecalculationsNeeded took a huge amount of time when tasks can be reached from several paths in complex graphs. In the case a task is reached from another path the recalculation for that task was removed from the result and added to the pending queue. Thus it was guaranteed that the result was a topological order, since recalculations would be pushed to the end. But the recalculations dependent of the already added were still in the pending queue and executed. This caused more removals from the result and subsequent additions to the pending queue. Now a topological order is applied to the recalculations calculated. For each task point a depth value is calculated. A topological order is necessary, so a recalculation is executed after all its predecessors. FEA: ItEr74S04BugFixing --- .../org/zkoss/ganttz/data/DependencyType.java | 11 + .../zkoss/ganttz/data/GanttDiagramGraph.java | 260 +++++++++++++++--- 2 files changed, 229 insertions(+), 42 deletions(-) diff --git a/ganttzk/src/main/java/org/zkoss/ganttz/data/DependencyType.java b/ganttzk/src/main/java/org/zkoss/ganttz/data/DependencyType.java index 4859a9949..7807fd9b3 100644 --- a/ganttzk/src/main/java/org/zkoss/ganttz/data/DependencyType.java +++ b/ganttzk/src/main/java/org/zkoss/ganttz/data/DependencyType.java @@ -75,6 +75,17 @@ public enum DependencyType { public enum Point { VOID, START, END; + + public Point getOther() { + switch (this) { + case START: + return END; + case END: + return START; + default: + return VOID; + } + } } private final Point source; diff --git a/ganttzk/src/main/java/org/zkoss/ganttz/data/GanttDiagramGraph.java b/ganttzk/src/main/java/org/zkoss/ganttz/data/GanttDiagramGraph.java index 0f18bd59c..a132b02ad 100644 --- a/ganttzk/src/main/java/org/zkoss/ganttz/data/GanttDiagramGraph.java +++ b/ganttzk/src/main/java/org/zkoss/ganttz/data/GanttDiagramGraph.java @@ -26,6 +26,7 @@ import static java.util.Arrays.asList; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; +import java.util.Comparator; import java.util.EnumSet; import java.util.HashMap; import java.util.HashSet; @@ -33,7 +34,6 @@ import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.LinkedList; import java.util.List; -import java.util.ListIterator; import java.util.Map; import java.util.Queue; import java.util.Set; @@ -250,6 +250,8 @@ public class GanttDiagramGraph> implements private final DirectedGraph graph; + private final TopologicalSorter topologicalSorter; + private List topLevelTasks = new ArrayList(); private Map fromChildToParent = new HashMap(); @@ -347,6 +349,7 @@ public class GanttDiagramGraph> implements this.globalEndConstraints = globalEndConstraints; this.dependenciesConstraintsHavePriority = dependenciesConstraintsHavePriority; this.graph = new SimpleDirectedGraph(adapter.getDependencyType()); + this.topologicalSorter = new TopologicalSorter(); } public void enforceAllRestrictions() { @@ -390,6 +393,96 @@ public class GanttDiagramGraph> implements } } + class TopologicalSorter { + + private Map taskPointsByDepth() { + Map result = new HashMap(); + Map> visitedBy = new HashMap>(); + + Queue withoutIncoming = getInitial(withoutVisibleIncomingDependencies(getTopLevelTasks())); + for (TaskPoint each : withoutIncoming) { + initializeIfNeededForKey(result, each, 0); + } + + while (!withoutIncoming.isEmpty()) { + TaskPoint current = withoutIncoming.poll(); + for (TaskPoint each : current.getImmediateSuccessors()) { + initializeIfNeededForKey(visitedBy, each, + new HashSet()); + Set visitors = visitedBy.get(each); + visitors.add(current); + Set predecessorsRequired = each + .getImmediatePredecessors(); + if (visitors.containsAll(predecessorsRequired)) { + initializeIfNeededForKey(result, each, + result.get(current) + 1); + withoutIncoming.offer(each); + } + } + } + return result; + } + + private void initializeIfNeededForKey(Map map, K key, + T initialValue) { + if (!map.containsKey(key)) { + map.put(key, initialValue); + } + } + + private LinkedList getInitial(List initial) { + LinkedList result = new LinkedList(); + for (V each : initial) { + result.add(allPointsPotentiallyModified(each)); + } + return result; + } + + public List sort( + Collection recalculationsToBeSorted) { + + List result = new ArrayList( + recalculationsToBeSorted); + final Map taskPointsByDepth = taskPointsByDepth(); + Collections.sort(result, new Comparator() { + + @Override + public int compare(Recalculation o1, Recalculation o2) { + int o1Depth = onNullDefault( + taskPointsByDepth.get(o1.taskPoint), + Integer.MAX_VALUE, "no depth value for " + + o1.taskPoint); + int o2Depth = onNullDefault( + taskPointsByDepth.get(o2.taskPoint), + Integer.MAX_VALUE, "no depth value for " + + o2.taskPoint); + int result = o1Depth - o2Depth; + if (result == 0) { + return asInt(o2.parentRecalculation) + - asInt(o1.parentRecalculation); + } + return result; + } + + private int asInt(boolean b) { + return b ? 1 : 0; + } + }); + return result; + } + } + + private static T onNullDefault(T value, T defaultValue, + String warnMessage) { + if (value == null) { + if (warnMessage != null) { + LOG.warn(warnMessage); + } + return defaultValue; + } + return value; + } + public void addTask(V original) { List stack = new LinkedList(); stack.add(original); @@ -900,37 +993,50 @@ public class GanttDiagramGraph> implements } List getRecalculationsNeededFrom(V task) { - List result = new LinkedList(); + List result = new ArrayList(); Set parentRecalculationsAlreadyDone = new HashSet(); Recalculation first = recalculationFor(allPointsPotentiallyModified(task)); first.couldHaveBeenModifiedBeforehand(); - Queue pendingOfNavigate = new LinkedList(); + result.addAll(getParentsRecalculations(parentRecalculationsAlreadyDone, first.taskPoint)); result.add(first); - pendingOfNavigate.offer(first); - while (!pendingOfNavigate.isEmpty()) { - Recalculation current = pendingOfNavigate.poll(); - for (TaskPoint each : current.taskPoint.getImmendiateReachable()) { - Recalculation recalculationToAdd = recalculationFor(each); - ListIterator listIterator = result - .listIterator(); - while (listIterator.hasNext()) { - Recalculation previous = listIterator.next(); - if (previous.equals(recalculationToAdd)) { - listIterator.remove(); - recalculationToAdd = previous; - break; - } + + Queue pendingOfVisit = new LinkedList(); + pendingOfVisit.offer(first); + + Map alreadyVisited = new HashMap(); + alreadyVisited.put(first, first); + + while (!pendingOfVisit.isEmpty()) { + Recalculation current = pendingOfVisit.poll(); + for (TaskPoint each : current.taskPoint.getImmediateSuccessors()) { + if (each.isImmediatelyDerivedFrom(current.taskPoint)) { + continue; } + Recalculation recalculationToAdd = getRecalcualtionToAdd(each, + alreadyVisited); recalculationToAdd.comesFromPredecessor(current); - result.addAll(getParentsRecalculations( - parentRecalculationsAlreadyDone, each)); - result.add(recalculationToAdd); - pendingOfNavigate.offer(recalculationToAdd); + if (!alreadyVisited.containsKey(recalculationToAdd)) { + result.addAll(getParentsRecalculations( + parentRecalculationsAlreadyDone, each)); + result.add(recalculationToAdd); + pendingOfVisit.offer(recalculationToAdd); + alreadyVisited.put(recalculationToAdd, recalculationToAdd); + } } } - return result; + return topologicalSorter.sort(result); + } + + private Recalculation getRecalcualtionToAdd(TaskPoint taskPoint, + Map alreadyVisited) { + Recalculation result = recalculationFor(taskPoint); + if (alreadyVisited.containsKey(result)) { + return alreadyVisited.get(result); + } else { + return result; + } } private List getParentsRecalculations( @@ -1875,18 +1981,18 @@ public class GanttDiagramGraph> implements } TaskPoint destinationPoint(D dependency) { - V source = getDependencyDestination(dependency); - return new TaskPoint(source, - potentiallyModifiedDestinationPoints(dependency.getType())); + V destination = getDependencyDestination(dependency); + return new TaskPoint(destination, + getDestinationPoint(dependency.getType())); } - private Set potentiallyModifiedDestinationPoints(DependencyType type) { - Point destinationPoint = getDestinationPoint(type); - if (isDominatingPoint(destinationPoint)) { - return EnumSet.of(Point.START, Point.END); - } else { - return EnumSet.of(destinationPoint); - } + private Point getDestinationPoint(DependencyType type) { + return type.getSourceAndDestination()[isScheduleForward() ? 1 : 0]; + } + + TaskPoint sourcePoint(D dependency) { + V source = getDependencySource(dependency); + return new TaskPoint(source, getSourcePoint(dependency.getType())); } /** @@ -1895,12 +2001,15 @@ public class GanttDiagramGraph> implements * start. */ private boolean isDominatingPoint(Point point) { - return isScheduleForward() && point == Point.START - || isScheduleBackwards() && point == Point.END; + return point == getDominatingPoint(); } - private Point getDestinationPoint(DependencyType type) { - return type.getSourceAndDestination()[isScheduleForward() ? 1 : 0]; + private Point getDominatingPoint() { + return isScheduleForward() ? Point.START : Point.END; + } + + private Point getSourcePoint(DependencyType type) { + return type.getSourceAndDestination()[isScheduleForward() ? 0 : 1]; } private V getDependencySource(D dependency) { @@ -1914,7 +2023,7 @@ public class GanttDiagramGraph> implements } TaskPoint allPointsPotentiallyModified(V task) { - return new TaskPoint(task, EnumSet.of(Point.START, Point.END)); + return new TaskPoint(task, getDominatingPoint()); } private class TaskPoint { @@ -1925,9 +2034,15 @@ public class GanttDiagramGraph> implements private final Set pointsModified; - TaskPoint(V task, Set pointsModified) { + private final Point entryPoint; + + TaskPoint(V task, Point entryPoint) { + Validate.notNull(task); + Validate.notNull(entryPoint); this.task = task; - this.pointsModified = Collections.unmodifiableSet(pointsModified); + this.entryPoint = entryPoint; + this.pointsModified = isDominatingPoint(entryPoint) ? EnumSet.of( + Point.START, Point.END) : EnumSet.of(entryPoint); this.isContainer = adapter.isContainer(task); } @@ -1974,7 +2089,7 @@ public class GanttDiagramGraph> implements pending.offer(this); while (!pending.isEmpty()) { TaskPoint current = pending.poll(); - Set immendiate = current.getImmendiateReachable(); + Set immendiate = current.getImmediateSuccessors(); for (TaskPoint each : immendiate) { if (!result.contains(each)) { result.add(each); @@ -1985,15 +2100,62 @@ public class GanttDiagramGraph> implements return result; } - private Set getImmendiateReachable() { + public boolean isImmediatelyDerivedFrom(TaskPoint other) { + return this.task.equals(other.task) + && other.pointsModified.containsAll(this.pointsModified); + } + + private Set cachedInmmediateSuccesors = null; + + public Set getImmediateSuccessors() { + if (cachedInmmediateSuccesors != null) { + return cachedInmmediateSuccesors; + } + Set result = new HashSet(); + result.addAll(getImmediatelyDerivedOnSameTask()); + Set candidates = immediateDependencies(); for (D each : candidates) { if (this.sendsModificationsThrough(each)) { result.add(destinationPoint(each)); } } - return result; + return cachedInmmediateSuccesors = Collections + .unmodifiableSet(result); + } + + private Set cachedImmediatePredecessors = null; + + public Set getImmediatePredecessors() { + if (cachedImmediatePredecessors != null) { + return cachedImmediatePredecessors; + } + Set result = new HashSet(); + if (!isDominatingPoint(entryPoint)) { + TaskPoint dominating = allPointsPotentiallyModified(task); + assert isDominatingPoint(dominating.entryPoint); + assert this.isImmediatelyDerivedFrom(dominating); + result.add(dominating); + } + for (D each : immediateIncomingDependencies()) { + if (this.receivesModificationsThrough(each)) { + TaskPoint sourcePoint = sourcePoint(each); + result.add(sourcePoint); + } + } + return cachedImmediatePredecessors = Collections + .unmodifiableSet(result); + } + + private Collection getImmediatelyDerivedOnSameTask() { + for (Point each : pointsModified) { + if (isDominatingPoint(each)) { + return Collections.singletonList(new TaskPoint(task, each + .getOther())); + } + } + return Collections.emptyList(); } private Set immediateDependencies() { @@ -2001,6 +2163,11 @@ public class GanttDiagramGraph> implements : graph.incomingEdgesOf(this.task); } + private Set immediateIncomingDependencies() { + return isScheduleForward() ? graph.incomingEdgesOf(this.task) + : graph.outgoingEdgesOf(this.task); + } + public boolean sendsModificationsThrough(D dependency) { V source = getDependencySource(dependency); Point dependencySourcePoint = getSourcePoint(adapter @@ -2015,6 +2182,15 @@ public class GanttDiagramGraph> implements Point[] sourceAndDestination = type.getSourceAndDestination(); return sourceAndDestination[isScheduleForward() ? 0 : 1]; } + + private boolean receivesModificationsThrough(D dependency) { + V destination = getDependencyDestination(dependency); + Point destinationPoint = getDestinationPoint(adapter + .getType(dependency)); + + return destination.equals(task) && entryPoint == destinationPoint; + } + }