[MPP] Improve performance of ExpectedActualDeclarationChecker.kt

A project like OKIO might have a lot (~700) module-dependsOn-paths
Prior this commit `findActualForExpected` was called for all modules
in all said module-dependsOn-paths  which resulted in several thousand
invocations.

The new approach is to call `findActualForExpected` only for known
leaf modules to find all reachable actuals. From there, the actuals
get associated with intermediate modules in this paths and reported
accordingly.

^KT-50156 Verification Pending
This commit is contained in:
sebastian.sellmair
2021-12-22 09:30:38 +01:00
committed by Space
parent 8324330823
commit 5e657784db
@@ -36,7 +36,6 @@ import org.jetbrains.kotlin.resolve.source.KotlinSourceElement
import org.jetbrains.kotlin.resolve.source.PsiSourceFile import org.jetbrains.kotlin.resolve.source.PsiSourceFile
import org.jetbrains.kotlin.types.KotlinType import org.jetbrains.kotlin.types.KotlinType
import org.jetbrains.kotlin.utils.addToStdlib.safeAs import org.jetbrains.kotlin.utils.addToStdlib.safeAs
import org.jetbrains.kotlin.utils.ifEmpty
import java.io.File import java.io.File
class ExpectedActualDeclarationChecker( class ExpectedActualDeclarationChecker(
@@ -93,20 +92,34 @@ class ExpectedActualDeclarationChecker(
expectActualTracker: ExpectActualTracker expectActualTracker: ExpectActualTracker
) { ) {
val allActualizationPaths = moduleStructureOracle.findAllReversedDependsOnPaths(descriptor.module) val allActualizationPaths = moduleStructureOracle.findAllReversedDependsOnPaths(descriptor.module)
val leafModuleToVisibleModules = allActualizationPaths.groupBy { it.nodes.last() } val allLeafModules = allActualizationPaths.map { it.nodes.last() }.toSet()
.mapValues { it.value.flatMap { it.nodes }.toSet() }
allLeafModules.forEach { leafModule ->
val actuals = ExpectedActualResolver.findActualForExpected(descriptor, leafModule) ?: return@forEach
for ((leafModule, modulesVisibleFromLeaf) in leafModuleToVisibleModules) {
checkExpectedDeclarationHasAtLeastOneActual( checkExpectedDeclarationHasAtLeastOneActual(
reportOn, reportOn, descriptor, actuals, trace, leafModule, checkActualModifier, expectActualTracker
descriptor, )
trace,
leafModule, checkExpectedDeclarationHasAtMostOneActual(
checkActualModifier, reportOn, descriptor, actuals, allActualizationPaths, trace
expectActualTracker,
moduleVisibilityFilter = { it in modulesVisibleFromLeaf }
) )
} }
}
private fun checkExpectedDeclarationHasAtMostOneActual(
reportOn: KtNamedDeclaration,
expectDescriptor: MemberDescriptor,
actuals: ActualsMap,
modulePaths: List<ModulePath>,
trace: BindingTrace,
) {
val atLeastWeaklyCompatibleActuals = actuals
.filterKeys { compatibility -> compatibility.isCompatibleOrWeakCompatible() }
.values.flatten()
// Eagerly return here: We won't find a duplicate in any module path in this case
if (atLeastWeaklyCompatibleActuals.size <= 1) return
/* /*
Note that we have to check for 'duplicate actuals' separately, considering paths Note that we have to check for 'duplicate actuals' separately, considering paths
@@ -125,76 +138,51 @@ class ExpectedActualDeclarationChecker(
If we merge behaviour (e.g. decide to report ERROR for first case too) If we merge behaviour (e.g. decide to report ERROR for first case too)
for those two cases, we can drop separate logic for DUPLICATE_ACTUALS for those two cases, we can drop separate logic for DUPLICATE_ACTUALS
*/ */
for (path in allActualizationPaths) { val actualsByModulePath = modulePaths.associateWith { path ->
val modulesOnThisPath = path.nodes.toSet() atLeastWeaklyCompatibleActuals.filter { it.module in path.nodes }
checkExpectedDeclarationHasAtMostOneActual(
reportOn, descriptor, trace, path, moduleVisibilityFilter = { it in modulesOnThisPath }
)
} }
}
private fun checkExpectedDeclarationHasAtMostOneActual( actualsByModulePath.forEach { (_, actualsInPath) ->
reportOn: KtNamedDeclaration, if (actualsInPath.size > 1) {
descriptor: MemberDescriptor, trace.report(Errors.AMBIGUOUS_ACTUALS.on(
trace: BindingTrace, reportOn,
path: ModulePath, expectDescriptor,
moduleVisibilityFilter: ModuleFilter actualsInPath
) { .map { it.module }
val compatibility = path.nodes .sortedBy { it.name.asString() }
.mapNotNull { ExpectedActualResolver.findActualForExpected(descriptor, it, moduleVisibilityFilter) } ))
.ifEmpty { return }
.fold(LinkedHashMap<ExpectActualCompatibility<MemberDescriptor>, List<MemberDescriptor>>()) { resultMap, partialMap ->
resultMap.apply { putAll(partialMap) }
} }
// Several compatible actuals on one path: report AMBIGUIOUS_ACTUALS here
val atLeastWeaklyCompatibleActuals = compatibility
.filterKeys { it.isCompatibleOrWeakCompatible() }
.values
.flatten()
.distinct()
if (atLeastWeaklyCompatibleActuals.size > 1) {
trace.report(Errors.AMBIGUOUS_ACTUALS.on(
reportOn,
descriptor,
atLeastWeaklyCompatibleActuals
.map { it.module }
.sortedBy { it.name.asString() }
))
} }
} }
private fun checkExpectedDeclarationHasAtLeastOneActual( private fun checkExpectedDeclarationHasAtLeastOneActual(
reportOn: KtNamedDeclaration, reportOn: KtNamedDeclaration,
descriptor: MemberDescriptor, expectDescriptor: MemberDescriptor,
actuals: ActualsMap,
trace: BindingTrace, trace: BindingTrace,
module: ModuleDescriptor, module: ModuleDescriptor,
checkActualModifier: Boolean, checkActualModifier: Boolean,
expectActualTracker: ExpectActualTracker, expectActualTracker: ExpectActualTracker
moduleVisibilityFilter: ModuleFilter
) { ) {
// Only look for top level actual members; class members will be handled as a part of that expected class // Only look for top level actual members; class members will be handled as a part of that expected class
if (descriptor.containingDeclaration !is PackageFragmentDescriptor) return if (expectDescriptor.containingDeclaration !is PackageFragmentDescriptor) return
val compatibility = ExpectedActualResolver.findActualForExpected(descriptor, module, moduleVisibilityFilter) ?: return
// Only strong incompatibilities, but this is an OptionalExpectation -- don't report it // Only strong incompatibilities, but this is an OptionalExpectation -- don't report it
if (compatibility.allStrongIncompatibilities() && OptionalAnnotationUtil.isOptionalAnnotationClass(descriptor)) return if (actuals.allStrongIncompatibilities() && OptionalAnnotationUtil.isOptionalAnnotationClass(expectDescriptor)) return
// Only strong incompatibilities, or error won't be reported on actual: report NO_ACTUAL_FOR_EXPECT here // Only strong incompatibilities, or error won't be reported on actual: report NO_ACTUAL_FOR_EXPECT here
if (compatibility.allStrongIncompatibilities() || if (actuals.allStrongIncompatibilities() ||
Compatible !in compatibility && descriptor.hasNoActualWithDiagnostic(compatibility) Compatible !in actuals && expectDescriptor.hasNoActualWithDiagnostic(actuals)
) { ) {
assert(compatibility.keys.all { it is Incompatible }) assert(actuals.keys.all { it is Incompatible })
@Suppress("UNCHECKED_CAST") @Suppress("UNCHECKED_CAST")
val incompatibility = compatibility as Map<Incompatible<MemberDescriptor>, Collection<MemberDescriptor>> val incompatibility = actuals as Map<Incompatible<MemberDescriptor>, Collection<MemberDescriptor>>
trace.report(Errors.NO_ACTUAL_FOR_EXPECT.on(reportOn, descriptor, module, incompatibility)) trace.report(Errors.NO_ACTUAL_FOR_EXPECT.on(reportOn, expectDescriptor, module, incompatibility))
return return
} }
// Here we have exactly one compatible actual and/or some weakly incompatible. In either case, we don't report anything on expect... // Here we have exactly one compatible actual and/or some weakly incompatible. In either case, we don't report anything on expect...
val actualMembers = compatibility.asSequence() val actualMembers = actuals.asSequence()
.filter { it.key.isCompatibleOrWeakCompatible() }.flatMap { it.value.asSequence() } .filter { it.key.isCompatibleOrWeakCompatible() }.flatMap { it.value.asSequence() }
// ...except diagnostics regarding missing actual keyword, because in that case we won't start looking for the actual at all // ...except diagnostics regarding missing actual keyword, because in that case we won't start looking for the actual at all
@@ -202,7 +190,7 @@ class ExpectedActualDeclarationChecker(
actualMembers.forEach { reportMissingActualModifier(it, reportOn = null, trace) } actualMembers.forEach { reportMissingActualModifier(it, reportOn = null, trace) }
} }
expectActualTracker.reportExpectActual(expected = descriptor, actualMembers = actualMembers) expectActualTracker.reportExpectActual(expected = expectDescriptor, actualMembers = actualMembers)
} }
private fun reportMissingActualModifier(actual: MemberDescriptor, reportOn: KtNamedDeclaration?, trace: BindingTrace) { private fun reportMissingActualModifier(actual: MemberDescriptor, reportOn: KtNamedDeclaration?, trace: BindingTrace) {
@@ -415,3 +403,5 @@ class ExpectedActualDeclarationChecker(
this.keys.all { it is Incompatible && it.kind == IncompatibilityKind.STRONG } this.keys.all { it is Incompatible && it.kind == IncompatibilityKind.STRONG }
} }
} }
private typealias ActualsMap = Map<ExpectActualCompatibility<MemberDescriptor>, List<MemberDescriptor>>