From 842e2dc02facb77dd067ec7e84901a07f92c23af Mon Sep 17 00:00:00 2001 From: Denis Zharkov Date: Fri, 14 Feb 2020 15:10:59 +0300 Subject: [PATCH] Optimize retaining most specific methods in LazyJavaScope See 62a55b7b00e776fd666697251fcbe43f41fc3df3 Previously, it was working for O(n^2) Now, we first group it by jvm descriptor, then for each groups of size g_i finding the most specific for O(g_i^2) It should help for the cases when we have a lot of overloads with different JVM descriptors (modulo return type) NB: Having the same JVM descriptors is rather rare, because in Java one cannot generate such a class. Looks like it's only possible for Scala or some other JVM languages (KT-17560) It should help a lot for KT-35135 --- .../genericVarianceViolation/simple.fir.kt | 10 +++---- .../genericVarianceViolation/simple.kt | 10 +++---- .../genericVarianceViolation/simple.txt | 2 +- .../java/lazy/descriptors/LazyJavaScope.kt | 17 ++++++++--- .../kotlin/resolve/overridingUtils.kt | 30 ++++++++++++------- 5 files changed, 44 insertions(+), 25 deletions(-) diff --git a/compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/simple.fir.kt b/compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/simple.fir.kt index e272ad4a854..1e1af3fc634 100644 --- a/compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/simple.fir.kt +++ b/compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/simple.fir.kt @@ -11,7 +11,7 @@ public class A { void foo(Map x) {} void foo(Map.Entry x) {} - void foo(List> x) {} + void foo1(List> x) {} } // FILE: main.kt @@ -59,9 +59,9 @@ fun main( a.foo(me as Map.Entry) // Lists of lists - a.foo(mll) - a.foo(ll) - a.foo(mll as MutableList>) - a.foo(ll as List>) + a.foo1(mll) + a.foo1(ll) + a.foo1(mll as MutableList>) + a.foo1(ll as List>) } diff --git a/compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/simple.kt b/compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/simple.kt index 303e271f312..b180f8cead2 100644 --- a/compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/simple.kt +++ b/compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/simple.kt @@ -11,7 +11,7 @@ public class A { void foo(Map x) {} void foo(Map.Entry x) {} - void foo(List> x) {} + void foo1(List> x) {} } // FILE: main.kt @@ -59,9 +59,9 @@ fun main( a.foo(me as Map.Entry) // Lists of lists - a.foo(mll) - a.foo(ll) - a.foo(mll as MutableList>) - a.foo(ll as List>) + a.foo1(mll) + a.foo1(ll) + a.foo1(mll as MutableList>) + a.foo1(ll as List>) } diff --git a/compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/simple.txt b/compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/simple.txt index 865c30abc4f..620f091ecbe 100644 --- a/compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/simple.txt +++ b/compiler/testData/diagnostics/tests/platformTypes/genericVarianceViolation/simple.txt @@ -8,10 +8,10 @@ public open class A { public/*package*/ open fun foo(/*0*/ x: kotlin.collections.(Mutable)Iterable!): kotlin.Unit public/*package*/ open fun foo(/*0*/ x: kotlin.collections.(Mutable)Iterator!): kotlin.Unit public/*package*/ open fun foo(/*0*/ x: kotlin.collections.(Mutable)List!): kotlin.Unit - public/*package*/ open fun foo(/*0*/ x: kotlin.collections.(Mutable)List!>!): kotlin.Unit public/*package*/ open fun foo(/*0*/ x: kotlin.collections.(Mutable)Map.(Mutable)Entry!): kotlin.Unit public/*package*/ open fun foo(/*0*/ x: kotlin.collections.(Mutable)Map!): kotlin.Unit public/*package*/ open fun foo(/*0*/ x: kotlin.collections.(Mutable)Set!): kotlin.Unit + public/*package*/ open fun foo1(/*0*/ x: kotlin.collections.(Mutable)List!>!): kotlin.Unit public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String } diff --git a/core/descriptors.jvm/src/org/jetbrains/kotlin/load/java/lazy/descriptors/LazyJavaScope.kt b/core/descriptors.jvm/src/org/jetbrains/kotlin/load/java/lazy/descriptors/LazyJavaScope.kt index fea51ab2259..722ffa47c0b 100644 --- a/core/descriptors.jvm/src/org/jetbrains/kotlin/load/java/lazy/descriptors/LazyJavaScope.kt +++ b/core/descriptors.jvm/src/org/jetbrains/kotlin/load/java/lazy/descriptors/LazyJavaScope.kt @@ -35,12 +35,11 @@ import org.jetbrains.kotlin.load.java.structure.JavaArrayType import org.jetbrains.kotlin.load.java.structure.JavaField import org.jetbrains.kotlin.load.java.structure.JavaMethod import org.jetbrains.kotlin.load.java.structure.JavaValueParameter +import org.jetbrains.kotlin.load.kotlin.computeJvmDescriptor import org.jetbrains.kotlin.name.Name -import org.jetbrains.kotlin.resolve.DescriptorFactory -import org.jetbrains.kotlin.resolve.DescriptorUtils +import org.jetbrains.kotlin.resolve.* import org.jetbrains.kotlin.resolve.constants.StringValue import org.jetbrains.kotlin.resolve.descriptorUtil.firstArgument -import org.jetbrains.kotlin.resolve.retainMostSpecificInEachOverridableGroup import org.jetbrains.kotlin.resolve.scopes.DescriptorKindExclude.NonExtensions import org.jetbrains.kotlin.resolve.scopes.DescriptorKindFilter import org.jetbrains.kotlin.resolve.scopes.MemberScope @@ -116,13 +115,23 @@ abstract class LazyJavaScope( private val functions = c.storageManager.createMemoizedFunction> { name -> val result = LinkedHashSet(declaredFunctions(name)) - result.retainMostSpecificInEachOverridableGroup() + result.retainMostSpecificMethods() computeNonDeclaredFunctions(result, name) c.components.signatureEnhancement.enhanceSignatures(c, result).toList() } + private fun MutableSet.retainMostSpecificMethods() { + val groups = groupBy { it.computeJvmDescriptor(withReturnType = false) }.values + for (group in groups) { + if (group.size == 1) continue + val mostSpecificMethods = group.selectMostSpecificFromOverridableGroup() + removeAll(group) + addAll(mostSpecificMethods) + } + } + protected open fun JavaMethodDescriptor.isVisibleAsFunction() = true protected data class MethodSignatureData( diff --git a/core/descriptors/src/org/jetbrains/kotlin/resolve/overridingUtils.kt b/core/descriptors/src/org/jetbrains/kotlin/resolve/overridingUtils.kt index 1dfd6b638cc..d3e26ce1e50 100644 --- a/core/descriptors/src/org/jetbrains/kotlin/resolve/overridingUtils.kt +++ b/core/descriptors/src/org/jetbrains/kotlin/resolve/overridingUtils.kt @@ -66,12 +66,7 @@ fun Collection.selectMostSpecificInEachOverridableGroup( continue } - val mostSpecific = OverridingUtil.selectMostSpecificMember(overridableGroup, descriptorByHandle) - val mostSpecificDescriptor = mostSpecific.descriptorByHandle() - - overridableGroup.filterNotTo(conflictedHandles) { - OverridingUtil.isMoreSpecific(mostSpecificDescriptor, it.descriptorByHandle()) - } + val mostSpecific = mostSpecific(overridableGroup, conflictedHandles, descriptorByHandle) if (conflictedHandles.isNotEmpty()) { result.addAll(conflictedHandles) @@ -82,8 +77,23 @@ fun Collection.selectMostSpecificInEachOverridableGroup( return result } -fun MutableCollection.retainMostSpecificInEachOverridableGroup() { - val newResult = selectMostSpecificInEachOverridableGroup { this } - if (size == newResult.size) return - retainAll(newResult) +private fun mostSpecific( + overridableGroup: Collection, + conflictedHandles: MutableSet, + descriptorByHandle: H.() -> CallableDescriptor +): H { + val mostSpecific = OverridingUtil.selectMostSpecificMember(overridableGroup, descriptorByHandle) + val mostSpecificDescriptor = mostSpecific.descriptorByHandle() + + overridableGroup.filterNotTo(conflictedHandles) { + OverridingUtil.isMoreSpecific(mostSpecificDescriptor, it.descriptorByHandle()) + } + return mostSpecific +} + +// Return multiple elements in case of conflicts +fun Collection.selectMostSpecificFromOverridableGroup(): Collection { + val result = mutableSetOf() + result.add(mostSpecific(this, conflictedHandles = result) { this }) + return result }