From 0a3c031528f71f5e9783d4e7380c7cc96df20b4c Mon Sep 17 00:00:00 2001 From: Ilya Chernikov Date: Tue, 28 Mar 2017 16:13:46 +0200 Subject: [PATCH] Optimize and simplify script deps cache and appropriate index rebuilding --- .../KotlinScriptExternalImportsProvider.kt | 93 ++++++++----------- .../KotlinScriptConfigurationManager.kt | 45 ++++----- 2 files changed, 56 insertions(+), 82 deletions(-) diff --git a/compiler/frontend/src/org/jetbrains/kotlin/script/KotlinScriptExternalImportsProvider.kt b/compiler/frontend/src/org/jetbrains/kotlin/script/KotlinScriptExternalImportsProvider.kt index 72670ce6751..fac33978ac5 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/script/KotlinScriptExternalImportsProvider.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/script/KotlinScriptExternalImportsProvider.kt @@ -28,8 +28,7 @@ import kotlin.concurrent.write class KotlinScriptExternalImportsProvider(val project: Project, private val scriptDefinitionProvider: KotlinScriptDefinitionProvider) { private val cacheLock = ReentrantReadWriteLock() - private val cache = hashMapOf() - private val cacheOfNulls = hashSetOf() + private val cache = hashMapOf() fun getExternalImports(file: TF): KotlinScriptExternalDependencies? = cacheLock.read { calculateExternalDependencies(file) } @@ -39,58 +38,62 @@ class KotlinScriptExternalImportsProvider(val project: Project, private val scri private fun calculateExternalDependencies(file: TF): KotlinScriptExternalDependencies? { val path = getFilePath(file) - return cache[path] - ?: if (cacheOfNulls.contains(path)) null - else scriptDefinitionProvider.findScriptDefinition(file)?.getDependenciesFor(file, project, null) - .apply { - if (this != null) { - log.info("[kts] new cached deps for $path: ${this.classpath.joinToString(File.pathSeparator)}") - } - cacheLock.write { - if (this == null) { - cacheOfNulls.add(path) - } - else { - cache.put(path, this) - } - } - } + val cached = cache[path] + return if (cached != null) cached else { + val scriptDef = scriptDefinitionProvider.findScriptDefinition(file) + if (scriptDef != null) { + val deps = scriptDef.getDependenciesFor(file, project, null) + if (deps != null) { + log.info("[kts] new cached deps for $path: ${deps.classpath.joinToString(File.pathSeparator)}") + } + cacheLock.write { + cache.put(path, deps) + } + deps + } + else null + } } // optimized for initial caching, additional handling of possible duplicates to save a call to distinct - fun cacheExternalImports(files: Iterable): Unit = cacheLock.write { + // returns list of cached files + fun cacheExternalImports(files: Iterable): Iterable = cacheLock.write { val uncached = hashSetOf() - files.forEach { file -> + files.mapNotNull { file -> val path = getFilePath(file) - if (isValidFile(file) && !cache.containsKey(path) && !cacheOfNulls.contains(path) && !uncached.contains(path)) { + if (isValidFile(file) && !cache.containsKey(path) && !uncached.contains(path)) { val scriptDef = scriptDefinitionProvider.findScriptDefinition(file) if (scriptDef != null) { val deps = scriptDef.getDependenciesFor(file, project, null) - log.info("[kts] cached deps for $path: ${deps?.classpath?.joinToString(File.pathSeparator)}") if (deps != null) { - cache.put(path, deps) - } - else { - cacheOfNulls.add(path) + log.info("[kts] cached deps for $path: ${deps.classpath.joinToString(File.pathSeparator)}") } + cache.put(path, deps) + file } else { uncached.add(path) + null } } + else null } } // optimized for update, no special duplicates handling + // returns files with valid script definition (or deleted from cache - which in fact should have script def too) + // TODO: this is the badly designed contract, since it mixes the entities, but these files are needed on the calling site now. Find out other solution fun updateExternalImportsCache(files: Iterable): Iterable = cacheLock.write { files.mapNotNull { file -> val path = getFilePath(file) if (!isValidFile(file)) { - if (cache.remove(path) != null || cacheOfNulls.remove(path)) { - log.info("[kts] removed deps for invalid file $path") + if (cache.remove(path) != null) { + log.debug("[kts] removed deps for file $path") file } // cleared - else null // unknown + else { + null // unknown + } } else { val scriptDef = scriptDefinitionProvider.findScriptDefinition(file) @@ -103,22 +106,17 @@ class KotlinScriptExternalImportsProvider(val project: Project, private val scri // changed or new log.info("[kts] updated/new cached deps for $path: ${deps.classpath.joinToString(File.pathSeparator)}") cache.put(path, deps) - cacheOfNulls.remove(path) - file } deps != null -> { // same as before - log.info("[kts] unchanged deps for $path") - null } else -> { - if (cache.remove(path) != null || cacheOfNulls.remove(path)) { - log.info("[kts] removed deps for $path") - file + if (cache.remove(path) != null) { + log.debug("[kts] removed deps for $path") } // cleared - else null // same as before } } + file } else null // not a script } @@ -126,30 +124,15 @@ class KotlinScriptExternalImportsProvider(val project: Project, private val scri } fun invalidateCaches() { - cacheLock.write { - cache.clear() - cacheOfNulls.clear() - } - } - - fun invalidateCachesFor(vararg files: TF) { invalidateCachesFor(files.asIterable()) } - - fun invalidateCachesFor(files: Iterable) { - cacheLock.write { - files.forEach { file -> - val path = getFilePath(file) - cache.remove(path) - cacheOfNulls.remove(path) - } - } + cacheLock.write(cache::clear) } fun getKnownCombinedClasspath(): List = cacheLock.read { - cache.values.flatMap { it.classpath } + cache.values.flatMap { it?.classpath ?: emptyList() } }.distinct() fun getKnownSourceRoots(): List = cacheLock.read { - cache.values.flatMap { it.sources } + cache.values.flatMap { it?.sources ?: emptyList() } }.distinct() fun getCombinedClasspathFor(files: Iterable): List = diff --git a/idea/idea-core/src/org/jetbrains/kotlin/idea/core/script/KotlinScriptConfigurationManager.kt b/idea/idea-core/src/org/jetbrains/kotlin/idea/core/script/KotlinScriptConfigurationManager.kt index 31d8d6669a7..1294f7404c6 100644 --- a/idea/idea-core/src/org/jetbrains/kotlin/idea/core/script/KotlinScriptConfigurationManager.kt +++ b/idea/idea-core/src/org/jetbrains/kotlin/idea/core/script/KotlinScriptConfigurationManager.kt @@ -50,7 +50,6 @@ import kotlin.concurrent.write @Suppress("SimplifyAssertNotNull") class KotlinScriptConfigurationManager( private val project: Project, - private val dumbService: DumbService, private val scriptDefinitionProvider: KotlinScriptDefinitionProvider, private val scriptExternalImportsProvider: KotlinScriptExternalImportsProvider ) { @@ -60,15 +59,16 @@ class KotlinScriptConfigurationManager( StartupManager.getInstance(project).runWhenProjectIsInitialized { DumbService.getInstance(project).smartInvokeLater { - cacheAllScriptsExtraImports() - invalidateLocalCaches() - notifyRootsChanged() + if (cacheAllScriptsExtraImports()) { + invalidateLocalCaches() + notifyRootsChanged() + } } } project.messageBus.connect().subscribe(VirtualFileManager.VFS_CHANGES, object : BulkFileListener.Adapter() { override fun after(events: List) { - updateExternalImportsCache(events.mapNotNull { it.file }) { + if (updateExternalImportsCache(events.mapNotNull { it.file })) { invalidateLocalCaches() notifyRootsChanged() } @@ -100,9 +100,7 @@ class KotlinScriptConfigurationManager( if (project.isDisposed) return@runWriteAction ProjectRootManagerEx.getInstanceEx(project)?.makeRootsChange(EmptyRunnable.getInstance(), false, true) - dumbService.runWhenSmart { - ScriptDependenciesModificationTracker.getInstance(project).incModificationCount() - } + ScriptDependenciesModificationTracker.getInstance(project).incModificationCount() } } @@ -128,28 +126,21 @@ class KotlinScriptConfigurationManager( fun getAllLibrarySourcesScope() = allLibrarySourcesScope.get() private fun reloadScriptDefinitions() { - makeScriptDefsFromTemplatesProviderExtensions(project, { ep, ex -> log.warn("[kts] Error loading definition from ${ep.id}", ex) }).let { - scriptDefinitionProvider.setScriptDefinitions(it) + val def = makeScriptDefsFromTemplatesProviderExtensions(project, { ep, ex -> log.warn("[kts] Error loading definition from ${ep.id}", ex) }) + scriptDefinitionProvider.setScriptDefinitions(def) + } + + private fun cacheAllScriptsExtraImports(): Boolean = runReadAction { + scriptExternalImportsProvider.run { + invalidateCaches() + cacheExternalImports( + scriptDefinitionProvider.getAllKnownFileTypes() + .flatMap { FileTypeIndex.getFiles(it, GlobalSearchScope.allScope(project)) } + ).any() } } - private fun cacheAllScriptsExtraImports() { - runReadAction { - scriptExternalImportsProvider.apply { - invalidateCaches() - cacheExternalImports( - scriptDefinitionProvider.getAllKnownFileTypes() - .flatMap { FileTypeIndex.getFiles(it, GlobalSearchScope.allScope(project)) }) - } - } - } - - private fun updateExternalImportsCache(files: Iterable, onChange: () -> Unit) { - val isChanged = scriptExternalImportsProvider.updateExternalImportsCache(files).any() - if (isChanged) { - onChange() - } - } + private fun updateExternalImportsCache(files: Iterable) = scriptExternalImportsProvider.updateExternalImportsCache(files).any() private fun invalidateLocalCaches() { allScriptsClasspathCache.clear()