From 88441da1314045cf4a3fae0c160aca831bd339a1 Mon Sep 17 00:00:00 2001 From: Alexey Andreev Date: Tue, 17 Oct 2017 17:36:38 +0300 Subject: [PATCH] JS: cache PropertyMetadata objects in property delegates Also, remove unused PropertyMetadata instances See KT-20737, KT-20738 --- .../js/test/semantics/BoxJsTestGenerated.java | 12 ++++++ .../js/translate/context/StaticContext.java | 21 ++++++++++ .../translate/context/TranslationContext.java | 5 +++ .../declaration/PropertyTranslator.kt | 6 ++- .../metadataReferentialEquality.kt | 40 +++++++++++++++++++ .../unusedPropertyMetadata.kt | 12 ++++++ .../testData/lineNumbers/delegateMemberVal.kt | 2 +- .../testData/lineNumbers/delegatedProperty.kt | 2 +- 8 files changed, 96 insertions(+), 4 deletions(-) create mode 100644 js/js.translator/testData/box/delegateProperty/metadataReferentialEquality.kt create mode 100644 js/js.translator/testData/box/delegateProperty/unusedPropertyMetadata.kt diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/BoxJsTestGenerated.java b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/BoxJsTestGenerated.java index e86d6605b0d..febd8dac82a 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/BoxJsTestGenerated.java +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/BoxJsTestGenerated.java @@ -1352,6 +1352,12 @@ public class BoxJsTestGenerated extends AbstractBoxJsTest { doTest(fileName); } + @TestMetadata("metadataReferentialEquality.kt") + public void testMetadataReferentialEquality() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("js/js.translator/testData/box/delegateProperty/metadataReferentialEquality.kt"); + doTest(fileName); + } + @TestMetadata("peculiarName.kt") public void testPeculiarName() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("js/js.translator/testData/box/delegateProperty/peculiarName.kt"); @@ -1388,6 +1394,12 @@ public class BoxJsTestGenerated extends AbstractBoxJsTest { doTest(fileName); } + @TestMetadata("unusedPropertyMetadata.kt") + public void testUnusedPropertyMetadata() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("js/js.translator/testData/box/delegateProperty/unusedPropertyMetadata.kt"); + doTest(fileName); + } + @TestMetadata("withGenerics.kt") public void testWithGenerics() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("js/js.translator/testData/box/delegateProperty/withGenerics.kt"); diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/StaticContext.java b/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/StaticContext.java index 67253bbe6c1..1d035fe0b38 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/StaticContext.java +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/StaticContext.java @@ -146,6 +146,8 @@ public final class StaticContext { @NotNull private final SourceFilePathResolver sourceFilePathResolver; + private final Map propertyMetadataVariables = new HashMap<>(); + private final boolean isStdlib; private static final Set BUILTIN_JS_PROPERTIES = Sets.union( @@ -890,4 +892,23 @@ public final class StaticContext { return null; } + + @NotNull + public JsName getVariableForPropertyMetadata(@NotNull VariableDescriptorWithAccessors property) { + return propertyMetadataVariables.computeIfAbsent(property, p -> { + String id = getSuggestedName(property) + "_metadata"; + JsName name = JsScope.declareTemporaryName(NameSuggestion.sanitizeName(id)); + + // Unexpectedly! However, the only thing, for which 'imported' property is relevant, is a import clener. + // We want similar cleanup to be performed for unused MetadataProperty instances. + // TODO: consider a different name for 'imported' property + MetadataProperties.setImported(name, true); + + JsStringLiteral propertyNameLiteral = new JsStringLiteral(property.getName().asString()); + JsExpression construction = new JsNew(getReferenceToIntrinsic("PropertyMetadata"), + Collections.singletonList(propertyNameLiteral)); + fragment.getDeclarationBlock().getStatements().add(JsAstUtils.newVar(name, construction)); + return name; + }); + } } diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/TranslationContext.java b/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/TranslationContext.java index ed9fff35280..79fd2da80a5 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/TranslationContext.java +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/TranslationContext.java @@ -911,4 +911,9 @@ public class TranslationContext { public SourceFilePathResolver getSourceFilePathResolver() { return staticContext.getSourceFilePathResolver(); } + + @NotNull + public JsName getVariableForPropertyMetadata(@NotNull VariableDescriptorWithAccessors property) { + return staticContext.getVariableForPropertyMetadata(property); + } } diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/declaration/PropertyTranslator.kt b/js/js.translator/src/org/jetbrains/kotlin/js/translate/declaration/PropertyTranslator.kt index b2cfb84a5c9..fdf4ef32170 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/declaration/PropertyTranslator.kt +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/declaration/PropertyTranslator.kt @@ -20,6 +20,7 @@ import org.jetbrains.kotlin.descriptors.* import org.jetbrains.kotlin.descriptors.impl.LocalVariableAccessorDescriptor import org.jetbrains.kotlin.descriptors.impl.LocalVariableDescriptor import org.jetbrains.kotlin.js.backend.ast.* +import org.jetbrains.kotlin.js.backend.ast.metadata.synthetic import org.jetbrains.kotlin.js.translate.callTranslator.CallTranslator import org.jetbrains.kotlin.js.translate.context.Namer import org.jetbrains.kotlin.js.translate.context.Namer.getReceiverParameterName @@ -29,6 +30,7 @@ import org.jetbrains.kotlin.js.translate.expression.translateFunction import org.jetbrains.kotlin.js.translate.general.AbstractTranslator import org.jetbrains.kotlin.js.translate.general.Translation import org.jetbrains.kotlin.js.translate.utils.BindingUtils +import org.jetbrains.kotlin.js.translate.utils.JsAstUtils.pureFqn import org.jetbrains.kotlin.js.translate.utils.JsDescriptorUtils import org.jetbrains.kotlin.js.translate.utils.TranslationUtils import org.jetbrains.kotlin.js.translate.utils.TranslationUtils.* @@ -187,15 +189,15 @@ fun TranslationContext.contextWithPropertyMetadataCreationIntrinsified( property: VariableDescriptorWithAccessors, host: JsExpression ): TranslationContext { - val propertyNameLiteral = JsStringLiteral(property.name.asString()) // 0th argument is instance, 1st is KProperty, 2nd (for setter) is value val hostExpression = (delegatedCall.valueArgumentsByIndex!![0] as ExpressionValueArgument).valueArgument!!.getArgumentExpression() val fakeArgumentExpression = (delegatedCall.valueArgumentsByIndex!![1] as ExpressionValueArgument).valueArgument!!.getArgumentExpression() + val metadataRef = pureFqn(getVariableForPropertyMetadata(property), null).apply { synthetic = true } return innerContextWithAliasesForExpressions(mapOf( hostExpression to host, - fakeArgumentExpression to JsNew(getReferenceToIntrinsic("PropertyMetadata"), listOf(propertyNameLiteral)) + fakeArgumentExpression to metadataRef )) } diff --git a/js/js.translator/testData/box/delegateProperty/metadataReferentialEquality.kt b/js/js.translator/testData/box/delegateProperty/metadataReferentialEquality.kt new file mode 100644 index 00000000000..25e4d3d120d --- /dev/null +++ b/js/js.translator/testData/box/delegateProperty/metadataReferentialEquality.kt @@ -0,0 +1,40 @@ +// EXPECTED_REACHABLE_NODES: 1144 +import kotlin.reflect.KProperty + +var lastGeneratedId = 0 +val idMap = mutableMapOf, Int>() + +class MyDelegate(val value: String) { + operator fun getValue(receiver: Any?, property: KProperty<*>): String { + val id = idMap.getOrPut(property) { lastGeneratedId++ } + return "${property.name}:$value:$id" + } +} + +class C { + val foo by MyDelegate("C") +} + +val bar by MyDelegate("toplevel") + +fun box(): String { + val c = C() + + var a = c.foo + var b = c.foo + if (a !== b) return "fail: member property referential equality" + if (!a.startsWith("foo:C:")) return "fail: member property value" + + a = bar + b = bar + if (a !== b) return "fail: top level property referential equality" + if (!a.startsWith("bar:toplevel:")) return "fail: top level property value" + + val baz by MyDelegate("local") + a = baz + b = baz + if (a !== b) return "fail: local property referential equality" + if (!a.startsWith("baz:local:")) return "fail: local property value" + + return "OK" +} \ No newline at end of file diff --git a/js/js.translator/testData/box/delegateProperty/unusedPropertyMetadata.kt b/js/js.translator/testData/box/delegateProperty/unusedPropertyMetadata.kt new file mode 100644 index 00000000000..7d26c1832ba --- /dev/null +++ b/js/js.translator/testData/box/delegateProperty/unusedPropertyMetadata.kt @@ -0,0 +1,12 @@ +// EXPECTED_REACHABLE_NODES: 1131 +// PROPERTY_NOT_USED: PropertyMetadata +import kotlin.reflect.KProperty + +class MyDelegate(val value: String) { + inline operator fun getValue(receiver: Any?, property: KProperty<*>): String = value +} + +fun box(): String { + val x by MyDelegate("OK") + return x +} \ No newline at end of file diff --git a/js/js.translator/testData/lineNumbers/delegateMemberVal.kt b/js/js.translator/testData/lineNumbers/delegateMemberVal.kt index a1219b32f1c..ea5080f7dee 100644 --- a/js/js.translator/testData/lineNumbers/delegateMemberVal.kt +++ b/js/js.translator/testData/lineNumbers/delegateMemberVal.kt @@ -13,4 +13,4 @@ class Delegate(val f: () -> Int) { } } -// LINES: 1 2 3 2 2 3 3 3 * 8 7 7 10 10 13 12 12 +// LINES: 1 2 3 * 2 2 3 3 3 * 8 7 7 10 10 13 12 12 diff --git a/js/js.translator/testData/lineNumbers/delegatedProperty.kt b/js/js.translator/testData/lineNumbers/delegatedProperty.kt index 1ca49c2812b..34ac21a74e8 100644 --- a/js/js.translator/testData/lineNumbers/delegatedProperty.kt +++ b/js/js.translator/testData/lineNumbers/delegatedProperty.kt @@ -15,4 +15,4 @@ class B { } } -// LINES: 3 4 5 4 4 4 2 * 8 11 10 10 15 14 14 \ No newline at end of file +// LINES: 3 4 5 * 4 4 4 2 * 8 11 10 10 15 14 14 \ No newline at end of file