From b8086d9093bc979326dfb43a7fac4dbcf96f8246 Mon Sep 17 00:00:00 2001 From: Natalia Selezneva Date: Thu, 15 Nov 2018 12:44:31 +0300 Subject: [PATCH] Do not start multiple background threads loading dependencies for different scripts ^KT-27743 Fixed --- .../core/script/ScriptDependenciesManager.kt | 2 +- .../core/script/ScriptDependenciesUpdater.kt | 28 +++- .../AsyncScriptDependenciesLoader.kt | 158 ++++++++---------- ...omFileAttributeScriptDependenciesLoader.kt | 13 +- .../dependencies/ScriptDependenciesLoader.kt | 74 +++----- .../SyncScriptDependenciesLoader.kt | 12 +- 6 files changed, 120 insertions(+), 167 deletions(-) diff --git a/idea/idea-core/src/org/jetbrains/kotlin/idea/core/script/ScriptDependenciesManager.kt b/idea/idea-core/src/org/jetbrains/kotlin/idea/core/script/ScriptDependenciesManager.kt index 332d2ccd2ff..1e9f5b309ab 100644 --- a/idea/idea-core/src/org/jetbrains/kotlin/idea/core/script/ScriptDependenciesManager.kt +++ b/idea/idea-core/src/org/jetbrains/kotlin/idea/core/script/ScriptDependenciesManager.kt @@ -77,7 +77,7 @@ class ScriptDependenciesManager internal constructor( @TestOnly fun updateScriptDependenciesSynchronously(virtualFile: VirtualFile, project: Project) { val scriptDefinition = findScriptDefinition(virtualFile, project)!! - SyncScriptDependenciesLoader(virtualFile, scriptDefinition, project).updateDependencies() + SyncScriptDependenciesLoader(project).updateDependencies(virtualFile, scriptDefinition) } } } 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 8e87cc98715..326260f810a 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 @@ -32,12 +32,16 @@ import com.intellij.psi.PsiManager import com.intellij.util.Alarm import org.jetbrains.annotations.TestOnly import org.jetbrains.kotlin.idea.KotlinFileType +import org.jetbrains.kotlin.idea.core.script.dependencies.AsyncScriptDependenciesLoader import org.jetbrains.kotlin.idea.core.script.dependencies.FromFileAttributeScriptDependenciesLoader -import org.jetbrains.kotlin.idea.core.script.dependencies.ScriptDependenciesLoader +import org.jetbrains.kotlin.idea.core.script.dependencies.SyncScriptDependenciesLoader import org.jetbrains.kotlin.idea.util.ProjectRootsUtil import org.jetbrains.kotlin.psi.KtFile import org.jetbrains.kotlin.psi.NotNullableUserDataProperty +import org.jetbrains.kotlin.script.KotlinScriptDefinition +import org.jetbrains.kotlin.script.LegacyResolverWrapper import org.jetbrains.kotlin.script.findScriptDefinition +import kotlin.script.experimental.dependencies.AsyncDependenciesResolver import kotlin.script.experimental.dependencies.ScriptDependencies class ScriptDependenciesUpdater( @@ -47,6 +51,10 @@ class ScriptDependenciesUpdater( private val scriptsQueue = Alarm(Alarm.ThreadToUse.SWING_THREAD, project) private val scriptChangesListenerDelay = 1400 + private val asyncLoader = AsyncScriptDependenciesLoader(project) + private val syncLoader = SyncScriptDependenciesLoader(project) + private val fileAttributeLoader = FromFileAttributeScriptDependenciesLoader(project) + init { listenForChangesInScripts() } @@ -56,12 +64,21 @@ class ScriptDependenciesUpdater( val scriptDef = findScriptDefinition(file, project) ?: return ScriptDependencies.Empty - FromFileAttributeScriptDependenciesLoader(file, scriptDef, project).updateDependencies() - ScriptDependenciesLoader.updateDependencies(file, scriptDef, project) + fileAttributeLoader.updateDependencies(file, scriptDef) + + updateDependencies(file, scriptDef) return cache[file] ?: ScriptDependencies.Empty } + private fun updateDependencies(file: VirtualFile, scriptDef: KotlinScriptDefinition) { + val loader = when (scriptDef.dependencyResolver) { + is AsyncDependenciesResolver, is LegacyResolverWrapper -> asyncLoader + else -> syncLoader + } + loader.updateDependencies(file, scriptDef) + } + private fun listenForChangesInScripts() { project.messageBus.connect().subscribe(FileEditorManagerListener.FILE_EDITOR_MANAGER, object : FileEditorManagerListener { override fun fileOpened(source: FileEditorManager, file: VirtualFile) { @@ -81,7 +98,8 @@ class ScriptDependenciesUpdater( val scriptDef = ktFile.script?.kotlinScriptDefinition ?: return if (!ProjectRootsUtil.isInProjectSource(ktFile, includeScriptsOutsideSourceRoots = true)) return - ScriptDependenciesLoader.updateDependencies(file, scriptDef, project) + + updateDependencies(file, scriptDef) } }) @@ -113,7 +131,7 @@ class ScriptDependenciesUpdater( scriptsQueue.addRequest( { FileDocumentManager.getInstance().saveDocument(document) - ScriptDependenciesLoader.updateDependencies(file, scriptDef, project) + updateDependencies(file, scriptDef) }, scriptChangesListenerDelay, true diff --git a/idea/idea-core/src/org/jetbrains/kotlin/idea/core/script/dependencies/AsyncScriptDependenciesLoader.kt b/idea/idea-core/src/org/jetbrains/kotlin/idea/core/script/dependencies/AsyncScriptDependenciesLoader.kt index 50bd136dfca..8f83c860add 100644 --- a/idea/idea-core/src/org/jetbrains/kotlin/idea/core/script/dependencies/AsyncScriptDependenciesLoader.kt +++ b/idea/idea-core/src/org/jetbrains/kotlin/idea/core/script/dependencies/AsyncScriptDependenciesLoader.kt @@ -5,128 +5,104 @@ package org.jetbrains.kotlin.idea.core.script.dependencies +import com.intellij.openapi.application.runReadAction +import com.intellij.openapi.progress.ProgressIndicator +import com.intellij.openapi.progress.Task +import com.intellij.openapi.progress.util.BackgroundTaskUtil import com.intellij.openapi.project.Project import com.intellij.openapi.vfs.VirtualFile -import kotlinx.coroutines.* +import kotlinx.coroutines.Runnable +import kotlinx.coroutines.runBlocking import org.jetbrains.kotlin.idea.core.script.settings.KotlinScriptingSettings -import org.jetbrains.kotlin.idea.core.util.cancelOnDisposal import org.jetbrains.kotlin.script.KotlinScriptDefinition import org.jetbrains.kotlin.script.LegacyResolverWrapper import org.jetbrains.kotlin.script.asResolveFailure -import java.util.concurrent.Executors +import org.jetbrains.kotlin.script.findScriptDefinition +import java.util.concurrent.ConcurrentLinkedQueue +import java.util.concurrent.locks.ReentrantReadWriteLock +import kotlin.concurrent.write import kotlin.script.experimental.dependencies.AsyncDependenciesResolver import kotlin.script.experimental.dependencies.DependenciesResolver -@Suppress("EXPERIMENTAL_FEATURE_WARNING") -class AsyncScriptDependenciesLoader( - file: VirtualFile, - scriptDef: KotlinScriptDefinition, - project: Project -) : ScriptDependenciesLoader(file, scriptDef, project) { +class AsyncScriptDependenciesLoader internal constructor(project: Project) : ScriptDependenciesLoader(project) { + private val backgroundTaskLock = ReentrantReadWriteLock() + private var backgroundTasksQueue: LoaderBackgroundTask? = null - override fun loadDependencies() { - if (!shouldSendNewRequest(lastRequest)) { - return - } - - lastRequest?.cancel() - lastRequest = sendRequest().stampBy(file) - - if (shouldUseBackgroundThread()) { - runBlocking { - lastRequest?.job?.actualJob?.join() + override fun loadDependencies(file: VirtualFile, scriptDef: KotlinScriptDefinition) { + backgroundTaskLock.write { + if (backgroundTasksQueue == null) { + backgroundTasksQueue = LoaderBackgroundTask() + backgroundTasksQueue!!.addTask(file) + backgroundTasksQueue!!.start() + } else { + backgroundTasksQueue!!.addTask(file) } } } - override fun shouldUseBackgroundThread() = KotlinScriptingSettings.getInstance(project).isAutoReloadEnabled override fun shouldShowNotification(): Boolean = !KotlinScriptingSettings.getInstance(project).isAutoReloadEnabled - private var lastRequest: ModStampedRequest? = null - - private val asyncUpdatesDispatcher = Executors.newFixedThreadPool(1).asCoroutineDispatcher() - private val legacyUpdatesDispatcher = - Executors.newFixedThreadPool( - (Runtime.getRuntime().availableProcessors() / 2).coerceAtLeast(1) - ).asCoroutineDispatcher() - - private fun shouldSendNewRequest(previousRequest: ModStampedRequest?): Boolean { - if (previousRequest == null) return true - return file.modificationStamp != previousRequest.modificationStamp + private fun runDependenciesUpdate(file: VirtualFile) { + val scriptDef = runReadAction { findScriptDefinition(file, project) } ?: return + // runBlocking is using there to avoid loading dependencies asynchronously + // because it leads to starting more than one gradle daemon in case of resolving dependencies in build.gradle.kts + // It is more efficient to use one hot daemon consistently than multiple daemon in parallel + val result = runBlocking { + try { + resolveDependencies(file, scriptDef) + } catch (t: Throwable) { + t.asResolveFailure(scriptDef) + } + } + processResult(result, file, scriptDef) } - private fun sendRequest(): TimeStampedJob { - val currentTimeStamp = TimeStamps.next() - + private suspend fun resolveDependencies(file: VirtualFile, scriptDef: KotlinScriptDefinition): DependenciesResolver.ResolveResult { val dependenciesResolver = scriptDef.dependencyResolver val scriptContents = contentLoader.getScriptContents(scriptDef, file) val environment = contentLoader.getEnvironment(scriptDef) - val newJob = if (dependenciesResolver is AsyncDependenciesResolver) { - launchAsyncUpdate(asyncUpdatesDispatcher, currentTimeStamp) { - dependenciesResolver.resolveAsync(scriptContents, environment) - } + return if (dependenciesResolver is AsyncDependenciesResolver) { + dependenciesResolver.resolveAsync(scriptContents, environment) } else { assert(dependenciesResolver is LegacyResolverWrapper) - launchAsyncUpdate(legacyUpdatesDispatcher, currentTimeStamp) { - dependenciesResolver.resolve(scriptContents, environment) + dependenciesResolver.resolve(scriptContents, environment) + } + } + + private inner class LoaderBackgroundTask { + private val sequenceOfFiles: ConcurrentLinkedQueue = ConcurrentLinkedQueue() + + fun start() { + if (shouldShowNotification()) { + BackgroundTaskUtil.executeOnPooledThread(project, Runnable { + loadDependencies(null) + }) + } else { + object : Task.Backgroundable(project, "Kotlin: Loading script dependencies...", true) { + override fun run(indicator: ProgressIndicator) { + loadDependencies(indicator) + } + + }.queue() } } - return TimeStampedJob(newJob, currentTimeStamp) - } - - private fun launchAsyncUpdate( - dispatcher: CoroutineDispatcher, - currentTimeStamp: TimeStamp, - doResolve: suspend () -> DependenciesResolver.ResolveResult - ): Job = GlobalScope.launch(dispatcher + project.cancelOnDisposal) { - val result = try { - doResolve() - } catch (t: Throwable) { - t.asResolveFailure(scriptDef) + fun addTask(file: VirtualFile) { + sequenceOfFiles.add(file) } - processResult(currentTimeStamp, result) - } - - private fun processResult( - currentTimeStamp: TimeStamp, - result: DependenciesResolver.ResolveResult - ) { - val lastTimeStamp = lastRequest?.job?.timeStamp - val isLastSentRequest = lastTimeStamp == null || lastTimeStamp == currentTimeStamp - if (isLastSentRequest) { - if (lastRequest != null) { - // no job running atm unless there is a job started while we process this result - lastRequest = ModStampedRequest(lastRequest!!.modificationStamp, job = null) + private fun loadDependencies(indicator: ProgressIndicator?) { + while (true) { + if (indicator?.isCanceled == true || sequenceOfFiles.isEmpty()) { + backgroundTaskLock.write { + backgroundTasksQueue = null + } + return + } + runDependenciesUpdate(sequenceOfFiles.poll()) } - - processResult(result) } } - - private class ModStampedRequest(val modificationStamp: Long, val job: TimeStampedJob?) { - fun cancel() = job?.actualJob?.cancel() - } - - private class TimeStampedJob(val actualJob: Job, val timeStamp: TimeStamp) { - fun stampBy(virtualFile: VirtualFile) = - ModStampedRequest( - virtualFile.modificationStamp, - this - ) - } - - private data class TimeStamp(private val stamp: Long) { - operator fun compareTo(other: TimeStamp) = this.stamp.compareTo(other.stamp) - } - - private object TimeStamps { - private var current: Long = 0 - - fun next() = - TimeStamp(current++) - } } diff --git a/idea/idea-core/src/org/jetbrains/kotlin/idea/core/script/dependencies/FromFileAttributeScriptDependenciesLoader.kt b/idea/idea-core/src/org/jetbrains/kotlin/idea/core/script/dependencies/FromFileAttributeScriptDependenciesLoader.kt index 84e2d7c91da..599a3478071 100644 --- a/idea/idea-core/src/org/jetbrains/kotlin/idea/core/script/dependencies/FromFileAttributeScriptDependenciesLoader.kt +++ b/idea/idea-core/src/org/jetbrains/kotlin/idea/core/script/dependencies/FromFileAttributeScriptDependenciesLoader.kt @@ -11,18 +11,14 @@ import org.jetbrains.kotlin.idea.core.script.scriptDependencies import org.jetbrains.kotlin.script.KotlinScriptDefinition import kotlin.script.experimental.dependencies.ScriptDependencies -class FromFileAttributeScriptDependenciesLoader( - file: VirtualFile, - scriptDef: KotlinScriptDefinition, - project: Project -) : ScriptDependenciesLoader(file, scriptDef, project) { +class FromFileAttributeScriptDependenciesLoader(project: Project) : ScriptDependenciesLoader(project) { - override fun loadDependencies() { + override fun loadDependencies(file: VirtualFile, scriptDef: KotlinScriptDefinition) { val deserializedDependencies = file.scriptDependencies ?: return - saveToCache(deserializedDependencies) + saveToCache(deserializedDependencies, file) } - private fun saveToCache(deserialized: ScriptDependencies) { + private fun saveToCache(deserialized: ScriptDependencies, file: VirtualFile) { val rootsChanged = cache.hasNotCachedRoots(deserialized) cache.save(file, deserialized) if (rootsChanged) { @@ -30,6 +26,5 @@ class FromFileAttributeScriptDependenciesLoader( } } - override fun shouldUseBackgroundThread() = false override fun shouldShowNotification(): Boolean = false } \ No newline at end of file diff --git a/idea/idea-core/src/org/jetbrains/kotlin/idea/core/script/dependencies/ScriptDependenciesLoader.kt b/idea/idea-core/src/org/jetbrains/kotlin/idea/core/script/dependencies/ScriptDependenciesLoader.kt index 829185ba7ad..b796a11fc4f 100644 --- a/idea/idea-core/src/org/jetbrains/kotlin/idea/core/script/dependencies/ScriptDependenciesLoader.kt +++ b/idea/idea-core/src/org/jetbrains/kotlin/idea/core/script/dependencies/ScriptDependenciesLoader.kt @@ -8,60 +8,33 @@ package org.jetbrains.kotlin.idea.core.script.dependencies import com.intellij.openapi.application.ApplicationManager import com.intellij.openapi.application.TransactionGuard import com.intellij.openapi.components.ServiceManager -import com.intellij.openapi.progress.ProgressIndicator -import com.intellij.openapi.progress.Task import com.intellij.openapi.project.Project import com.intellij.openapi.roots.ex.ProjectRootManagerEx import com.intellij.openapi.util.EmptyRunnable import com.intellij.openapi.vfs.VirtualFile +import com.intellij.util.containers.SLRUMap import com.intellij.util.ui.UIUtil import org.jetbrains.kotlin.idea.core.script.* import org.jetbrains.kotlin.idea.util.application.runWriteAction -import org.jetbrains.kotlin.script.* -import java.util.concurrent.ConcurrentHashMap -import kotlin.script.experimental.dependencies.AsyncDependenciesResolver +import org.jetbrains.kotlin.script.KotlinScriptDefinition +import org.jetbrains.kotlin.script.ScriptContentLoader +import org.jetbrains.kotlin.script.ScriptReportSink +import org.jetbrains.kotlin.script.adjustByDefinition import kotlin.script.experimental.dependencies.DependenciesResolver -abstract class ScriptDependenciesLoader( - protected val file: VirtualFile, - protected val scriptDef: KotlinScriptDefinition, - protected val project: Project -) { - companion object { - private val loaders = ConcurrentHashMap() +abstract class ScriptDependenciesLoader(protected val project: Project) { - fun updateDependencies( - file: VirtualFile, - scriptDef: KotlinScriptDefinition, - project: Project - ) { - val existingLoader = loaders[file] - if (existingLoader != null) return existingLoader.updateDependencies() + fun updateDependencies(file: VirtualFile, scriptDef: KotlinScriptDefinition) { + if (cache[file] == null || fileModificationStamps[file.path] != file.modificationStamp) { + fileModificationStamps.put(file.path, file.modificationStamp) - val newLoader = when (scriptDef.dependencyResolver) { - is AsyncDependenciesResolver, - is LegacyResolverWrapper -> AsyncScriptDependenciesLoader(file, scriptDef, project) - else -> SyncScriptDependenciesLoader(file, scriptDef, project) - } - loaders.put(file, newLoader) - newLoader.updateDependencies() + loadDependencies(file, scriptDef) } } - fun updateDependencies() { - if (shouldUseBackgroundThread()) { - object : Task.Backgroundable(project, "Kotlin: Loading dependencies for ${file.name} ...", true) { - override fun run(indicator: ProgressIndicator) { - loadDependencies() - } - }.queue() - } else { - loadDependencies() - } - } + private val fileModificationStamps: SLRUMap = SLRUMap(10, 10) - protected abstract fun loadDependencies() - protected abstract fun shouldUseBackgroundThread(): Boolean + protected abstract fun loadDependencies(file: VirtualFile, scriptDef: KotlinScriptDefinition) protected abstract fun shouldShowNotification(): Boolean protected val contentLoader = ScriptContentLoader(project) @@ -69,12 +42,10 @@ abstract class ScriptDependenciesLoader( private val reporter: ScriptReportSink = ServiceManager.getService(project, ScriptReportSink::class.java) - protected fun processResult(result: DependenciesResolver.ResolveResult) { - loaders.remove(file) - + protected fun processResult(result: DependenciesResolver.ResolveResult, file: VirtualFile, scriptDef: KotlinScriptDefinition) { if (cache[file] == null) { - saveDependencies(result) - attachReportsIfChanged(result) + saveDependencies(result, file, scriptDef) + attachReportsIfChanged(result, file, scriptDef) return } @@ -82,15 +53,15 @@ abstract class ScriptDependenciesLoader( if (cache[file] != newDependencies) { if (shouldShowNotification() && !ApplicationManager.getApplication().isUnitTestMode) { file.addScriptDependenciesNotificationPanel(result, project) { - saveDependencies(it) - attachReportsIfChanged(it) + saveDependencies(it, file, scriptDef) + attachReportsIfChanged(it, file, scriptDef) } } else { - saveDependencies(result) - attachReportsIfChanged(result) + saveDependencies(result, file, scriptDef) + attachReportsIfChanged(result, file, scriptDef) } } else { - attachReportsIfChanged(result) + attachReportsIfChanged(result, file, scriptDef) if (shouldShowNotification()) { file.removeScriptDependenciesNotificationPanel(project) @@ -98,13 +69,13 @@ abstract class ScriptDependenciesLoader( } } - private fun attachReportsIfChanged(result: DependenciesResolver.ResolveResult) { + private fun attachReportsIfChanged(result: DependenciesResolver.ResolveResult, file: VirtualFile, scriptDef: KotlinScriptDefinition) { if (file.getUserData(IdeScriptReportSink.Reports) != result.reports.takeIf { it.isNotEmpty() }) { reporter.attachReports(file, result.reports) } } - private fun saveDependencies(result: DependenciesResolver.ResolveResult) { + private fun saveDependencies(result: DependenciesResolver.ResolveResult, file: VirtualFile, scriptDef: KotlinScriptDefinition) { if (shouldShowNotification()) { file.removeScriptDependenciesNotificationPanel(project) } @@ -120,7 +91,6 @@ abstract class ScriptDependenciesLoader( } } - @Suppress("EXPERIMENTAL_FEATURE_WARNING") protected fun notifyRootsChanged() { val doNotifyRootsChanged = Runnable { runWriteAction { diff --git a/idea/idea-core/src/org/jetbrains/kotlin/idea/core/script/dependencies/SyncScriptDependenciesLoader.kt b/idea/idea-core/src/org/jetbrains/kotlin/idea/core/script/dependencies/SyncScriptDependenciesLoader.kt index 2eb5cc944d2..9c292bffddc 100644 --- a/idea/idea-core/src/org/jetbrains/kotlin/idea/core/script/dependencies/SyncScriptDependenciesLoader.kt +++ b/idea/idea-core/src/org/jetbrains/kotlin/idea/core/script/dependencies/SyncScriptDependenciesLoader.kt @@ -9,17 +9,11 @@ import com.intellij.openapi.project.Project import com.intellij.openapi.vfs.VirtualFile import org.jetbrains.kotlin.script.KotlinScriptDefinition -class SyncScriptDependenciesLoader( - file: VirtualFile, - scriptDef: KotlinScriptDefinition, - project: Project -) : ScriptDependenciesLoader(file, scriptDef, project) { - - override fun loadDependencies() { +class SyncScriptDependenciesLoader internal constructor(project: Project) : ScriptDependenciesLoader(project) { + override fun loadDependencies(file: VirtualFile, scriptDef: KotlinScriptDefinition) { val result = contentLoader.loadContentsAndResolveDependencies(scriptDef, file) - processResult(result) + processResult(result, file, scriptDef) } - override fun shouldUseBackgroundThread(): Boolean = false override fun shouldShowNotification(): Boolean = false } \ No newline at end of file