From 3d8e5c4f8ea920f111549aece083290b35392d41 Mon Sep 17 00:00:00 2001 From: Mikhail Zarechenskiy Date: Thu, 10 Oct 2019 09:42:19 +0300 Subject: [PATCH] Fix performance issue for case when there are a lot of warnings in file There was a problem with `reportDiagnosticOnce` method which was called, for example, to report diagnostics about experimentalities. It was crucial for code as in the issue (KT-34204). The whole complexity was "count of experimental diagnostics" multiply "count of all diagnostics" multiply very large constant. Almost on each `reportDiagnosticOnce` method `readonlyView()` was called which in turn called computation of cache. During cache computation we were iterating through the all diagnostics and also were using KotlinSuppressCache, which is not very fast. #KT-34204 Fixed --- .../kotlin/codegen/state/GenerationState.kt | 3 +++ .../kotlin/diagnostics/diagnosticUtils.kt | 6 ++--- .../kotlin/resolve/CompositeBindingContext.kt | 4 ++++ .../kotlin/resolve/diagnostics/Diagnostics.kt | 5 ++++ .../MutableDiagnosticsWithSuppression.kt | 23 ++++++++++++++----- .../resolve/diagnostics/SimpleDiagnostics.kt | 1 - 6 files changed, 32 insertions(+), 10 deletions(-) diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/state/GenerationState.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/state/GenerationState.kt index 86a5c808a40..a897b4c5f4e 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/state/GenerationState.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/state/GenerationState.kt @@ -336,6 +336,9 @@ private class LazyJvmDiagnostics(compute: () -> Diagnostics) : Diagnostics { override fun noSuppression() = delegate.noSuppression() + override fun hasDiagnostic(diagnostic: Diagnostic) = + delegate.hasDiagnostic(diagnostic) + override fun iterator() = delegate.iterator() } diff --git a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/diagnosticUtils.kt b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/diagnosticUtils.kt index c6a4957c83f..b3cc4d33ef0 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/diagnosticUtils.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/diagnostics/diagnosticUtils.kt @@ -112,9 +112,9 @@ fun ResolutionContext<*>.reportTypeMismatchDueToTypeProjection( } fun BindingTrace.reportDiagnosticOnce(diagnostic: Diagnostic) { - if (bindingContext.diagnostics.forElement(diagnostic.psiElement).any { it.factory == diagnostic.factory }) return - - report(diagnostic) + if (!bindingContext.diagnostics.hasDiagnostic(diagnostic)) { + report(diagnostic) + } } class TypeMismatchDueToTypeProjectionsData( diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/CompositeBindingContext.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/CompositeBindingContext.kt index c4aee43a64f..fdeadd1fca8 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/CompositeBindingContext.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/CompositeBindingContext.kt @@ -93,5 +93,9 @@ class CompositeBindingContext private constructor( override fun noSuppression(): Diagnostics { return CompositeDiagnostics(delegates.map { it.noSuppression() }) } + + override fun hasDiagnostic(diagnostic: Diagnostic): Boolean { + return delegates.any { it.hasDiagnostic(diagnostic) } + } } } diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/diagnostics/Diagnostics.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/diagnostics/Diagnostics.kt index da83ac5b40c..811b6e91994 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/diagnostics/Diagnostics.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/diagnostics/Diagnostics.kt @@ -34,6 +34,10 @@ interface Diagnostics : Iterable { fun noSuppression(): Diagnostics + @JvmDefault + fun hasDiagnostic(diagnostic: Diagnostic): Boolean = + forElement(diagnostic.psiElement).any { it.factory == diagnostic.factory } + override fun iterator() = all().iterator() companion object { @@ -42,6 +46,7 @@ interface Diagnostics : Iterable { override val modificationTracker: ModificationTracker = ModificationTracker.NEVER_CHANGED override fun all() = listOf() override fun forElement(psiElement: PsiElement) = listOf() + override fun hasDiagnostic(diagnostic: Diagnostic): Boolean = false } } } diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/diagnostics/MutableDiagnosticsWithSuppression.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/diagnostics/MutableDiagnosticsWithSuppression.kt index 47b3f4934ce..b414bd80e08 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/diagnostics/MutableDiagnosticsWithSuppression.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/diagnostics/MutableDiagnosticsWithSuppression.kt @@ -17,11 +17,11 @@ package org.jetbrains.kotlin.resolve.diagnostics import org.jetbrains.kotlin.diagnostics.Diagnostic -import java.util.ArrayList import com.intellij.openapi.util.CompositeModificationTracker import com.intellij.util.CachedValueImpl import com.intellij.psi.util.CachedValueProvider import com.intellij.psi.PsiElement +import com.intellij.util.SmartList import org.jetbrains.annotations.TestOnly import org.jetbrains.kotlin.resolve.BindingContext @@ -29,11 +29,12 @@ class MutableDiagnosticsWithSuppression @JvmOverloads constructor( private val bindingContext: BindingContext, private val delegateDiagnostics: Diagnostics = Diagnostics.EMPTY ) : Diagnostics { - private val diagnosticList = ArrayList() + // Usually, we don't report on an element more than one diagnostic, so it's better to use SmartList here + private val diagnosticMap = HashMap>() //NOTE: CachedValuesManager is not used because it requires Project passed to this object private val cache = CachedValueImpl(CachedValueProvider { - val allDiagnostics = delegateDiagnostics.noSuppression().all() + diagnosticList + val allDiagnostics = delegateDiagnostics.noSuppression().all() + getOwnDiagnostics() CachedValueProvider.Result(DiagnosticsWithSuppression(bindingContext, allDiagnostics), modificationTracker) }) @@ -47,19 +48,29 @@ class MutableDiagnosticsWithSuppression @JvmOverloads constructor( //essential that this list is readonly fun getOwnDiagnostics(): List { - return diagnosticList + return diagnosticMap.values.flatten() } fun report(diagnostic: Diagnostic) { - diagnosticList.add(diagnostic) + diagnosticMap.getOrPut(diagnostic.psiElement, { SmartList() }).add(diagnostic) modificationTracker.incModificationCount() } fun clear() { - diagnosticList.clear() + diagnosticMap.clear() modificationTracker.incModificationCount() } + override fun hasDiagnostic(diagnostic: Diagnostic): Boolean { + if (cache.hasUpToDateValue()) return cache.value.hasDiagnostic(diagnostic) + if (delegateDiagnostics.hasDiagnostic(diagnostic)) return true + + // It's totally fine to check without any suppression logic as if passed diagnostic has the same factory & psiElement, + // then it has the same suppression logic, so it doesn't matter if we add it or not later + val list = diagnosticMap[diagnostic.psiElement] ?: return false + return list.any { it.factory == diagnostic.factory && it.psiElement == diagnostic.psiElement } + } + @TestOnly fun getReadonlyView(): DiagnosticsWithSuppression = readonlyView() } diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/diagnostics/SimpleDiagnostics.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/diagnostics/SimpleDiagnostics.kt index 6c4af145141..4de1bd9f889 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/diagnostics/SimpleDiagnostics.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/diagnostics/SimpleDiagnostics.kt @@ -16,7 +16,6 @@ package org.jetbrains.kotlin.resolve.diagnostics -import com.intellij.openapi.util.Condition import com.intellij.psi.PsiElement import org.jetbrains.kotlin.diagnostics.Diagnostic import java.util.ArrayList