diff --git a/hot-reload-agent/src/main/kotlin/org/jetbrains/compose/reload/agent/staticsInitialization.kt b/hot-reload-agent/src/main/kotlin/org/jetbrains/compose/reload/agent/staticsInitialization.kt index 53e8099..04531d9 100644 --- a/hot-reload-agent/src/main/kotlin/org/jetbrains/compose/reload/agent/staticsInitialization.kt +++ b/hot-reload-agent/src/main/kotlin/org/jetbrains/compose/reload/agent/staticsInitialization.kt @@ -7,7 +7,7 @@ import org.jetbrains.compose.reload.analysis.classId import org.jetbrains.compose.reload.analysis.classInitializerMethodId import org.jetbrains.compose.reload.analysis.resolveInvalidationKey import org.jetbrains.compose.reload.core.createLogger -import org.jetbrains.compose.reload.core.topologicalSort +import org.jetbrains.compose.reload.core.sortedByTopology import java.lang.instrument.ClassDefinition import java.lang.reflect.Modifier @@ -81,9 +81,9 @@ internal fun reinitializeStaticsIfNecessary( Step 2: Let us ensure we're executing the initializers in a reasonable order: if class 'A' depends on 'B' */ - val topologicallySortedDirtyClasses = dirtyClasses.topologicalSort( + val topologicallySortedDirtyClasses = dirtyClasses.sortedByTopology( onCycle = { logger.warn(" cycle detected: ${it.joinToString(", ") { it.simpleName }}") }, - follows = { clazz -> classInitializerDependencies[clazz].orEmpty() } + edges = { clazz -> classInitializerDependencies[clazz].orEmpty() } ) /** diff --git a/hot-reload-core/src/benchmark/kotlin/org/jetbrains/compose/reload/core/TopologicalSortBenchmark.kt b/hot-reload-core/src/benchmark/kotlin/org/jetbrains/compose/reload/core/TopologicalSortBenchmark.kt index 3e6efdd..6f2c553 100644 --- a/hot-reload-core/src/benchmark/kotlin/org/jetbrains/compose/reload/core/TopologicalSortBenchmark.kt +++ b/hot-reload-core/src/benchmark/kotlin/org/jetbrains/compose/reload/core/TopologicalSortBenchmark.kt @@ -11,7 +11,7 @@ import kotlin.random.Random @BenchmarkMode(Mode.AverageTime) open class TopologicalSortBenchmark { - @Param("10", "1000", "10000") + @Param("10", "1000", "10000", "100000") var nodesCount: Int = 0 lateinit var nodes: List @@ -28,6 +28,7 @@ open class TopologicalSortBenchmark { } override fun equals(other: Any?): Boolean { + if (other === this) return true return other is Node && other.name == name } } @@ -45,11 +46,11 @@ open class TopologicalSortBenchmark { @Benchmark fun topologicalSort(): List { - return nodes.topologicalSort { it.dependencies } + return nodes.sortedByTopology { it.dependencies } } @Benchmark fun defaultSort(): List { return nodes.sortedBy { it.dependencies.hashCode() } } -} \ No newline at end of file +} diff --git a/hot-reload-core/src/main/kotlin/org/jetbrains/compose/reload/core/sort.kt b/hot-reload-core/src/main/kotlin/org/jetbrains/compose/reload/core/sort.kt deleted file mode 100644 index f24b785..0000000 --- a/hot-reload-core/src/main/kotlin/org/jetbrains/compose/reload/core/sort.kt +++ /dev/null @@ -1,84 +0,0 @@ -package org.jetbrains.compose.reload.core - -public inline fun Iterable.topologicalSort( - noinline onCycle: ((cycle: Iterable) -> Unit)? = null, - noinline follows: (T) -> Iterable -): List { - - val incomingEdges = incomingEdges(follows) - val closures by lazy(LazyThreadSafetyMode.NONE) { associateWith { element -> element.closure(follows).toMutableList() } } - val result = mutableListOf() - - while (incomingEdges.isNotEmpty()) { - val destinations = run { - val roots = incomingEdges.filter { (_, sources) -> sources.isEmpty() }.keys - if (roots.isNotEmpty()) return@run roots - - /* Handle cycles */ - val next = resolveCircle(incomingEdges, closures) - if (onCycle != null) { - onCycle(next.findCircle(follows)) - } - - listOf(next) - } - - result.addAll(destinations) - destinations.forEach { destination -> incomingEdges.remove(destination) } - incomingEdges.values.forEach { sources -> sources.removeAll(destinations) } - } - - return result.toList() -} - -@PublishedApi -internal inline fun resolveCircle( - incomingEdges: MutableMap>, closures: Map> -): T { - return incomingEdges.keys.maxByOrNull { element -> closures.getValue(element).size } ?: error("no elements") -} - -@PublishedApi -internal fun T.findCircle(follows: (T) -> Iterable): List { - val stack = ArrayDeque() - fun search(element: T): List? { - stack.add(element) - if (stack.size > 1 && element == this) return stack.toList() - - follows(element).forEach { next -> - search(next)?.let { return it } - } - - stack.removeLast() - return null - } - - return search(this).orEmpty() -} - -@PublishedApi -internal fun Iterable.incomingEdges( - follows: (T) -> Iterable -): MutableMap> { - - val incomingEdges = mutableMapOf>() - forEach { node -> incomingEdges.getOrPut(node) { mutableSetOf() } } - if (incomingEdges.isEmpty()) return incomingEdges - - val queue = ArrayDeque() - val visited = hashSetOf() - queue.addAll(this) - - while (queue.isNotEmpty()) { - val source = queue.removeFirst() - if (!visited.add(source)) continue - - val destinations = follows(source) - queue.addAll(destinations) - - destinations.forEach { destination -> - incomingEdges.getValue(destination).add(source) - } - } - return incomingEdges -} diff --git a/hot-reload-core/src/main/kotlin/org/jetbrains/compose/reload/core/topologicalSort.kt b/hot-reload-core/src/main/kotlin/org/jetbrains/compose/reload/core/topologicalSort.kt new file mode 100644 index 0000000..c27ff64 --- /dev/null +++ b/hot-reload-core/src/main/kotlin/org/jetbrains/compose/reload/core/topologicalSort.kt @@ -0,0 +1,192 @@ +package org.jetbrains.compose.reload.core + +/** + * Will sort a graph by its topology, provided in [edges]. + * For example: + * + * ``` + * /** + * * a + * * | \ + * * b c – + + * * | \ \ + * * d e f + * * \ + * * g + * */ + * ``` + * + * Will sort + * `from: c, e, a, g, b, f, d` + * `to: a, c, b, e, f, d, g` + * + * Note: + * Duplicates in [this] iterable will be removed. The returned List will mention the node only once! + * + * Note 2: + * Cycles in the graph are allowed. + * Detected cycles will be reported using the [onCycle] callback. + * Cycles will be resolved by breaking the edge which breaks the fewest dependencies. + */ +public fun Iterable.sortedByTopology( + onCycle: ((cycle: Iterable) -> Unit)? = null, + edges: (T) -> Iterable +): List { + val set = this as? Set ?: toSet() + val nodes = set.buildNodes(edges).ranked() + val result = ArrayList(set.size) + + var roots = nodes.filter { it.isRoot() } + while (nodes.isNotEmpty()) { + val newRoots = mutableListOf>() + + roots.forEach { root -> + nodes.remove(root) + if (root.isVisible) { + result.add(root.value) + } + newRoots.addAll(root.release()) + } + newRoots.sortBy { it.originalOrdinal } + + /* Handle cycles */ + if (newRoots.isEmpty() && nodes.isNotEmpty()) { + val next: Node = nodes.first() + if (onCycle != null) { + onCycle(next.value.findCircle(edges)) + } + + newRoots.add(next) + } + + roots = newRoots + } + return result +} + +private fun T.findCircle(edges: (T) -> Iterable): List { + val stack = ArrayDeque() + fun search(element: T): List? { + stack.add(element) + if (stack.size > 1 && element == this) return stack.toList() + + edges(element).forEach { next -> + search(next)?.let { return it } + } + + stack.removeLast() + return null + } + + return search(this).orEmpty() +} + +private fun List>.ranked(): MutableSet> { + val ranks = hashMapOf, Int>() + val inStack = hashSetOf>() + + val rank = DeepRecursiveFunction rank@{ node: Node -> + if (!inStack.add(node)) return@rank 0 + ranks[node]?.let { return@rank it } + + val rank = node.children.sumOf { callRecursive(it) } + 1 + ranks[node] = rank + inStack.remove(node) + return@rank rank + } + + forEach { node -> rank(node) } + + val ranked = sortedByDescending { node -> ranks[node] } + return ranked.toMutableSet() +} + +private fun Set.buildNodes( + edges: (T) -> Iterable +): List> { + if (this.isEmpty()) return emptyList() + + val nodes = mapIndexedTo(ArrayList(size + 16)) { index, value -> Node(value, index) } + val nodesMap = nodes.associateByTo(LinkedHashMap(nodes.size + 16)) { it.value } + + val queue = ArrayDeque>() + val visited = hashSetOf>() + queue.addAll(nodes) + + while (queue.isNotEmpty()) { + val sourceNode = queue.removeFirst() + if (!visited.add(sourceNode)) continue + + val destinations = edges(sourceNode.value).toList() + + destinations.forEach { destination -> + val destinationNode = nodesMap.getOrPut(destination) { + val newNode = Node(destination, -1, isVisible = false) + nodes.add(newNode) + newNode + } + + if (destinationNode.originalOrdinal == -1 && sourceNode.originalOrdinal >= 0) { + destinationNode.originalOrdinal = sourceNode.originalOrdinal + 1 + } + + queue.add(destinationNode) + sourceNode.addChild(destinationNode) + } + } + + return nodes +} + +private class Node( + val value: T, + /** + * If the value is present in the 'to be sorted' collection, then this ordinal will be the + * index of the element. If the node is not present in said collection, then this will be inferred + * by using the ordinal of the first source node pointing to this 'invisible' node. + */ + var originalOrdinal: Int, + + /** + * true, if the node is present in the List of elements to be sorted (and therefore visible in the results). + * false, if the node is not present but was found in the edges of another node + */ + val isVisible: Boolean = true, +) { + + var isReleased = false + private set + + var incomingEdgesCounter = 0 + private set + + val children = mutableListOf>() + + fun isRoot() = incomingEdgesCounter == 0 + + fun addChild(node: Node) { + children.add(node) + node.incomingEdgesCounter++ + } + + fun release(): List> { + if (isReleased) { + error("Already released this node: $value") + } + isReleased = true + + val newRoots = mutableListOf>() + children.forEach { child -> + child.incomingEdgesCounter-- + if (child.isRoot() && !child.isReleased) { + newRoots.add(child) + } + } + children.clear() + return newRoots.toList() + } + + override fun toString(): String { + return value.toString() + } +} diff --git a/hot-reload-core/src/test/kotlin/TopologicalSortTest.kt b/hot-reload-core/src/test/kotlin/TopologicalSortTest.kt index 12ff2d7..149032e 100644 --- a/hot-reload-core/src/test/kotlin/TopologicalSortTest.kt +++ b/hot-reload-core/src/test/kotlin/TopologicalSortTest.kt @@ -1,4 +1,4 @@ -import org.jetbrains.compose.reload.core.topologicalSort +import org.jetbrains.compose.reload.core.sortedByTopology import kotlin.random.Random import kotlin.test.Test import kotlin.test.assertEquals @@ -23,9 +23,9 @@ class TopologicalSortTest { b.dependencies.add(c) val expected = listOf(a, b, c) - assertEquals(expected, listOf(a, b, c).topologicalSort { it.dependencies }) - assertEquals(expected, listOf(c, b, a).topologicalSort { it.dependencies }) - assertEquals(expected, listOf(b, c, a).topologicalSort { it.dependencies }) + assertEquals(expected, listOf(a, b, c).sortedByTopology { it.dependencies }) + assertEquals(expected, listOf(c, b, a).sortedByTopology { it.dependencies }) + assertEquals(expected, listOf(b, c, a).sortedByTopology { it.dependencies }) } /** @@ -59,17 +59,52 @@ class TopologicalSortTest { assertEquals( listOf(a, b, c, d, e, f, g), - listOf(a, b, c, d, e, f, g).topologicalSort { it.dependencies } + listOf(a, b, c, d, e, f, g).sortedByTopology { it.dependencies } ) assertEquals( listOf(a, c, b, f, e, d, g), - listOf(g, f, e, d, c, b, a).topologicalSort { it.dependencies } + listOf(g, f, e, d, c, b, a).sortedByTopology { it.dependencies } ) assertEquals( listOf(a, c, b, e, f, d, g), - listOf(c, e, a, g, b, f, d).topologicalSort { it.dependencies } + listOf(c, e, a, g, b, f, d).sortedByTopology { it.dependencies } + ) + } + + /** + * a + * | \ + * b c – + + * | \ \ + * d e f + * \ + * g + */ + @Test + fun `test - simple graph - with duplicate entries`() { + val a = Node("a") + val b = Node("b") + val c = Node("c") + val d = Node("d") + val e = Node("e") + val f = Node("f") + val g = Node("g") + + a.dependencies.add(b) + a.dependencies.add(c) + + b.dependencies.add(d) + + c.dependencies.add(e) + c.dependencies.add(f) + + e.dependencies.add(g) + + assertEquals( + listOf(a, c, b, e, f, d, g), + listOf(c, e, a, a, g, b, f, a, d).sortedByTopology { it.dependencies } ) } @@ -95,12 +130,12 @@ class TopologicalSortTest { assertEquals( listOf(a, b, c, d), - listOf(a, b, c, d).topologicalSort { it.dependencies } + listOf(a, b, c, d).sortedByTopology { it.dependencies } ) assertEquals( listOf(b, d, a, c), - listOf(d, c, b, a).topologicalSort { it.dependencies } + listOf(d, c, b, a).sortedByTopology { it.dependencies } ) } @@ -133,17 +168,17 @@ class TopologicalSortTest { assertEquals( listOf(a, b, c, d, e, f), - listOf(a, b, c, d, e, f).topologicalSort { it.dependencies } + listOf(a, b, c, d, e, f).sortedByTopology { it.dependencies } ) assertEquals( listOf(d, f, e, a, c, b), - listOf(f, e, d, c, b, a).topologicalSort { it.dependencies } + listOf(f, e, d, c, b, a).sortedByTopology { it.dependencies } ) run { var cycle = emptyList() - listOf(f, e, d, c, b, a).topologicalSort(onCycle = { cycle = it.toList() }) { it.dependencies } + listOf(f, e, d, c, b, a).sortedByTopology(onCycle = { cycle = it.toList() }) { it.dependencies } assertEquals(listOf(d, a, b, d), cycle) } @@ -152,13 +187,46 @@ class TopologicalSortTest { assertEquals( listOf(a, c, b, d, f, e), - listOf(a, f, d, e, c, b).topologicalSort(onCycle = { cycle = it.toList() }) { it.dependencies } + listOf(a, f, d, e, c, b).sortedByTopology(onCycle = { cycle = it.toList() }) { it.dependencies } ) assertEquals(listOf(a, b, d, a), cycle) } } + /** + * +---> a + * | / \ + * | b c + * | / + * d + * / \ + * e f + */ + @Test + fun `test - cycle 2 - with duplicate entries`() { + val a = Node("a") + val b = Node("b") + val c = Node("c") + val d = Node("d") + val e = Node("e") + val f = Node("f") + + a.dependencies.add(b) + a.dependencies.add(c) + + b.dependencies.add(d) + + d.dependencies.add(a) + d.dependencies.add(e) + d.dependencies.add(f) + + assertEquals( + listOf(d, f, e, a, c, b), + listOf(f, e, f, d, c, b, a, b, d).sortedByTopology { it.dependencies } + ) + } + /** * +-> a * | / \ @@ -189,7 +257,7 @@ class TopologicalSortTest { var cycle = emptyList() assertEquals( listOf(a, c, x, b, d), - listOf(a, b, c, d, x).topologicalSort(onCycle = { cycle = it.toList() }) { it.dependencies } + listOf(a, b, c, d, x).sortedByTopology(onCycle = { cycle = it.toList() }) { it.dependencies } ) assertEquals(listOf(a, b, a), cycle) @@ -209,12 +277,31 @@ class TopologicalSortTest { val b = Node("b") val c = Node("c") val d = Node("d") + val x = Node("x") a.dependencies.add(b) a.dependencies.add(c) c.dependencies.add(b) + c.dependencies.add(d) + b.dependencies.add(x) + b.dependencies.add(a) + + assertEquals( + listOf(a, c, b, d, x), + listOf(a, b, c, d, x).sortedByTopology { it.dependencies } + ) + + assertEquals( + listOf(a, c, d, b, x), + listOf(a, x, c, d, b).sortedByTopology { it.dependencies } + ) + + assertEquals( + listOf(a, d, b), + listOf(a, d, b).sortedByTopology { it.dependencies } + ) } /** @@ -239,17 +326,65 @@ class TopologicalSortTest { assertEquals( listOf(a, x, b, c, y, z), - listOf(a, b, c, x, y, z).topologicalSort { it.dependencies } + listOf(a, b, c, x, y, z).sortedByTopology { it.dependencies } ) assertEquals( listOf(x, a, y, z, b, c), - listOf(x, y, z, a, b, c).topologicalSort { it.dependencies } + listOf(x, y, z, a, b, c).sortedByTopology { it.dependencies } ) assertEquals( listOf(x, a, y, b, z, c), - listOf(y, b, x, a, z, c).topologicalSort { it.dependencies } + listOf(y, b, x, a, z, c).sortedByTopology { it.dependencies } + ) + } + + /** + * a <==> a + */ + @Test + fun `test - self-dependency`() { + val a = Node("a") + a.dependencies.add(a) + + var cycle = emptyList() + listOf(a).sortedByTopology(onCycle = { cycle = it.toList() }) { it.dependencies } + + assertEquals(listOf(a, a), cycle) + } + + /** + * a + * | \ + * -b- c + * | + * d + */ + @Test + fun `test - sort subset of graph nodes`() { + val a = Node("a") + val b = Node("b") + val c = Node("c") + val d = Node("d") + + a.dependencies.add(b) + a.dependencies.add(c) + b.dependencies.add(d) + + assertEquals( + listOf(a, c, d), + listOf(d, c, a).sortedByTopology { it.dependencies } + ) + + assertEquals( + listOf(a, c, d), + listOf(a, d, c).sortedByTopology { it.dependencies } + ) + + assertEquals( + listOf(a, c, d), + listOf(c, d, a).sortedByTopology { it.dependencies } ) } @@ -282,27 +417,31 @@ class TopologicalSortTest { assertEquals( listOf(x, a, b, c, d), - listOf(x, a, b, c, d).topologicalSort { it.dependencies } + listOf(x, a, b, c, d).sortedByTopology { it.dependencies } ) } + @Test fun `test - smoke 1`() = runSmokeTest(1902) @Test fun `test - smoke 2`() = runSmokeTest(2411) + @Test + fun `test - smoke 3`() = runSmokeTest(0, 100000) + - private fun runSmokeTest(seed: Int) { + private fun runSmokeTest(seed: Int, size: Int = 1024) { val random = Random(seed) - val nodes = buildList { repeat(1024) { add(Node("Node $it")) } } + val nodes = buildList { repeat(size) { add(Node("Node $it")) } } nodes.forEach { node -> repeat(random.nextInt(0, 12)) { node.dependencies.add(nodes.random(random)) } } - val sorted = nodes.topologicalSort { it.dependencies } + val sorted = nodes.sortedByTopology { it.dependencies } assertEquals(nodes.size, sorted.size) assertEquals(nodes.toSet(), sorted.toSet()) }