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
This commit is contained in:
Mikhail Zarechenskiy
2019-10-10 09:42:19 +03:00
parent 71cb5e07db
commit 3d8e5c4f8e
6 changed files with 32 additions and 10 deletions
@@ -336,6 +336,9 @@ private class LazyJvmDiagnostics(compute: () -> Diagnostics) : Diagnostics {
override fun noSuppression() = delegate.noSuppression() override fun noSuppression() = delegate.noSuppression()
override fun hasDiagnostic(diagnostic: Diagnostic) =
delegate.hasDiagnostic(diagnostic)
override fun iterator() = delegate.iterator() override fun iterator() = delegate.iterator()
} }
@@ -112,9 +112,9 @@ fun ResolutionContext<*>.reportTypeMismatchDueToTypeProjection(
} }
fun BindingTrace.reportDiagnosticOnce(diagnostic: Diagnostic) { fun BindingTrace.reportDiagnosticOnce(diagnostic: Diagnostic) {
if (bindingContext.diagnostics.forElement(diagnostic.psiElement).any { it.factory == diagnostic.factory }) return if (!bindingContext.diagnostics.hasDiagnostic(diagnostic)) {
report(diagnostic)
report(diagnostic) }
} }
class TypeMismatchDueToTypeProjectionsData( class TypeMismatchDueToTypeProjectionsData(
@@ -93,5 +93,9 @@ class CompositeBindingContext private constructor(
override fun noSuppression(): Diagnostics { override fun noSuppression(): Diagnostics {
return CompositeDiagnostics(delegates.map { it.noSuppression() }) return CompositeDiagnostics(delegates.map { it.noSuppression() })
} }
override fun hasDiagnostic(diagnostic: Diagnostic): Boolean {
return delegates.any { it.hasDiagnostic(diagnostic) }
}
} }
} }
@@ -34,6 +34,10 @@ interface Diagnostics : Iterable<Diagnostic> {
fun noSuppression(): Diagnostics fun noSuppression(): Diagnostics
@JvmDefault
fun hasDiagnostic(diagnostic: Diagnostic): Boolean =
forElement(diagnostic.psiElement).any { it.factory == diagnostic.factory }
override fun iterator() = all().iterator() override fun iterator() = all().iterator()
companion object { companion object {
@@ -42,6 +46,7 @@ interface Diagnostics : Iterable<Diagnostic> {
override val modificationTracker: ModificationTracker = ModificationTracker.NEVER_CHANGED override val modificationTracker: ModificationTracker = ModificationTracker.NEVER_CHANGED
override fun all() = listOf<Diagnostic>() override fun all() = listOf<Diagnostic>()
override fun forElement(psiElement: PsiElement) = listOf<Diagnostic>() override fun forElement(psiElement: PsiElement) = listOf<Diagnostic>()
override fun hasDiagnostic(diagnostic: Diagnostic): Boolean = false
} }
} }
} }
@@ -17,11 +17,11 @@
package org.jetbrains.kotlin.resolve.diagnostics package org.jetbrains.kotlin.resolve.diagnostics
import org.jetbrains.kotlin.diagnostics.Diagnostic import org.jetbrains.kotlin.diagnostics.Diagnostic
import java.util.ArrayList
import com.intellij.openapi.util.CompositeModificationTracker import com.intellij.openapi.util.CompositeModificationTracker
import com.intellij.util.CachedValueImpl import com.intellij.util.CachedValueImpl
import com.intellij.psi.util.CachedValueProvider import com.intellij.psi.util.CachedValueProvider
import com.intellij.psi.PsiElement import com.intellij.psi.PsiElement
import com.intellij.util.SmartList
import org.jetbrains.annotations.TestOnly import org.jetbrains.annotations.TestOnly
import org.jetbrains.kotlin.resolve.BindingContext import org.jetbrains.kotlin.resolve.BindingContext
@@ -29,11 +29,12 @@ class MutableDiagnosticsWithSuppression @JvmOverloads constructor(
private val bindingContext: BindingContext, private val bindingContext: BindingContext,
private val delegateDiagnostics: Diagnostics = Diagnostics.EMPTY private val delegateDiagnostics: Diagnostics = Diagnostics.EMPTY
) : Diagnostics { ) : Diagnostics {
private val diagnosticList = ArrayList<Diagnostic>() // Usually, we don't report on an element more than one diagnostic, so it's better to use SmartList here
private val diagnosticMap = HashMap<PsiElement, SmartList<Diagnostic>>()
//NOTE: CachedValuesManager is not used because it requires Project passed to this object //NOTE: CachedValuesManager is not used because it requires Project passed to this object
private val cache = CachedValueImpl(CachedValueProvider { private val cache = CachedValueImpl(CachedValueProvider {
val allDiagnostics = delegateDiagnostics.noSuppression().all() + diagnosticList val allDiagnostics = delegateDiagnostics.noSuppression().all() + getOwnDiagnostics()
CachedValueProvider.Result(DiagnosticsWithSuppression(bindingContext, allDiagnostics), modificationTracker) CachedValueProvider.Result(DiagnosticsWithSuppression(bindingContext, allDiagnostics), modificationTracker)
}) })
@@ -47,19 +48,29 @@ class MutableDiagnosticsWithSuppression @JvmOverloads constructor(
//essential that this list is readonly //essential that this list is readonly
fun getOwnDiagnostics(): List<Diagnostic> { fun getOwnDiagnostics(): List<Diagnostic> {
return diagnosticList return diagnosticMap.values.flatten()
} }
fun report(diagnostic: Diagnostic) { fun report(diagnostic: Diagnostic) {
diagnosticList.add(diagnostic) diagnosticMap.getOrPut(diagnostic.psiElement, { SmartList() }).add(diagnostic)
modificationTracker.incModificationCount() modificationTracker.incModificationCount()
} }
fun clear() { fun clear() {
diagnosticList.clear() diagnosticMap.clear()
modificationTracker.incModificationCount() 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 @TestOnly
fun getReadonlyView(): DiagnosticsWithSuppression = readonlyView() fun getReadonlyView(): DiagnosticsWithSuppression = readonlyView()
} }
@@ -16,7 +16,6 @@
package org.jetbrains.kotlin.resolve.diagnostics package org.jetbrains.kotlin.resolve.diagnostics
import com.intellij.openapi.util.Condition
import com.intellij.psi.PsiElement import com.intellij.psi.PsiElement
import org.jetbrains.kotlin.diagnostics.Diagnostic import org.jetbrains.kotlin.diagnostics.Diagnostic
import java.util.ArrayList import java.util.ArrayList