From df74fb23f8dea6c1243813412ce53ac2899ac3b8 Mon Sep 17 00:00:00 2001 From: "Pavel V. Talanov" Date: Thu, 19 Oct 2017 15:29:54 +0300 Subject: [PATCH] ScriptDependenciesCache: limit simultaneously cached script count Do not schedule updates for dependencies of scripts that were not requested (opened in the editor or such) for some time This fixes a problem when vcs updates changing scripts can trigger undesired script dependencies updates (probable cause of script files never highlighted correctly in some cases) Refactor: use VirtualFile instead of String and move code from GradleScriptTemplateProvider --- .../core/script/ScriptDependenciesCache.kt | 44 ++++++++++++++----- .../core/script/ScriptDependenciesUpdater.kt | 13 +++--- .../script/GradleScriptTemplateProvider.kt | 21 +-------- 3 files changed, 44 insertions(+), 34 deletions(-) diff --git a/idea/idea-core/src/org/jetbrains/kotlin/idea/core/script/ScriptDependenciesCache.kt b/idea/idea-core/src/org/jetbrains/kotlin/idea/core/script/ScriptDependenciesCache.kt index e06a4660865..da6f544cc4f 100644 --- a/idea/idea-core/src/org/jetbrains/kotlin/idea/core/script/ScriptDependenciesCache.kt +++ b/idea/idea-core/src/org/jetbrains/kotlin/idea/core/script/ScriptDependenciesCache.kt @@ -24,8 +24,10 @@ import com.intellij.openapi.vfs.VirtualFile import com.intellij.psi.PsiElementFinder import com.intellij.psi.PsiManager import com.intellij.psi.search.NonClasspathDirectoriesScope +import com.intellij.util.containers.SLRUMap import kotlinx.coroutines.experimental.launch import org.jetbrains.kotlin.idea.core.util.EDT +import java.io.File import java.util.concurrent.locks.ReentrantReadWriteLock import kotlin.concurrent.read import kotlin.concurrent.write @@ -35,14 +37,16 @@ import kotlin.reflect.KProperty0 import kotlin.reflect.jvm.isAccessible import kotlin.script.experimental.dependencies.ScriptDependencies +private const val MAX_SCRIPTS_CACHED = 15 + class ScriptDependenciesCache(private val project: Project) { private val cacheLock = ReentrantReadWriteLock() - private val cache = hashMapOf() + private val cache = SLRUMap(0, MAX_SCRIPTS_CACHED) - operator fun get(virtualFile: VirtualFile): ScriptDependencies? = cacheLock.read { cache[virtualFile.path] } + operator fun get(virtualFile: VirtualFile): ScriptDependencies? = cacheLock.read { cache[virtualFile] } val allScriptsClasspath by ClearableLazyValue(cacheLock) { - val files = cache.values.flatMap { it.classpath }.distinct() + val files = cache.entrySet().flatMap { it.value.classpath }.distinct() ScriptDependenciesManager.toVfsRoots(files) } @@ -51,15 +55,13 @@ class ScriptDependenciesCache(private val project: Project) { } val allLibrarySources by ClearableLazyValue(cacheLock) { - ScriptDependenciesManager.toVfsRoots(cache.values.flatMap { it.sources }.distinct()) + ScriptDependenciesManager.toVfsRoots(cache.entrySet().flatMap { it.value.sources }.distinct()) } val allLibrarySourcesScope by ClearableLazyValue(cacheLock) { NonClasspathDirectoriesScope(allLibrarySources) } - fun inspectCache(body: (Map) -> T): T = cacheLock.read { body(cache) } - private fun onChange(file: VirtualFile?) { this::allScriptsClasspath.clearValue() this::allScriptsClasspathScope.clearValue() @@ -102,10 +104,9 @@ class ScriptDependenciesCache(private val project: Project) { } fun save(virtualFile: VirtualFile, new: ScriptDependencies): Boolean { - val path = virtualFile.path val old = cacheLock.write { - val old = cache[path] - cache[path] = new + val old = cache[virtualFile] + cache.put(virtualFile, new) old } val changed = new != old @@ -118,13 +119,36 @@ class ScriptDependenciesCache(private val project: Project) { fun delete(virtualFile: VirtualFile): Boolean { val changed = cacheLock.write { - cache.remove(virtualFile.path) != null + cache.remove(virtualFile) != null } if (changed) { onChange(virtualFile) } return changed } + + fun combineDependencies(filePredicate: (VirtualFile) -> Boolean): ScriptDependencies = cacheLock.read { + val sources = mutableListOf() + val binaries = mutableListOf() + val imports = mutableListOf() + val scripts = mutableListOf() + + val relevantEntries = cache.entrySet().filter { filePredicate(it.key) }.map { it.value } + relevantEntries.forEach { + sources += it.sources + binaries += it.classpath + imports += it.imports + scripts += it.scripts + } + + return ScriptDependencies( + classpath = binaries.distinct(), + sources = sources.distinct(), + imports = imports.distinct(), + scripts = scripts.distinct(), + javaHome = relevantEntries.map { it.javaHome }.firstOrNull() + ) + } } private fun KProperty0.clearValue() { diff --git a/idea/idea-core/src/org/jetbrains/kotlin/idea/core/script/ScriptDependenciesUpdater.kt b/idea/idea-core/src/org/jetbrains/kotlin/idea/core/script/ScriptDependenciesUpdater.kt index 47559619e70..e156566ff97 100644 --- a/idea/idea-core/src/org/jetbrains/kotlin/idea/core/script/ScriptDependenciesUpdater.kt +++ b/idea/idea-core/src/org/jetbrains/kotlin/idea/core/script/ScriptDependenciesUpdater.kt @@ -84,7 +84,7 @@ internal class ScriptDependenciesUpdater( tryUsingDefault(file) } - updateCache(listOf(file)) + performUpdate(file) return cache[file] ?: ScriptDependencies.Empty } @@ -108,17 +108,20 @@ internal class ScriptDependenciesUpdater( } } - private fun updateCache(files: Iterable) = + private fun requestUpdate(files: Iterable) = files.map { file -> if (!file.isValid) { return cache.delete(file) } + else if (cache[file] != null) { // only update dependencies for scripts that were touched recently + performUpdate(file) + } else { - updateForFile(file) + false } }.contains(true) - private fun updateForFile(file: VirtualFile): Boolean { + private fun performUpdate(file: VirtualFile): Boolean { val scriptDef = scriptDefinitionProvider.findScriptDefinition(file) ?: return false return when (scriptDef.dependencyResolver) { @@ -263,7 +266,7 @@ internal class ScriptDependenciesUpdater( return } - if (updateCache(events.mapNotNull { + if (requestUpdate(events.mapNotNull { // The check is partly taken from the BuildManager.java it.file?.takeIf { // the isUnitTestMode check fixes ScriptConfigurationHighlighting & Navigation tests, since they are not trigger proper update mechanims diff --git a/idea/idea-gradle/src/org/jetbrains/kotlin/idea/core/script/GradleScriptTemplateProvider.kt b/idea/idea-gradle/src/org/jetbrains/kotlin/idea/core/script/GradleScriptTemplateProvider.kt index 1ca71fb8047..d2081fb0baf 100644 --- a/idea/idea-gradle/src/org/jetbrains/kotlin/idea/core/script/GradleScriptTemplateProvider.kt +++ b/idea/idea-gradle/src/org/jetbrains/kotlin/idea/core/script/GradleScriptTemplateProvider.kt @@ -181,23 +181,6 @@ class GradleScriptDefaultDependenciesProvider( return previouslyAnalyzedScriptsCombinedDependencies().takeUnless { it.classpath.isEmpty() } } - private fun previouslyAnalyzedScriptsCombinedDependencies(): ScriptDependencies { - val sources = mutableListOf() - val binaries = mutableListOf() - val imports = mutableListOf() - - scriptDependenciesCache.inspectCache { cache -> - cache.entries.filter { it.key.endsWith(KOTLIN_BUILD_FILE_NAME) } - .map { it.value }.forEach { - sources += it.sources - binaries += it.classpath - imports += it.imports - } - } - return ScriptDependencies( - classpath = binaries.distinct(), - sources = sources.distinct(), - imports = imports.distinct() - ) - } + private fun previouslyAnalyzedScriptsCombinedDependencies() = + scriptDependenciesCache.combineDependencies { it.name == KOTLIN_BUILD_FILE_NAME } }