diff --git a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/ExpressionDecomposer.kt b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/ExpressionDecomposer.kt index f1750efbdbb..3b2202c0354 100644 --- a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/ExpressionDecomposer.kt +++ b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/ExpressionDecomposer.kt @@ -267,10 +267,9 @@ internal class ExpressionDecomposer private constructor( // Qualifier might be a reference to lambda property. See KT-7674 // An exception here is `fn.call()`, which are marked as side effect free. Further recognition of such // case in inliner might be quite difficult, so never extract such call (and other calls marked this way). - if (qualifier in containsNodeWithSideEffect && (qualifier as? HasMetadata)?.sideEffects != SideEffectKind.PURE) { - qualifier = qualifier.extractToTemporary() - } - else if (callee != null && receiver != null && receiver in containsNodeWithSideEffect) { + if ((qualifier !in containsNodeWithSideEffect || (qualifier as? HasMetadata)?.sideEffects == SideEffectKind.PURE) && + (callee != null && receiver != null && receiver in containsNodeWithSideEffect) + ) { val receiverTmp = receiver.extractToTemporary() callee.qualifier = receiverTmp } diff --git a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/IneffectiveStatementElimination.kt b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/IneffectiveStatementElimination.kt index db13093fdb5..c612300231f 100644 --- a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/IneffectiveStatementElimination.kt +++ b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/IneffectiveStatementElimination.kt @@ -161,7 +161,7 @@ class IneffectiveStatementElimination(private val root: JsFunction) { } // Although it can be suspicious case, sometimes it really helps. - // Consider the following case: `Kotlin.modules['foo'].bar()`, where `foo` is inlineable. Expression decomposer produces + // Consider the following case: `Kotlin.modules['foo'].bar()`, where `bar` is inlineable. Expression decomposer produces // var $tmp = Kotlin.modules['foo']; // $tmp.bar(); // Then, inlined body of `bar` never uses `$tmp`, therefore we can eliminate it, so `Kotlin.modules['foo']` remains. diff --git a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/TemporaryAssignmentElimination.kt b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/TemporaryAssignmentElimination.kt index 3713812989b..19570433fc7 100644 --- a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/TemporaryAssignmentElimination.kt +++ b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/TemporaryAssignmentElimination.kt @@ -24,8 +24,7 @@ import org.jetbrains.kotlin.js.inline.util.collectFreeVariables import org.jetbrains.kotlin.js.translate.utils.JsAstUtils internal class TemporaryAssignmentElimination(private val root: JsBlock) { - // We say "use" about any references to a temporary variable - private val useCount = mutableMapOf() + private val referenceCount = mutableMapOf() // We say "usage" about special kind of a reference to a temporary variable in the following cases: // someVar = $tmp; @@ -129,7 +128,7 @@ internal class TemporaryAssignmentElimination(private val root: JsBlock) { private fun getUsageSequence(name: JsName): UsageSequence? { return usageSequences.getOrPut(name) { - if (useCount[name] != 1) return null + if (referenceCount[name] != 1) return null val usage = usages[name] val mappedUsage: UsageSequence? = when (usage) { @@ -255,7 +254,7 @@ internal class TemporaryAssignmentElimination(private val root: JsBlock) { } private fun use(name: JsName) { - useCount[name] = 1 + (useCount[name] ?: 0) + referenceCount[name] = 1 + (referenceCount[name] ?: 0) } private sealed class Usage(val statement: JsStatement) { diff --git a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/TemporaryVariableElimination.kt b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/TemporaryVariableElimination.kt index 4bef65233eb..d996fafaac4 100644 --- a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/TemporaryVariableElimination.kt +++ b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/TemporaryVariableElimination.kt @@ -33,7 +33,7 @@ internal class TemporaryVariableElimination(function: JsFunction) { private val definedValues = mutableMapOf() private val temporary = mutableSetOf() private var hasChanges = false - private val namesToProcess = function.collectLocalVariables() + private val localVariables = function.collectLocalVariables() private val statementsToRemove = mutableSetOf() private val namesToSubstitute = mutableSetOf() private val variablesToRemove = mutableSetOf() @@ -54,7 +54,7 @@ internal class TemporaryVariableElimination(function: JsFunction) { val assignment = JsAstUtils.decomposeAssignmentToVariable(x.expression) if (assignment != null) { val (name, value) = assignment - if (name in namesToProcess) { + if (name in localVariables) { assignVariable(name, value) addVar(name) accept(value) @@ -71,7 +71,7 @@ internal class TemporaryVariableElimination(function: JsFunction) { for (v in x.vars) { val name = v.name val value = v.initExpression - if (value != null && name in namesToProcess) { + if (value != null && name in localVariables) { assignVariable(name, value) addVar(name) accept(value) @@ -84,7 +84,7 @@ internal class TemporaryVariableElimination(function: JsFunction) { override fun visitNameRef(nameRef: JsNameRef) { val name = nameRef.name - if (name != null && nameRef.qualifier == null && name in namesToProcess) { + if (name != null && nameRef.qualifier == null && name in localVariables) { useVariable(name) if (name !in currentScope) { inconsistent += name @@ -523,13 +523,16 @@ internal class TemporaryVariableElimination(function: JsFunction) { usages[name] = (usages[name] ?: 0) + 1 } - private fun shouldConsiderUnused(name: JsName) = (definitions[name] ?: 0) == 1 && (usages[name] ?: 0) == 0 && name in temporary + private fun shouldConsiderUnused(name: JsName) = definitions[name] == 1 && (usages[name] ?: 0) == 0 && name in temporary private fun shouldConsiderTemporary(name: JsName): Boolean { - if (definitions[name] ?: 0 != 1 || name !in temporary) return false + if (definitions[name] != 1 || name !in temporary) return false val expr = definedValues[name] - return (expr != null && isTrivial(expr)) || usages[name] ?: 0 == 1 + // It's useful to copy trivial expressions when they are used more than once. Example are temporary variables + // that receiver another (non-temporary) variables. To prevent code from bloating, we don't treat large value literals + // as trivial expressions. + return (expr != null && isTrivial(expr)) || usages[name] == 1 } private fun isTrivial(expr: JsExpression): Boolean = when (expr) { @@ -540,14 +543,15 @@ internal class TemporaryVariableElimination(function: JsFunction) { } else { val name = expr.name - name in namesToProcess && when (definitions[name]) { + name in localVariables && when (definitions[name]) { + // Local variables with zero definitions are function parameters. We can relocate and copy them. null, 0 -> true 1 -> name !in namesToSubstitute || definedValues[name]?.let { isTrivial(it) } ?: false else -> false } } } - is JsLiteral.JsValueLiteral -> true + is JsLiteral.JsValueLiteral -> expr.toString().length < 10 is JsInvocation -> expr.sideEffects == SideEffectKind.PURE && isTrivial(expr.qualifier) && expr.arguments.all { isTrivial(it) } is JsArrayAccess -> isTrivial(expr.arrayExpression) && isTrivial(expr.indexExpression) else -> false diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/InlineSizeReductionTestGenerated.java b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/InlineSizeReductionTestGenerated.java index 75e03ff150c..ddef1a35e06 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/InlineSizeReductionTestGenerated.java +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/InlineSizeReductionTestGenerated.java @@ -65,6 +65,12 @@ public class InlineSizeReductionTestGenerated extends AbstractInlineSizeReductio doTest(fileName); } + @TestMetadata("propertyReferenceDoesNotProduceSideEffect.kt") + public void testPropertyReferenceDoesNotProduceSideEffect() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("js/js.translator/testData/inlineSizeReduction/cases/propertyReferenceDoesNotProduceSideEffect.kt"); + doTest(fileName); + } + @TestMetadata("returnInlineCall.kt") public void testReturnInlineCall() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("js/js.translator/testData/inlineSizeReduction/cases/returnInlineCall.kt"); diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/callTranslator/CallInfoExtensions.kt b/js/js.translator/src/org/jetbrains/kotlin/js/translate/callTranslator/CallInfoExtensions.kt index 9b072e06aaa..cb34e1a56de 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/callTranslator/CallInfoExtensions.kt +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/callTranslator/CallInfoExtensions.kt @@ -77,6 +77,9 @@ fun VariableAccessInfo.constructAccessExpression(ref: JsExpression): JsExpressio return ref } else { return if (value !is JsEmptyExpression) { + // This is useful when passing AST to TemporaryAssignmentElimination. It can bring + // property assignment like `obj.propertyName = $tmp` to places where `$tmp` gets its value, + // but only when it's sure that no side effects possible. (ref as? HasMetadata)?.let { it.sideEffects = SideEffectKind.PURE } JsAstUtils.assignment(ref, value!!) } diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/callTranslator/VariableCallCases.kt b/js/js.translator/src/org/jetbrains/kotlin/js/translate/callTranslator/VariableCallCases.kt index 0725fd26396..d569c4a5d07 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/callTranslator/VariableCallCases.kt +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/callTranslator/VariableCallCases.kt @@ -75,7 +75,7 @@ object DefaultVariableAccessCase : VariableAccessCase() { override fun VariableAccessInfo.dispatchReceiver(): JsExpression { val accessor = JsNameRef(variableName, dispatchReceiver!!) val descriptor = callableDescriptor - if (descriptor is PropertyDescriptor && !JsDescriptorUtils.sideEffectsPossible(descriptor)) { + if (descriptor is PropertyDescriptor && !JsDescriptorUtils.sideEffectsPossibleOnRead(descriptor)) { accessor.sideEffects = SideEffectKind.DEPENDS_ON_STATE } return constructAccessExpression(accessor) diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/utils/JsDescriptorUtils.java b/js/js.translator/src/org/jetbrains/kotlin/js/translate/utils/JsDescriptorUtils.java index 21d3b9aa9ac..2bab3cb86f8 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/utils/JsDescriptorUtils.java +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/utils/JsDescriptorUtils.java @@ -147,7 +147,7 @@ public final class JsDescriptorUtils { return accessorDescriptor == null || accessorDescriptor.isDefault(); } - public static boolean sideEffectsPossible(@NotNull PropertyDescriptor property) { + public static boolean sideEffectsPossibleOnRead(@NotNull PropertyDescriptor property) { return !isDefaultAccessor(property.getGetter()) || ModalityKt.isOverridableOrOverrides(property) || isStaticInitializationPossible(property); } diff --git a/js/js.translator/testData/inlineSizeReduction/cases/propertyReferenceDoesNotProduceSideEffect.kt b/js/js.translator/testData/inlineSizeReduction/cases/propertyReferenceDoesNotProduceSideEffect.kt new file mode 100644 index 00000000000..c19d415941f --- /dev/null +++ b/js/js.translator/testData/inlineSizeReduction/cases/propertyReferenceDoesNotProduceSideEffect.kt @@ -0,0 +1,29 @@ +// CHECK_VARS_COUNT: function=box count=1 + +package foo + +var log = "" + +class A() { + var x = 23 +} + +fun sideEffect(): Int { + log += "sideEffect();" + return 42 +} + +fun bar(a: Int, b: Int) { + log += "bar($a, $b);" +} + +inline fun test(a: Int, b: Int) { + bar(b, a) +} + +fun box(): String { + val a = A() + test(sideEffect(), a.x) + assertEquals("sideEffect();bar(23, 42);", log) + return "OK" +} \ No newline at end of file diff --git a/js/js.translator/testData/js-optimizer/temporary-assignment/ifWithoutElse.original.js b/js/js.translator/testData/js-optimizer/temporary-assignment/ifWithoutElse.original.js index 86180f197f2..ad974fbb19f 100644 --- a/js/js.translator/testData/js-optimizer/temporary-assignment/ifWithoutElse.original.js +++ b/js/js.translator/testData/js-optimizer/temporary-assignment/ifWithoutElse.original.js @@ -4,18 +4,18 @@ function f(x) { function box() { var result1, result2, $a, $b; - + if (f(true)) { $a = "1"; } result1 = $a; if (result1 !== "1") return "fail1: " + result1; - + if (f(false)) { $b = "1"; } result2 = $b; if (result2 !== void 0) return "fail2: " + result2; - + return "OK"; } \ No newline at end of file