From 770efb7ecb73eb53112242f45cd0a5b45eb3c33e Mon Sep 17 00:00:00 2001 From: "sebastian.sellmair" Date: Tue, 22 Jun 2021 15:21:58 +0200 Subject: [PATCH] [Commonizer] Replace InputTargetsSelectorInterface with a simple function ^KT-47301 --- .../kotlin/commonizer/CommonizerQueue.kt | 8 ++-- .../kotlin/commonizer/InputTargetsSelector.kt | 40 ++++++------------- .../kotlin/commonizer/CommonizerQueueTest.kt | 6 +-- ...ectorTest.kt => SelectInputTargetsTest.kt} | 24 +++++------ 4 files changed, 30 insertions(+), 48 deletions(-) rename native/commonizer/tests/org/jetbrains/kotlin/commonizer/{DefaultInputTargetsSelectorTest.kt => SelectInputTargetsTest.kt} (72%) diff --git a/native/commonizer/src/org/jetbrains/kotlin/commonizer/CommonizerQueue.kt b/native/commonizer/src/org/jetbrains/kotlin/commonizer/CommonizerQueue.kt index 6998573349e..f76a120a55a 100644 --- a/native/commonizer/src/org/jetbrains/kotlin/commonizer/CommonizerQueue.kt +++ b/native/commonizer/src/org/jetbrains/kotlin/commonizer/CommonizerQueue.kt @@ -22,8 +22,7 @@ internal fun CommonizerQueue(parameters: CommonizerParameters): CommonizerQueue CommonizerQueue.Deserializer { deserializeTarget(parameters, target) } }, commonizer = { inputs, output -> commonizeTarget(parameters, inputs, output) }, - serializer = { declarations, outputTarget -> serializeTarget(parameters, declarations, outputTarget) }, - inputTargetsSelector = DefaultInputTargetsSelector + serializer = { declarations, outputTarget -> serializeTarget(parameters, declarations, outputTarget) } ) } @@ -33,7 +32,6 @@ internal class CommonizerQueue( private val deserializers: TargetDependent, private val commonizer: Commonizer, private val serializer: Serializer, - private val inputTargetsSelector: InputTargetsSelector ) { fun interface Deserializer { @@ -116,7 +114,7 @@ internal class CommonizerQueue( } private fun registerTargetDependencies(outputTarget: OutputCommonizerTarget) { - targetDependencies[outputTarget] = inputTargetsSelector(outputTargets + deserializers.targets, outputTarget) + targetDependencies[outputTarget] = selectInputTargets(outputTargets + deserializers.targets, outputTarget) } private fun removeTargetDependencies(target: OutputCommonizerTarget) { @@ -140,4 +138,4 @@ internal class CommonizerQueue( init { outputTargets.forEach(this::enqueue) } -} \ No newline at end of file +} diff --git a/native/commonizer/src/org/jetbrains/kotlin/commonizer/InputTargetsSelector.kt b/native/commonizer/src/org/jetbrains/kotlin/commonizer/InputTargetsSelector.kt index c4685407f7f..dae272adc47 100644 --- a/native/commonizer/src/org/jetbrains/kotlin/commonizer/InputTargetsSelector.kt +++ b/native/commonizer/src/org/jetbrains/kotlin/commonizer/InputTargetsSelector.kt @@ -8,34 +8,20 @@ package org.jetbrains.kotlin.commonizer import org.jetbrains.kotlin.commonizer.utils.isProperSubsetOf import org.jetbrains.kotlin.commonizer.utils.isSubsetOf -internal fun interface InputTargetsSelector { - operator fun invoke(inputTargets: Set, outputTarget: SharedCommonizerTarget): Set -} +internal fun selectInputTargets(inputTargets: Set, outputTarget: SharedCommonizerTarget): Set { + val subsetInputTargets = inputTargets + .filter { inputTarget -> inputTarget != outputTarget && inputTarget.allLeaves() isSubsetOf outputTarget.allLeaves() } + .sortedBy { it.allLeaves().size } -internal operator fun InputTargetsSelector.invoke( - parameters: CommonizerParameters, - outputTarget: SharedCommonizerTarget -): Set { - return invoke(parameters.outputTargets + parameters.targetProviders.targets, outputTarget) -} - -internal object DefaultInputTargetsSelector : InputTargetsSelector { - override fun invoke(inputTargets: Set, outputTarget: SharedCommonizerTarget): Set { - - val subsetInputTargets = inputTargets - .filter { inputTarget -> inputTarget != outputTarget && inputTarget.allLeaves() isSubsetOf outputTarget.allLeaves() } - .sortedBy { it.allLeaves().size } - - val disjointSubsetInputTargets = subsetInputTargets - .filter { inputTarget -> - subsetInputTargets.none { potentialSuperSet -> inputTarget.allLeaves() isProperSubsetOf potentialSuperSet.allLeaves() } - } - - return outputTarget.allLeaves().fold(setOf()) { selectedInputTargets, outputLeafTarget -> - if (outputLeafTarget in selectedInputTargets.allLeaves()) return@fold selectedInputTargets - selectedInputTargets + (disjointSubsetInputTargets.firstOrNull { inputTarget -> outputLeafTarget in inputTarget.allLeaves() } - ?: failedSelectingInputTargets(inputTargets, outputTarget)) + val disjointSubsetInputTargets = subsetInputTargets + .filter { inputTarget -> + subsetInputTargets.none { potentialSuperSet -> inputTarget.allLeaves() isProperSubsetOf potentialSuperSet.allLeaves() } } + + return outputTarget.allLeaves().fold(setOf()) { selectedInputTargets, outputLeafTarget -> + if (outputLeafTarget in selectedInputTargets.allLeaves()) return@fold selectedInputTargets + selectedInputTargets + (disjointSubsetInputTargets.firstOrNull { inputTarget -> outputLeafTarget in inputTarget.allLeaves() } + ?: failedSelectingInputTargets(inputTargets, outputTarget)) } } @@ -45,4 +31,4 @@ private fun failedSelectingInputTargets(inputTargets: Set, out "inputTargets=$inputTargets\n" + "missing leaf targets: ${outputTarget.allLeaves() - inputTargets.allLeaves()}" ) -} \ No newline at end of file +} diff --git a/native/commonizer/tests/org/jetbrains/kotlin/commonizer/CommonizerQueueTest.kt b/native/commonizer/tests/org/jetbrains/kotlin/commonizer/CommonizerQueueTest.kt index dd6870c7673..8974c0db1a9 100644 --- a/native/commonizer/tests/org/jetbrains/kotlin/commonizer/CommonizerQueueTest.kt +++ b/native/commonizer/tests/org/jetbrains/kotlin/commonizer/CommonizerQueueTest.kt @@ -25,7 +25,6 @@ class CommonizerQueueTest { ) { CommonizerQueue.Deserializer { null } }, commonizer = { _, _ -> null }, serializer = { _, _ -> }, - inputTargetsSelector = DefaultInputTargetsSelector ) assertEquals( @@ -104,7 +103,6 @@ class CommonizerQueueTest { CirRootNode(CommonizedGroup(0), storageManager.createNullableLazyValue { CirRoot.create(output) }) }, serializer = { _, _ -> }, - inputTargetsSelector = DefaultInputTargetsSelector ) queue.invokeAll() @@ -121,14 +119,14 @@ class CommonizerQueueTest { val abInvocation = commonizerInvocations.single { it.output == abOutputTarget } assertEquals( - DefaultInputTargetsSelector(providedTargets + outputTargets, abOutputTarget), + selectInputTargets(providedTargets + outputTargets, abOutputTarget), abInvocation.inputs.targets.toSet(), "Expected commonizer being invoked with selected targets for abInvocation" ) val abcInvocation = commonizerInvocations.single { it.output == abcOutputTarget } assertEquals( - DefaultInputTargetsSelector(providedTargets + outputTargets, abcOutputTarget), + selectInputTargets(providedTargets + outputTargets, abcOutputTarget), abcInvocation.inputs.targets.toSet(), "Expected commonizer being invoked with selected targets for abcInvocation" ) diff --git a/native/commonizer/tests/org/jetbrains/kotlin/commonizer/DefaultInputTargetsSelectorTest.kt b/native/commonizer/tests/org/jetbrains/kotlin/commonizer/SelectInputTargetsTest.kt similarity index 72% rename from native/commonizer/tests/org/jetbrains/kotlin/commonizer/DefaultInputTargetsSelectorTest.kt rename to native/commonizer/tests/org/jetbrains/kotlin/commonizer/SelectInputTargetsTest.kt index cf8f27deb9d..2de646a84c9 100644 --- a/native/commonizer/tests/org/jetbrains/kotlin/commonizer/DefaultInputTargetsSelectorTest.kt +++ b/native/commonizer/tests/org/jetbrains/kotlin/commonizer/SelectInputTargetsTest.kt @@ -10,14 +10,14 @@ import kotlin.test.assertEquals import kotlin.test.assertFailsWith import kotlin.test.assertTrue -class DefaultInputTargetsSelectorTest { +class SelectInputTargetsTest { @Test fun `missing leaf targets`() { val inputTargets = setOf(LeafCommonizerTarget("a"), LeafCommonizerTarget("b")) val exception = assertFailsWith { - DefaultInputTargetsSelector(inputTargets, parseCommonizerTarget("(a, b, c)") as SharedCommonizerTarget) + selectInputTargets(inputTargets, parseCommonizerTarget("(a, b, c)") as SharedCommonizerTarget) } assertTrue( @@ -43,17 +43,17 @@ class DefaultInputTargetsSelectorTest { assertEquals( setOf(LeafCommonizerTarget("a"), LeafCommonizerTarget("b")), - DefaultInputTargetsSelector(inputTargets, parseCommonizerTarget("(a, b)") as SharedCommonizerTarget) + selectInputTargets(inputTargets, parseCommonizerTarget("(a, b)") as SharedCommonizerTarget) ) assertEquals( setOf(LeafCommonizerTarget("a"), LeafCommonizerTarget("b"), LeafCommonizerTarget("c")), - DefaultInputTargetsSelector(inputTargets, parseCommonizerTarget("(a, b, c)") as SharedCommonizerTarget) + selectInputTargets(inputTargets, parseCommonizerTarget("(a, b, c)") as SharedCommonizerTarget) ) assertEquals( setOf(LeafCommonizerTarget("a"), LeafCommonizerTarget("b"), parseCommonizerTarget("(c, d)")), - DefaultInputTargetsSelector(inputTargets, parseCommonizerTarget("(a, b, c, d)") as SharedCommonizerTarget) + selectInputTargets(inputTargets, parseCommonizerTarget("(a, b, c, d)") as SharedCommonizerTarget) ) } @@ -70,17 +70,17 @@ class DefaultInputTargetsSelectorTest { assertEquals( setOf("(a, b, c, d)", "f").map(::parseCommonizerTarget).toSet(), - DefaultInputTargetsSelector(inputTargets, parseCommonizerTarget("(a, b, c, d, f)") as SharedCommonizerTarget) + selectInputTargets(inputTargets, parseCommonizerTarget("(a, b, c, d, f)") as SharedCommonizerTarget) ) assertEquals( setOf("(a, b, c, d)", "(c, d, e)").map(::parseCommonizerTarget).toSet(), - DefaultInputTargetsSelector(inputTargets, parseCommonizerTarget("(a, b, c, d, e)") as SharedCommonizerTarget) + selectInputTargets(inputTargets, parseCommonizerTarget("(a, b, c, d, e)") as SharedCommonizerTarget) ) assertEquals( setOf("(a, b, c, d)", "(c, d, e)", "f").map(::parseCommonizerTarget).toSet(), - DefaultInputTargetsSelector(inputTargets, parseCommonizerTarget("(a, b, c, d, e, f)") as SharedCommonizerTarget) + selectInputTargets(inputTargets, parseCommonizerTarget("(a, b, c, d, e, f)") as SharedCommonizerTarget) ) } @@ -88,7 +88,7 @@ class DefaultInputTargetsSelectorTest { fun `empty outputTarget`() { assertEquals( emptySet(), - DefaultInputTargetsSelector(emptySet(), parseCommonizerTarget("()") as SharedCommonizerTarget) + selectInputTargets(emptySet(), parseCommonizerTarget("()") as SharedCommonizerTarget) ) } @@ -96,7 +96,7 @@ class DefaultInputTargetsSelectorTest { fun `single leaf outputTarget`() { assertEquals( setOf(LeafCommonizerTarget("a")), - DefaultInputTargetsSelector(setOf(LeafCommonizerTarget("a")), parseCommonizerTarget("(a)") as SharedCommonizerTarget) + selectInputTargets(setOf(LeafCommonizerTarget("a")), parseCommonizerTarget("(a)") as SharedCommonizerTarget) ) } @@ -104,10 +104,10 @@ class DefaultInputTargetsSelectorTest { fun `exact output available`() { assertEquals( setOf(parseCommonizerTarget("(a, b)"), parseCommonizerTarget("(c, d)")), - DefaultInputTargetsSelector( + selectInputTargets( setOf("a", "b", "c", "d", "(a, b)", "(c, d)", "(a, b, c, d)").map(::parseCommonizerTarget).toSet(), parseCommonizerTarget("(a, b, c, d)") as SharedCommonizerTarget ) ) } -} \ No newline at end of file +}