Commit 57b023c1 authored by Fabien Viale's avatar Fabien Viale
Browse files

Remove and comment recursive algorithm in InternalJob

Replaced by a simple iterative search

After analysis of the code, it tries to find an InternalTask based on its name, but to do so, it does a complex  potentially exponential recursive algorithm. This code hanged a task termination on a workflow containing a loop which had around 1400 tasks.

For now I comment the previous code to keep its track.
parent 1ba5c072
......@@ -635,37 +635,48 @@ public abstract class InternalJob extends JobState {
return true;
}
/**
* Walk up <code>down</code>'s dependences until a task <code>name</code> is
* met
* <p>
* also walks weak references created by {@link FlowActionType#IF}
*
* @return the task names <code>name</code>, or null
*/
public InternalTask findTaskUp(String name, InternalTask down) {
InternalTask ret = null;
List<InternalTask> ideps = new ArrayList<>();
if (down.getIDependences() != null) {
ideps.addAll(down.getIDependences());
}
if (down.getJoinedBranches() != null) {
ideps.addAll(down.getJoinedBranches());
}
if (down.getIfBranch() != null) {
ideps.add(down.getIfBranch());
}
for (InternalTask up : ideps) {
if (up.getName().equals(name)) {
ret = up;
} else {
InternalTask r = findTaskUp(name, up);
if (r != null) {
ret = r;
}
// Recursive algorithm is causing issues on loops with a large number of tasks
// /**
// * Walk up <code>down</code>'s dependences until a task <code>name</code> is
// * met
// * <p>
// * also walks weak references created by {@link FlowActionType#IF}
// *
// * @return the task names <code>name</code>, or null
// */
// public InternalTask findTaskUp(String name, InternalTask down) {
// InternalTask ret = null;
// List<InternalTask> ideps = new ArrayList<>();
// if (down.getIDependences() != null) {
// ideps.addAll(down.getIDependences());
// }
// if (down.getJoinedBranches() != null) {
// ideps.addAll(down.getJoinedBranches());
// }
// if (down.getIfBranch() != null) {
// ideps.add(down.getIfBranch());
// }
// for (InternalTask up : ideps) {
// if (up.getName().equals(name)) {
// ret = up;
// } else {
// InternalTask r = findTaskUp(name, up);
// if (r != null) {
// ret = r;
// }
// }
// }
// return ret;
// }
// replaced findTaskUp call due to recursion issues
public InternalTask findTask(String name) {
for (InternalTask task : tasks.values()) {
if (name.equals(task.getName())) {
return task;
}
}
return ret;
return null;
}
/**
......
......@@ -170,7 +170,7 @@ public class TerminateIfTaskHandler {
targetElse = it;
}
} else if (action.getTargetContinuation().equals(it.getName())) {
InternalTask up = internalJob.findTaskUp(initiator.getName(), it);
InternalTask up = internalJob.findTask(initiator.getName());
if (up != null && up.equals(initiator)) {
targetJoin = it;
}
......
......@@ -57,7 +57,7 @@ public class TerminateLoopHandler {
if (action.getTarget().equals(initiator.getName())) {
target = initiator;
} else {
target = internalJob.findTaskUp(action.getTarget(), initiator);
target = internalJob.findTask(action.getTarget());
}
boolean replicateForNextLoopIteration = internalJob.replicateForNextLoopIteration(initiator,
target,
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment