Skip to content

Commit 1467603

Browse files
committed
GoMod: Fix the algorithm for handling cycles
The previous algorithm exhibits the following issues when run on graphs with cycles, in particular when the amount of edges is large: 1. Cycles of length 1 lead to infinite recursion. 2. The algorithm is inefficient in terms of execution time and memory allocation, as it creates a copy of the `predecessorNodes` set per recursion. 3. When run against [1] the execution of `toPackageReferenceForest()` didn't finish within 15 minutes, causing high CPU load and memory consumption. If GoMod used the dependency graph format already, the solution would be as simple as calling `DependencyGraphBuilder.breakCycles()`. Fix the problem by copying the code of `DependencyGraphBuilder.breakCycles()` as a temporary quick fix. This saves effort, because refactoring GoMod to use the dependency graph is planned anyway [2], which will allow for removing that copied code again. [1] https://github.com/ossf/scorecard [2] #4249 Fixes #5627. Signed-off-by: Frank Viernau <[email protected]>
1 parent 918e64e commit 1467603

File tree

1 file changed

+52
-5
lines changed
  • analyzer/src/main/kotlin/managers

1 file changed

+52
-5
lines changed

Diff for: analyzer/src/main/kotlin/managers/GoMod.kt

+52-5
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import com.fasterxml.jackson.core.JsonFactory
2525
import com.fasterxml.jackson.databind.ObjectMapper
2626

2727
import java.io.File
28+
import java.util.LinkedList
2829
import java.util.SortedSet
2930

3031
import org.ossreviewtoolkit.analyzer.AbstractPackageManagerFactory
@@ -186,7 +187,7 @@ class GoMod(
186187
graph = graph.subgraph(vendorModules)
187188
}
188189

189-
return graph
190+
return graph.breakCycles()
190191
}
191192

192193
@JsonIgnoreProperties(ignoreUnknown = true)
@@ -373,14 +374,60 @@ private class Graph(private val nodeMap: MutableMap<Identifier, Set<Identifier>>
373374
idsWithoutVersion.first()
374375
}
375376

377+
private enum class NodeColor { WHITE, GRAY, BLACK }
378+
379+
/**
380+
* Return a copy of this Graph with edges removed so that no circle remains.
381+
* TODO: The code has been copied from DependencyGraphBuilder as a temporary solutions. Once GoMod is migrated to
382+
* use the dependency graph, this function can be dropped and the one from DependencyGraphBuilder can be re-used,
383+
* see also https://github.com/oss-review-toolkit/ort/issues/4249.
384+
*/
385+
fun breakCycles(): Graph {
386+
val outgoingEdgesForNodes = nodeMap.mapValuesTo(mutableMapOf()) { it.value.toMutableSet() }
387+
val color = outgoingEdgesForNodes.keys.associateWithTo(mutableMapOf()) { NodeColor.WHITE }
388+
389+
fun visit(u: Identifier) {
390+
color[u] = NodeColor.GRAY
391+
392+
val nodesClosingCircle = mutableSetOf<Identifier>()
393+
394+
outgoingEdgesForNodes[u].orEmpty().forEach { v ->
395+
if (color[v] == NodeColor.WHITE) {
396+
visit(v)
397+
} else if (color[v] == NodeColor.GRAY) {
398+
nodesClosingCircle += v
399+
}
400+
}
401+
402+
outgoingEdgesForNodes[u]?.removeAll(nodesClosingCircle)
403+
nodesClosingCircle.forEach { v ->
404+
logger.debug { "Removing edge: ${u.toCoordinates()} -> ${v.toCoordinates()}}." }
405+
}
406+
407+
color[u] = NodeColor.BLACK
408+
}
409+
410+
val queue = LinkedList(outgoingEdgesForNodes.keys)
411+
412+
while (queue.isNotEmpty()) {
413+
val v = queue.removeFirst()
414+
415+
if (color.getValue(v) != NodeColor.WHITE) continue
416+
417+
visit(v)
418+
}
419+
420+
return Graph(outgoingEdgesForNodes.mapValuesTo(mutableMapOf()) { it.value.toSet() })
421+
}
422+
376423
/**
377424
* Convert this [Graph] to a set of [PackageReference]s that spawn the dependency trees of the direct dependencies
378-
* of the given [root] package.
425+
* of the given [root] package. The graph must not contain any cycles, so [breakCycles] should be called before.
379426
*/
380427
fun toPackageReferenceForest(root: Identifier): SortedSet<PackageReference> {
381-
fun getPackageReference(id: Identifier, predecessorNodes: Set<Identifier> = emptySet()): PackageReference {
382-
val dependencies = nodeMap.getValue(id).filter { it !in predecessorNodes }.mapTo(sortedSetOf()) {
383-
getPackageReference(it, predecessorNodes + id)
428+
fun getPackageReference(id: Identifier): PackageReference {
429+
val dependencies = nodeMap.getValue(id).mapTo(sortedSetOf()) {
430+
getPackageReference(it)
384431
}
385432

386433
return PackageReference(

0 commit comments

Comments
 (0)