From 0219b86d0637cafb438b1609add957adb04b63d6 Mon Sep 17 00:00:00 2001 From: Mikhail Zarechenskiy Date: Mon, 5 Aug 2019 14:41:36 +0300 Subject: [PATCH] [NI] Fix poor performance of recursive types approximation #KT-32407 Fixed --- .../fir/FirDiagnosticsSmokeTestGenerated.java | 5 + .../resolve/calls/tower/PSICallResolver.kt | 16 +++- .../kotlin/types/TypeApproximator.kt | 91 ++++++++++++++----- .../recursiveJavaTypeWithStarProjection.kt | 22 +++++ .../recursiveJavaTypeWithStarProjection.txt | 25 +++++ .../checkers/DiagnosticsTestGenerated.java | 5 + .../DiagnosticsUsingJavacTestGenerated.java | 5 + 7 files changed, 145 insertions(+), 24 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/inference/constraints/recursiveJavaTypeWithStarProjection.kt create mode 100644 compiler/testData/diagnostics/tests/inference/constraints/recursiveJavaTypeWithStarProjection.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 51a7a0199cb..e4e97d55ae7 100644 --- a/compiler/fir/resolve/tests/org/jetbrains/kotlin/fir/FirDiagnosticsSmokeTestGenerated.java +++ b/compiler/fir/resolve/tests/org/jetbrains/kotlin/fir/FirDiagnosticsSmokeTestGenerated.java @@ -10212,6 +10212,11 @@ public class FirDiagnosticsSmokeTestGenerated extends AbstractFirDiagnosticsSmok runTest("compiler/testData/diagnostics/tests/inference/constraints/operationsOnIntegerValueTypes.kt"); } + @TestMetadata("recursiveJavaTypeWithStarProjection.kt") + public void testRecursiveJavaTypeWithStarProjection() throws Exception { + runTest("compiler/testData/diagnostics/tests/inference/constraints/recursiveJavaTypeWithStarProjection.kt"); + } + @TestMetadata("returnLambdaFromLambda.kt") public void testReturnLambdaFromLambda() throws Exception { runTest("compiler/testData/diagnostics/tests/inference/constraints/returnLambdaFromLambda.kt"); diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/tower/PSICallResolver.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/tower/PSICallResolver.kt index 38af1d835ab..098bdaf3c88 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/tower/PSICallResolver.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/tower/PSICallResolver.kt @@ -106,7 +106,10 @@ class PSICallResolver( return OverloadResolutionResultsImpl.nameNotFound() } - return convertToOverloadResolutionResults(context, result, tracingStrategy) + val overloadResolutionResults = convertToOverloadResolutionResults(context, result, tracingStrategy) + return overloadResolutionResults.also { + clearCacheForApproximationResults() + } } // actually, `D` is at least FunctionDescriptor, but right now because of CallResolver it isn't possible change upper bound for `D` @@ -133,7 +136,16 @@ class PSICallResolver( val result = kotlinCallResolver.resolveGivenCandidates( scopeTower, resolutionCallbacks, kotlinCall, calculateExpectedType(context), givenCandidates, context.collectAllCandidates ) - return convertToOverloadResolutionResults(context, result, tracingStrategy) + val overloadResolutionResults = convertToOverloadResolutionResults(context, result, tracingStrategy) + return overloadResolutionResults.also { + clearCacheForApproximationResults() + } + } + + private fun clearCacheForApproximationResults() { + // Mostly, we approximate captured or some other internal types that don't live longer than resolve for a call, + // so it's quite useless to preserve cache for longer time + typeApproximator.clearCache() } private fun resolveToDeprecatedMod( diff --git a/compiler/resolution/src/org/jetbrains/kotlin/types/TypeApproximator.kt b/compiler/resolution/src/org/jetbrains/kotlin/types/TypeApproximator.kt index 8bcfc7186fe..e103a44e160 100644 --- a/compiler/resolution/src/org/jetbrains/kotlin/types/TypeApproximator.kt +++ b/compiler/resolution/src/org/jetbrains/kotlin/types/TypeApproximator.kt @@ -25,6 +25,7 @@ import org.jetbrains.kotlin.types.TypeApproximatorConfiguration.IntersectionStra import org.jetbrains.kotlin.types.checker.NewCapturedTypeConstructor import org.jetbrains.kotlin.types.model.* import org.jetbrains.kotlin.types.model.CaptureStatus.* +import java.util.concurrent.ConcurrentHashMap open class TypeApproximatorConfiguration { @@ -119,6 +120,10 @@ class TypeApproximator(builtIns: KotlinBuiltIns) : AbstractTypeApproximator(Clas abstract class AbstractTypeApproximator(val ctx: TypeSystemInferenceExtensionContext) : TypeSystemInferenceExtensionContext by ctx { + private class ApproximationResult(val type: KotlinTypeMarker?) + + private val cacheForIncorporationConfigToSuperDirection = ConcurrentHashMap() + private val cacheForIncorporationConfigToSubtypeDirection = ConcurrentHashMap() private val referenceApproximateToSuperType = this::approximateSimpleToSuperType private val referenceApproximateToSubType = this::approximateSimpleToSubType @@ -136,23 +141,69 @@ abstract class AbstractTypeApproximator(val ctx: TypeSystemInferenceExtensionCon fun approximateToSubType(type: KotlinTypeMarker, conf: TypeApproximatorConfiguration): KotlinTypeMarker? = approximateToSubType(type, conf, -type.typeDepth()) + fun clearCache() { + cacheForIncorporationConfigToSubtypeDirection.clear() + cacheForIncorporationConfigToSuperDirection.clear() + } + + private fun checkExceptionalCases( + type: KotlinTypeMarker, depth: Int, conf: TypeApproximatorConfiguration, toSuper: Boolean + ): ApproximationResult? { + return when { + type is TypeUtils.SpecialType -> + null.toApproximationResult() + + type.isError() -> + // todo -- fix builtIns. Now builtIns here is DefaultBuiltIns + (if (conf.errorType) null else type.defaultResult(toSuper)).toApproximationResult() + + depth > 3 -> + type.defaultResult(toSuper).toApproximationResult() + + else -> null + } + } + + private fun KotlinTypeMarker?.toApproximationResult(): ApproximationResult = ApproximationResult(this) + + private inline fun cachedValue( + type: KotlinTypeMarker, + conf: TypeApproximatorConfiguration, + toSuper: Boolean, + approximate: () -> KotlinTypeMarker? + ): KotlinTypeMarker? { + // Approximator depends on a configuration, so cache should take it into account + // Here, we cache only types for configuration "from incorporation", which is used most intensively + if (conf !is TypeApproximatorConfiguration.IncorporationConfiguration) return approximate() + + val cache = if (toSuper) cacheForIncorporationConfigToSuperDirection else cacheForIncorporationConfigToSubtypeDirection + return cache.getOrPut(type, { approximate().toApproximationResult() }).type + } + private fun approximateToSuperType(type: KotlinTypeMarker, conf: TypeApproximatorConfiguration, depth: Int): KotlinTypeMarker? { - if (type is TypeUtils.SpecialType) return null - return approximateTo( - prepareType(type), conf, { upperBound() }, - referenceApproximateToSuperType, depth - ) + checkExceptionalCases(type, depth, conf, toSuper = true)?.let { return it.type } + + return cachedValue(type, conf, toSuper = true) { + approximateTo( + prepareType(type), conf, { upperBound() }, + referenceApproximateToSuperType, depth + ) + } } private fun approximateToSubType(type: KotlinTypeMarker, conf: TypeApproximatorConfiguration, depth: Int): KotlinTypeMarker? { - if (type is TypeUtils.SpecialType) return null - return approximateTo( - prepareType(type), conf, { lowerBound() }, - referenceApproximateToSubType, depth - ) + checkExceptionalCases(type, depth, conf, toSuper = false)?.let { return it.type } + + return cachedValue(type, conf, toSuper = false) { + approximateTo( + prepareType(type), conf, { lowerBound() }, + referenceApproximateToSubType, depth + ) + } } - // comments for case bound = upperBound, approximateTo = toSuperType + // Don't call this method directly, it should be used only in approximateToSuperType/approximateToSubType (use these methods instead) + // This method contains detailed implementation only for type approximation, it doesn't check exceptional cases and doesn't use cache private fun approximateTo( type: KotlinTypeMarker, conf: TypeApproximatorConfiguration, @@ -330,13 +381,6 @@ abstract class AbstractTypeApproximator(val ctx: TypeSystemInferenceExtensionCon toSuper: Boolean, depth: Int ): KotlinTypeMarker? { - if (type.isError()) { - // todo -- fix builtIns. Now builtIns here is DefaultBuiltIns - return if (conf.errorType) null else type.defaultResult(toSuper) - } - - if (depth > 3) return type.defaultResult(toSuper) - if (type.argumentsCount() != 0) { return approximateParametrizedType(type, conf, toSuper, depth + 1) } @@ -382,12 +426,15 @@ abstract class AbstractTypeApproximator(val ctx: TypeSystemInferenceExtensionCon toSuper: Boolean, depth: Int ): KotlinTypeMarker? { - val approximatedOriginalType = approximateTo(type.original(), conf, toSuper, depth) + val originalType = type.original() + val approximatedOriginalType = + if (toSuper) approximateToSuperType(originalType, conf, depth) else approximateToSubType(originalType, conf, depth) + return if (conf.definitelyNotNullType) { approximatedOriginalType?.makeDefinitelyNotNullOrNotNull() } else { if (toSuper) - (approximatedOriginalType ?: type.original()).withNullability(false) + (approximatedOriginalType ?: originalType).withNullability(false) else type.defaultResult(toSuper) } @@ -519,8 +566,8 @@ abstract class AbstractTypeApproximator(val ctx: TypeSystemInferenceExtensionCon return type.replaceArguments(newArgumentsList) } - private fun SimpleTypeMarker.defaultResult(toSuper: Boolean) = if (toSuper) nullableAnyType() else { - if (isMarkedNullable()) nullableNothingType() else nothingType() + private fun KotlinTypeMarker.defaultResult(toSuper: Boolean) = if (toSuper) nullableAnyType() else { + if (this is SimpleTypeMarker && isMarkedNullable()) nullableNothingType() else nothingType() } // Any? or Any! diff --git a/compiler/testData/diagnostics/tests/inference/constraints/recursiveJavaTypeWithStarProjection.kt b/compiler/testData/diagnostics/tests/inference/constraints/recursiveJavaTypeWithStarProjection.kt new file mode 100644 index 00000000000..730fdd640d8 --- /dev/null +++ b/compiler/testData/diagnostics/tests/inference/constraints/recursiveJavaTypeWithStarProjection.kt @@ -0,0 +1,22 @@ +// !DIAGNOSTICS: -UNUSED_VARIABLE +// !LANGUAGE: +NewInference + +// FILE: MenuItemBase.java + +public class MenuItemBase, I extends MenuItemBase, S extends SubMenuBase> { + public String getText() { return null; } +} + +// FILE: ContextMenuBase.java + +public class ContextMenuBase, I extends MenuItemBase, S extends SubMenuBase> {} + +// FILE: SubMenuBase.java + +public class SubMenuBase, I extends MenuItemBase, S extends SubMenuBase> {} + +// FILE: test.kt + +fun test(m: MenuItemBase<*, *, *>) { + m.text +} \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/inference/constraints/recursiveJavaTypeWithStarProjection.txt b/compiler/testData/diagnostics/tests/inference/constraints/recursiveJavaTypeWithStarProjection.txt new file mode 100644 index 00000000000..99dabc01562 --- /dev/null +++ b/compiler/testData/diagnostics/tests/inference/constraints/recursiveJavaTypeWithStarProjection.txt @@ -0,0 +1,25 @@ +package + +public fun test(/*0*/ m: MenuItemBase<*, *, *>): kotlin.Unit + +public open class ContextMenuBase!, /*1*/ I : MenuItemBase!, /*2*/ S : SubMenuBase!> { + public constructor ContextMenuBase!, /*1*/ I : MenuItemBase!, /*2*/ S : SubMenuBase!>() + 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 +} + +public open class MenuItemBase!, /*1*/ I : MenuItemBase!, /*2*/ S : SubMenuBase!> { + public constructor MenuItemBase!, /*1*/ I : MenuItemBase!, /*2*/ S : SubMenuBase!>() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open fun getText(): kotlin.String! + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +public open class SubMenuBase!, /*1*/ I : MenuItemBase!, /*2*/ S : SubMenuBase!> { + public constructor SubMenuBase!, /*1*/ I : MenuItemBase!, /*2*/ S : SubMenuBase!>() + 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/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java index 34f92c4e9de..23803e81e85 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java @@ -10219,6 +10219,11 @@ public class DiagnosticsTestGenerated extends AbstractDiagnosticsTest { runTest("compiler/testData/diagnostics/tests/inference/constraints/operationsOnIntegerValueTypes.kt"); } + @TestMetadata("recursiveJavaTypeWithStarProjection.kt") + public void testRecursiveJavaTypeWithStarProjection() throws Exception { + runTest("compiler/testData/diagnostics/tests/inference/constraints/recursiveJavaTypeWithStarProjection.kt"); + } + @TestMetadata("returnLambdaFromLambda.kt") public void testReturnLambdaFromLambda() throws Exception { runTest("compiler/testData/diagnostics/tests/inference/constraints/returnLambdaFromLambda.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsUsingJavacTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsUsingJavacTestGenerated.java index 5e03bb62aa3..1234fc3147a 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsUsingJavacTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsUsingJavacTestGenerated.java @@ -10214,6 +10214,11 @@ public class DiagnosticsUsingJavacTestGenerated extends AbstractDiagnosticsUsing runTest("compiler/testData/diagnostics/tests/inference/constraints/operationsOnIntegerValueTypes.kt"); } + @TestMetadata("recursiveJavaTypeWithStarProjection.kt") + public void testRecursiveJavaTypeWithStarProjection() throws Exception { + runTest("compiler/testData/diagnostics/tests/inference/constraints/recursiveJavaTypeWithStarProjection.kt"); + } + @TestMetadata("returnLambdaFromLambda.kt") public void testReturnLambdaFromLambda() throws Exception { runTest("compiler/testData/diagnostics/tests/inference/constraints/returnLambdaFromLambda.kt");