From 932d84d5688ffba1f6835436e1ce7b882dcd9998 Mon Sep 17 00:00:00 2001 From: Mikhail Zarechenskiy Date: Mon, 30 Sep 2019 02:04:39 +0300 Subject: [PATCH] Fix performance regression in NI by fixing totally incorrect `hashCode` The actual problem was introduced in 4f1e85b46895d3799f6e361e0433d037215a15ff, note how `hashCode` is implemented: ``` var currentHashCode = cachedHashCode if (currentHashCode == 0) return currentHashCode ... ``` It's a silly bug, there should be check `if (currentHashCode != 0) ...` because `0` is used a marker for "uncomputed value". Now, in the commit 0219b86d0637cafb438b1609add957adb04b63d6 I added map with `KotlinType` as a key and because of constant `hash` for `KotlinType`, we basically got `List` instead of `Map`, which caused this performance regression #KT-34063 Fixed --- .../fir/FirDiagnosticsSmokeTestGenerated.java | 5 + .../commonSystem/manyArgumentsForVararg.kt | 178 ++++++++++++++++++ .../commonSystem/manyArgumentsForVararg.txt | 12 ++ .../diagnostics/notLinked/dfa/pos/41.kt | 2 +- .../diagnostics/notLinked/dfa/pos/6.kt | 10 +- .../checkers/DiagnosticsTestGenerated.java | 5 + .../DiagnosticsUsingJavacTestGenerated.java | 5 + .../org/jetbrains/kotlin/types/KotlinType.kt | 2 +- 8 files changed, 212 insertions(+), 7 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/inference/commonSystem/manyArgumentsForVararg.kt create mode 100644 compiler/testData/diagnostics/tests/inference/commonSystem/manyArgumentsForVararg.txt diff --git a/compiler/fir/resolve/tests/org/jetbrains/kotlin/fir/FirDiagnosticsSmokeTestGenerated.java b/compiler/fir/resolve/tests/org/jetbrains/kotlin/fir/FirDiagnosticsSmokeTestGenerated.java index 4e9a2fc3347..f28c73c9a11 100644 --- a/compiler/fir/resolve/tests/org/jetbrains/kotlin/fir/FirDiagnosticsSmokeTestGenerated.java +++ b/compiler/fir/resolve/tests/org/jetbrains/kotlin/fir/FirDiagnosticsSmokeTestGenerated.java @@ -10124,6 +10124,11 @@ public class FirDiagnosticsSmokeTestGenerated extends AbstractFirDiagnosticsSmok runTest("compiler/testData/diagnostics/tests/inference/commonSystem/lessSpecificTypeForArgumentCallWithExactAnnotation_ni.kt"); } + @TestMetadata("manyArgumentsForVararg.kt") + public void testManyArgumentsForVararg() throws Exception { + runTest("compiler/testData/diagnostics/tests/inference/commonSystem/manyArgumentsForVararg.kt"); + } + @TestMetadata("nestedLambdas.kt") public void testNestedLambdas() throws Exception { runTest("compiler/testData/diagnostics/tests/inference/commonSystem/nestedLambdas.kt"); diff --git a/compiler/testData/diagnostics/tests/inference/commonSystem/manyArgumentsForVararg.kt b/compiler/testData/diagnostics/tests/inference/commonSystem/manyArgumentsForVararg.kt new file mode 100644 index 00000000000..ad37f1b3559 --- /dev/null +++ b/compiler/testData/diagnostics/tests/inference/commonSystem/manyArgumentsForVararg.kt @@ -0,0 +1,178 @@ +// !LANGUAGE: +NewInference +// !DIAGNOSTICS: -UNUSED_PARAMETER + +val prop = mapOf( + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b"), + to("a", "b") +) + +fun to(a: A, b: B): Pair = TODO() +fun mapOf(vararg pairs: Pair): Map = TODO() + +class Pair \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/inference/commonSystem/manyArgumentsForVararg.txt b/compiler/testData/diagnostics/tests/inference/commonSystem/manyArgumentsForVararg.txt new file mode 100644 index 00000000000..39541a7c43b --- /dev/null +++ b/compiler/testData/diagnostics/tests/inference/commonSystem/manyArgumentsForVararg.txt @@ -0,0 +1,12 @@ +package + +public val prop: kotlin.collections.Map +public fun mapOf(/*0*/ vararg pairs: Pair /*kotlin.Array>*/): kotlin.collections.Map +public fun to(/*0*/ a: A, /*1*/ b: B): Pair + +public final class Pair { + public constructor Pair() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} diff --git a/compiler/tests-spec/testData/diagnostics/notLinked/dfa/pos/41.kt b/compiler/tests-spec/testData/diagnostics/notLinked/dfa/pos/41.kt index 3c16fb00041..5e12c2a1b14 100644 --- a/compiler/tests-spec/testData/diagnostics/notLinked/dfa/pos/41.kt +++ b/compiler/tests-spec/testData/diagnostics/notLinked/dfa/pos/41.kt @@ -19,7 +19,7 @@ fun case_1(x: Any) { if (x is Interface1) { if (x is Interface2) { x - x.itest() + x.itest() x.itest1() x.itest2() } diff --git a/compiler/tests-spec/testData/diagnostics/notLinked/dfa/pos/6.kt b/compiler/tests-spec/testData/diagnostics/notLinked/dfa/pos/6.kt index 20b635170e1..8e522c28619 100644 --- a/compiler/tests-spec/testData/diagnostics/notLinked/dfa/pos/6.kt +++ b/compiler/tests-spec/testData/diagnostics/notLinked/dfa/pos/6.kt @@ -1297,7 +1297,7 @@ fun case_71(t: Any?) { t t.itest1() t.itest2() - t.itest() + t.itest() t.let { it.itest1(); it.itest2() } } @@ -1318,7 +1318,7 @@ fun case_72(t: Any?, z1: Nothing?) { t t.itest1() t.itest2() - t.itest() + t.itest() t.let { it.itest1(); it.itest2() } } @@ -1340,7 +1340,7 @@ fun case_73(t: Any?) { if (t != `null`) { t.itest2() t.itest1() - t.itest() + t.itest() t.test1() t.test2() t @@ -1365,7 +1365,7 @@ fun case_74(t: Any?) { if (t is Interface3?) { t.itest2() t.itest1() - t.itest() + t.itest() t.test1() t.test2() t @@ -1388,7 +1388,7 @@ fun case_75(t: Any?, z: Nothing?) { if (t !is Interface2? || t !is Interface3?) {} else { t.itest2() t.itest1() - t.itest() + t.itest() t.test1() t.test2() t diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java index 4bce66ca116..cfcb365010b 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java @@ -10131,6 +10131,11 @@ public class DiagnosticsTestGenerated extends AbstractDiagnosticsTest { runTest("compiler/testData/diagnostics/tests/inference/commonSystem/lessSpecificTypeForArgumentCallWithExactAnnotation_ni.kt"); } + @TestMetadata("manyArgumentsForVararg.kt") + public void testManyArgumentsForVararg() throws Exception { + runTest("compiler/testData/diagnostics/tests/inference/commonSystem/manyArgumentsForVararg.kt"); + } + @TestMetadata("nestedLambdas.kt") public void testNestedLambdas() throws Exception { runTest("compiler/testData/diagnostics/tests/inference/commonSystem/nestedLambdas.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsUsingJavacTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsUsingJavacTestGenerated.java index 7b805010b87..83ec993f7d0 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsUsingJavacTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsUsingJavacTestGenerated.java @@ -10126,6 +10126,11 @@ public class DiagnosticsUsingJavacTestGenerated extends AbstractDiagnosticsUsing runTest("compiler/testData/diagnostics/tests/inference/commonSystem/lessSpecificTypeForArgumentCallWithExactAnnotation_ni.kt"); } + @TestMetadata("manyArgumentsForVararg.kt") + public void testManyArgumentsForVararg() throws Exception { + runTest("compiler/testData/diagnostics/tests/inference/commonSystem/manyArgumentsForVararg.kt"); + } + @TestMetadata("nestedLambdas.kt") public void testNestedLambdas() throws Exception { runTest("compiler/testData/diagnostics/tests/inference/commonSystem/nestedLambdas.kt"); diff --git a/core/descriptors/src/org/jetbrains/kotlin/types/KotlinType.kt b/core/descriptors/src/org/jetbrains/kotlin/types/KotlinType.kt index 7f8caf554a0..c64601745a4 100644 --- a/core/descriptors/src/org/jetbrains/kotlin/types/KotlinType.kt +++ b/core/descriptors/src/org/jetbrains/kotlin/types/KotlinType.kt @@ -101,7 +101,7 @@ sealed class KotlinType : Annotated, KotlinTypeMarker { final override fun hashCode(): Int { // NB: make one read to prevent race var currentHashCode = cachedHashCode - if (currentHashCode == 0) return currentHashCode + if (currentHashCode != 0) return currentHashCode currentHashCode = computeHashCode()