From 098a935aa7f772165604fbafd16d44eadb24a241 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Mon, 27 Apr 2020 21:00:03 +0200 Subject: [PATCH] Fix exponential string table size of anonymous classes during inlining When we inline an anonymous object which captures something such as crossinline values or reified parameters, we copy and transform its metadata in `AnonymousObjectTransformer.transformMetadata`. Basically we read the metadata of the original class, add a minor protobuf extension and write it to the new class. This also includes copying the string table. We read the string table into `JvmNameResolver` (a representation of string table used in deserialization), then construct a `JvmStringTable` (a representation used in _serialization_) and then write it back. There's a few optimizations in the string table representation in JVM metadata which allow to store less strings and thus take less space. See `StringTableTypes.Record` in `jvm_metadata.proto` for more information. One of the optimizations `Record.range` allows to avoid storing the same record many times in a sequence. For example, if we have N different strings in the string table but none of them require any operation (such as substring, char replacement, etc.), then we only store the record with all default values (no operation, no predefined string, etc.) and set its `range` to N. Upon reading such optimized record list in `JvmNameResolver`, we "expand" it back to normal, so that we could index it quickly and figure out what operation needs to be performed on each string from the string table. The problem was that when we expanded this list, we didn't set the range of the expanded record entry to 1. So each record in `JvmNameResolver.records` still has its original range. It doesn't cause any problems most of the time because the range in this expanded list is almost unused. However, when copying/transforming metadata for anonymous objects, we mistakenly passed this expanded list with incorrect ranges to `JvmStringTable`. So the metadata in the copied anonymous object ended up being incorrect: each record now was present the number of times equal to its range. Copying such metadata once again led to another multiplication of the record list size. Multiple copies resulted in exponential increase in memory consumption and quickly led to OOM. For the fix, we now take the original, unexpanded list of records when creating `JvmStringTable` out of `JvmNameResolver` for transformation of anonymous object metadata. Note that another possible fix would be to make range for each record in `JvmNameResolver.records` equal to 1. This is undesirable though, since then we'd need to copy each `JvmProtoBuf.StringTableTypes.Record` instance, of which there could be many, and use some memory for no apparent gain (since ranges in that expanded list are now not used at all). #KT-38197 Fixed --- .../boxInline/anonymousObject/kt38197.kt | 53 +++++++++++++++++++ .../BlackBoxInlineCodegenTestGenerated.java | 5 ++ ...otlinAgainstInlineKotlinTestGenerated.java | 5 ++ .../IrBlackBoxInlineCodegenTestGenerated.java | 5 ++ ...otlinAgainstInlineKotlinTestGenerated.java | 5 ++ .../jvm/deserialization/JvmNameResolver.kt | 4 +- .../jvm/serialization/JvmStringTable.kt | 2 +- .../IrJsCodegenInlineTestGenerated.java | 5 ++ .../JsCodegenInlineTestGenerated.java | 5 ++ 9 files changed, 87 insertions(+), 2 deletions(-) create mode 100644 compiler/testData/codegen/boxInline/anonymousObject/kt38197.kt diff --git a/compiler/testData/codegen/boxInline/anonymousObject/kt38197.kt b/compiler/testData/codegen/boxInline/anonymousObject/kt38197.kt new file mode 100644 index 00000000000..aa9e6933b1e --- /dev/null +++ b/compiler/testData/codegen/boxInline/anonymousObject/kt38197.kt @@ -0,0 +1,53 @@ +// FILE: test.kt +// WITH_RUNTIME +// WITH_COROUTINES + +var result = "Fail" + +fun use(token: String) { + result = token +} + +inline fun f(crossinline body: suspend () -> Unit) = + g< + Any, Any, Any, Any, Any, Any, Any, Any, Any, Any, + Any, Any, Any, Any, Any, Any, Any, Any, Any, Any, + >(body) + +inline fun < + reified U01, reified U02, reified U03, reified U04, reified U05, reified U06, reified U07, reified U08, reified U09, reified U10, + reified U11, reified U12, reified U13, reified U14, reified U15, reified U16, reified U17, reified U18, reified U19, reified U20, +> g(crossinline body: suspend () -> Unit): suspend () -> Unit { + return run { + run { + run { + run { + run { + run { + run { + run { + suspend { + body() + } + } + } + } + } + } + } + } + } +} + +// FILE: box.kt + +import kotlin.coroutines.* +import helpers.* + +fun box(): String { + var token = "OK" + f { + use(token) + }.startCoroutine(EmptyContinuation) + return result +} diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxInlineCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxInlineCodegenTestGenerated.java index 4e622d6d24e..73a553a9c9f 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxInlineCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxInlineCodegenTestGenerated.java @@ -231,6 +231,11 @@ public class BlackBoxInlineCodegenTestGenerated extends AbstractBlackBoxInlineCo runTest("compiler/testData/codegen/boxInline/anonymousObject/kt34656.kt"); } + @TestMetadata("kt38197.kt") + public void testKt38197() throws Exception { + runTest("compiler/testData/codegen/boxInline/anonymousObject/kt38197.kt"); + } + @TestMetadata("kt6552.kt") public void testKt6552() throws Exception { runTest("compiler/testData/codegen/boxInline/anonymousObject/kt6552.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/CompileKotlinAgainstInlineKotlinTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/CompileKotlinAgainstInlineKotlinTestGenerated.java index e790a47aacc..2ce3e8aa158 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/CompileKotlinAgainstInlineKotlinTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/CompileKotlinAgainstInlineKotlinTestGenerated.java @@ -231,6 +231,11 @@ public class CompileKotlinAgainstInlineKotlinTestGenerated extends AbstractCompi runTest("compiler/testData/codegen/boxInline/anonymousObject/kt34656.kt"); } + @TestMetadata("kt38197.kt") + public void testKt38197() throws Exception { + runTest("compiler/testData/codegen/boxInline/anonymousObject/kt38197.kt"); + } + @TestMetadata("kt6552.kt") public void testKt6552() throws Exception { runTest("compiler/testData/codegen/boxInline/anonymousObject/kt6552.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxInlineCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxInlineCodegenTestGenerated.java index faac3b839a0..0b59675bd1d 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxInlineCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxInlineCodegenTestGenerated.java @@ -231,6 +231,11 @@ public class IrBlackBoxInlineCodegenTestGenerated extends AbstractIrBlackBoxInli runTest("compiler/testData/codegen/boxInline/anonymousObject/kt34656.kt"); } + @TestMetadata("kt38197.kt") + public void testKt38197() throws Exception { + runTest("compiler/testData/codegen/boxInline/anonymousObject/kt38197.kt"); + } + @TestMetadata("kt6552.kt") public void testKt6552() throws Exception { runTest("compiler/testData/codegen/boxInline/anonymousObject/kt6552.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrCompileKotlinAgainstInlineKotlinTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrCompileKotlinAgainstInlineKotlinTestGenerated.java index eaff72b2f2e..8ef212481de 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrCompileKotlinAgainstInlineKotlinTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrCompileKotlinAgainstInlineKotlinTestGenerated.java @@ -231,6 +231,11 @@ public class IrCompileKotlinAgainstInlineKotlinTestGenerated extends AbstractIrC runTest("compiler/testData/codegen/boxInline/anonymousObject/kt34656.kt"); } + @TestMetadata("kt38197.kt") + public void testKt38197() throws Exception { + runTest("compiler/testData/codegen/boxInline/anonymousObject/kt38197.kt"); + } + @TestMetadata("kt6552.kt") public void testKt6552() throws Exception { runTest("compiler/testData/codegen/boxInline/anonymousObject/kt6552.kt"); diff --git a/core/metadata.jvm/src/org/jetbrains/kotlin/metadata/jvm/deserialization/JvmNameResolver.kt b/core/metadata.jvm/src/org/jetbrains/kotlin/metadata/jvm/deserialization/JvmNameResolver.kt index c32956b425a..45dfe1d8ea3 100644 --- a/core/metadata.jvm/src/org/jetbrains/kotlin/metadata/jvm/deserialization/JvmNameResolver.kt +++ b/core/metadata.jvm/src/org/jetbrains/kotlin/metadata/jvm/deserialization/JvmNameResolver.kt @@ -17,7 +17,9 @@ class JvmNameResolver( private val localNameIndices = types.localNameList.run { if (isEmpty()) emptySet() else toSet() } // Here we expand the 'range' field of the Record message for simplicity to a list of records - val records: List = ArrayList().apply { + // Note that as an optimization, range of each expanded record is equal to the original range, not 1. If correct ranges are needed, + // please use the original record representation in [types.recordList]. + private val records: List = ArrayList().apply { val records = types.recordList this.ensureCapacity(records.size) for (record in records) { diff --git a/core/metadata.jvm/src/org/jetbrains/kotlin/metadata/jvm/serialization/JvmStringTable.kt b/core/metadata.jvm/src/org/jetbrains/kotlin/metadata/jvm/serialization/JvmStringTable.kt index e3c23da8943..d9676c752b8 100644 --- a/core/metadata.jvm/src/org/jetbrains/kotlin/metadata/jvm/serialization/JvmStringTable.kt +++ b/core/metadata.jvm/src/org/jetbrains/kotlin/metadata/jvm/serialization/JvmStringTable.kt @@ -21,7 +21,7 @@ open class JvmStringTable(nameResolver: JvmNameResolver? = null) : StringTable { init { if (nameResolver != null) { strings.addAll(nameResolver.strings) - nameResolver.records.mapTo(records, JvmProtoBuf.StringTableTypes.Record::toBuilder) + nameResolver.types.recordList.mapTo(records, JvmProtoBuf.StringTableTypes.Record::toBuilder) for (index in strings.indices) { map[nameResolver.getString(index)] = index } diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/ir/semantics/IrJsCodegenInlineTestGenerated.java b/js/js.tests/test/org/jetbrains/kotlin/js/test/ir/semantics/IrJsCodegenInlineTestGenerated.java index 80146d5aaff..5407aa19ab8 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/ir/semantics/IrJsCodegenInlineTestGenerated.java +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/ir/semantics/IrJsCodegenInlineTestGenerated.java @@ -211,6 +211,11 @@ public class IrJsCodegenInlineTestGenerated extends AbstractIrJsCodegenInlineTes runTest("compiler/testData/codegen/boxInline/anonymousObject/kt34656.kt"); } + @TestMetadata("kt38197.kt") + public void testKt38197() throws Exception { + runTest("compiler/testData/codegen/boxInline/anonymousObject/kt38197.kt"); + } + @TestMetadata("kt6552.kt") public void testKt6552() throws Exception { runTest("compiler/testData/codegen/boxInline/anonymousObject/kt6552.kt"); diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenInlineTestGenerated.java b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenInlineTestGenerated.java index c23330608b4..03c76845928 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenInlineTestGenerated.java +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenInlineTestGenerated.java @@ -211,6 +211,11 @@ public class JsCodegenInlineTestGenerated extends AbstractJsCodegenInlineTest { runTest("compiler/testData/codegen/boxInline/anonymousObject/kt34656.kt"); } + @TestMetadata("kt38197.kt") + public void testKt38197() throws Exception { + runTest("compiler/testData/codegen/boxInline/anonymousObject/kt38197.kt"); + } + @TestMetadata("kt6552.kt") public void testKt6552() throws Exception { runTest("compiler/testData/codegen/boxInline/anonymousObject/kt6552.kt");