JPS, -Xcommon-sources: don't iterate on all sources, use root info from DirtySourceFilesHolder. Cleanup code.

Also: - Pass `-Xcommon-sources` argument to JS compiler. - Calculate all sources list only when needed (after change it is not required to calculate common-sources) - Remove `chunk: ModuleChunk` parameter from some `KotlinModuleBuildTarget` functions.
2018-09-10 11:16:06 +03:00
parent 359909dcff
commit 8cd3b46f05
8 changed files with 114 additions and 104 deletions
@@ -125,7 +125,8 @@ class JpsKotlinCompilerRunner : KotlinCompilerRunner<JpsCompilerEnvironment>() {
        k2jsArguments: K2JSCompilerArguments,
        compilerSettings: CompilerSettings,
        environment: JpsCompilerEnvironment,
-        sourceFiles: Collection<File>,
+        allSourceFiles: Collection<File>,
+        commonSources: Collection<File>,
        sourceMapRoots: Collection<File>,
        libraries: List<String>,
        friendModules: List<String>,
@@ -137,7 +138,7 @@ class JpsKotlinCompilerRunner : KotlinCompilerRunner<JpsCompilerEnvironment>() {
        val arguments = mergeBeans(commonArguments, XmlSerializerUtil.createCopy(k2jsArguments))
        log.debug("K2JS: merged arguments: " + ArgumentUtils.convertArgumentsToStringList(arguments))

-        setupK2JsArguments(outputFile, sourceFiles, libraries, friendModules, arguments)
+        setupK2JsArguments(outputFile, allSourceFiles, commonSources, libraries, friendModules, arguments)
        if (arguments.sourceMap) {
            arguments.sourceMapBaseDirs = sourceMapRoots.joinToString(File.pathSeparator) { it.path }
        }
@@ -290,14 +291,16 @@ class JpsKotlinCompilerRunner : KotlinCompilerRunner<JpsCompilerEnvironment>() {

    private fun setupK2JsArguments(
        _outputFile: File,
-        sourceFiles: Collection<File>,
+        allSourceFiles: Collection<File>,
+        _commonSources: Collection<File>,
        _libraries: List<String>,
        _friendModules: List<String>,
        settings: K2JSCompilerArguments
    ) {
        with(settings) {
            noStdlib = true
-            freeArgs = sourceFiles.map { it.path }.toMutableList()
+            freeArgs = allSourceFiles.map { it.path }.toMutableList()
+            commonSources = _commonSources.map { it.path }.toTypedArray()
            outputFile = _outputFile.path
            metaInfo = true
            libraries = _libraries.joinToString(File.pathSeparator)
@@ -64,7 +64,7 @@ class FSOperationsHelper(
    internal fun markFilesForCurrentRound(files: Iterable<File>) {
        files.forEach {
            val root = compileContext.projectDescriptor.buildRootIndex.findJavaRootDescriptor(compileContext, it)
-            if (root != null) dirtyFilesHolder.byTarget[root.target]?._markDirty(it)
+            if (root != null) dirtyFilesHolder.byTarget[root.target]?._markDirty(it, root)
        }

        markFilesImpl(files, currentRound = true) { it.exists() && moduleBasedFilter.accept(it) }
@@ -78,7 +78,8 @@ class FSOperationsHelper(

        val targetDirtyFiles = dirtyFilesHolder.byTarget[target]!!
        files.forEach {
-            targetDirtyFiles._markDirty(it)
+            val root = compileContext.projectDescriptor.buildRootIndex.findJavaRootDescriptor(compileContext, it)!!
+            targetDirtyFiles._markDirty(it, root)
        }

        markFilesImpl(files, currentRound = true) { it.exists() }
@@ -234,7 +234,7 @@ class KotlinBuilder : ModuleLevelBuilder(BuilderCategory.SOURCE_PROCESSOR) {
        val removedClasses = HashSet<String>()
        for (target in kotlinChunk.targets) {
            val cache = incrementalCaches[target] ?: continue
-            val dirtyFiles = dirtyFilesHolder.getDirtyFiles(target.jpsModuleBuildTarget)
+            val dirtyFiles = dirtyFilesHolder.getDirtyFiles(target.jpsModuleBuildTarget).keys
            val removedFiles = dirtyFilesHolder.getRemovedFiles(target.jpsModuleBuildTarget)

            val existingClasses = JpsKotlinCompilerRunner().classesFqNamesByFiles(environment, dirtyFiles)
@@ -497,17 +497,17 @@ class KotlinBuilder : ModuleLevelBuilder(BuilderCategory.SOURCE_PROCESSOR) {

                if (cache != null && targetDirtyFiles != null) {
                    val complementaryFiles = cache.clearComplementaryFilesMapping(
-                        targetDirtyFiles.dirty + targetDirtyFiles.removed
+                        targetDirtyFiles.dirty.keys + targetDirtyFiles.removed
                    )

                    fsOperations.markFilesForCurrentRound(jpsTarget, complementaryFiles)

-                    cache.markDirty(targetDirtyFiles.dirty + targetDirtyFiles.removed)
+                    cache.markDirty(targetDirtyFiles.dirty.keys + targetDirtyFiles.removed)
                }
            }
        }

-        val isDoneSomething = representativeTarget.compileModuleChunk(chunk, commonArguments, dirtyFilesHolder, environment)
+        val isDoneSomething = representativeTarget.compileModuleChunk(commonArguments, dirtyFilesHolder, environment)

        return if (isDoneSomething) environment.outputItemsCollector else null
    }
@@ -22,6 +22,7 @@ import org.jetbrains.jps.builders.DirtyFilesHolder
 import org.jetbrains.jps.builders.java.JavaSourceRootDescriptor
 import org.jetbrains.jps.incremental.CompileContext
 import org.jetbrains.jps.incremental.ModuleBuildTarget
+import org.jetbrains.kotlin.jps.targets.KotlinModuleBuildTarget
 import java.io.File

 /**
@@ -38,17 +39,17 @@ class KotlinDirtySourceFilesHolder(
    val byTarget: Map<ModuleBuildTarget, TargetFiles>

    inner class TargetFiles(val target: ModuleBuildTarget, val removed: Collection<File>) {
-        private val _dirty: MutableSet<File> = mutableSetOf()
+        private val _dirty: MutableMap<File, KotlinModuleBuildTarget.Source> = mutableMapOf()

-        val dirty: Set<File>
+        val dirty: Map<File, KotlinModuleBuildTarget.Source>
            get() = _dirty

        /**
         * Should be called only from [FSOperationsHelper.markFilesForCurrentRound]
         * and during KotlinDirtySourceFilesHolder initialization.
         */
-        internal fun _markDirty(file: File) {
-            _dirty.add(file.canonicalFile)
+        internal fun _markDirty(file: File, root: JavaSourceRootDescriptor) {
+            _dirty[file.canonicalFile] = KotlinModuleBuildTarget.Source(file, root is KotlinIncludedModuleSourceRoot)
        }
    }

@@ -74,7 +75,7 @@ class KotlinDirtySourceFilesHolder(
                    ?: error("processDirtyFiles should callback only on chunk `$chunk` targets (`$target` is not)")

            if (file.isKotlinSourceFile) {
-                targetInfo._markDirty(file)
+                targetInfo._markDirty(file, root)
            }

            true
@@ -83,14 +84,14 @@ class KotlinDirtySourceFilesHolder(
        this.byTarget = byTarget
    }

-    fun getDirtyFiles(target: ModuleBuildTarget): Set<File> =
-        byTarget[target]?.dirty ?: setOf()
+    fun getDirtyFiles(target: ModuleBuildTarget): Map<File, KotlinModuleBuildTarget.Source> =
+        byTarget[target]?.dirty ?: mapOf()

    fun getRemovedFiles(target: ModuleBuildTarget): Collection<File> =
        byTarget[target]?.removed ?: listOf()

    val allDirtyFiles: Set<File>
-        get() = byTarget.flatMapTo(mutableSetOf()) { it.value.dirty }
+        get() = byTarget.flatMapTo(mutableSetOf()) { it.value.dirty.keys }

    val allRemovedFilesFiles: Set<File>
        get() = byTarget.flatMapTo(mutableSetOf()) { it.value.removed }
@@ -5,7 +5,6 @@

 package org.jetbrains.kotlin.jps.targets

-import org.jetbrains.jps.ModuleChunk
 import org.jetbrains.jps.builders.storage.BuildDataPaths
 import org.jetbrains.jps.incremental.ModuleBuildTarget
 import org.jetbrains.jps.model.library.JpsOrderRootType
@@ -45,12 +44,13 @@ class KotlinCommonModuleBuildTarget(kotlinContext: KotlinCompileContext, jpsModu
        get() = "metadata-compiler"

    override fun compileModuleChunk(
-        chunk: ModuleChunk,
        commonArguments: CommonCompilerArguments,
        dirtyFilesHolder: KotlinDirtySourceFilesHolder,
        environment: JpsCompilerEnvironment
    ): Boolean {
-        reportAndSkipCircular(chunk, environment)
+        require(chunk.representativeTarget == this)
+
+        reportAndSkipCircular(environment)

        JpsKotlinCompilerRunner().runK2MetadataCompiler(
            commonArguments,
@@ -5,7 +5,6 @@

 package org.jetbrains.kotlin.jps.targets

-import org.jetbrains.jps.ModuleChunk
 import org.jetbrains.jps.builders.storage.BuildDataPaths
 import org.jetbrains.jps.incremental.ModuleBuildTarget
 import org.jetbrains.jps.model.library.JpsOrderRootType
@@ -86,16 +85,19 @@ class KotlinJsModuleBuildTarget(kotlinContext: KotlinCompileContext, jpsModuleBu
    }

    override fun compileModuleChunk(
-        chunk: ModuleChunk,
        commonArguments: CommonCompilerArguments,
        dirtyFilesHolder: KotlinDirtySourceFilesHolder,
        environment: JpsCompilerEnvironment
    ): Boolean {
-        require(chunk.representativeTarget() == jpsModuleBuildTarget)
-        if (reportAndSkipCircular(chunk, environment)) return false
+        require(chunk.representativeTarget == this)
+
+        if (reportAndSkipCircular(environment)) return false

        val sources = collectSourcesToCompile(dirtyFilesHolder)
-        if (!checkShouldCompileAndLog(dirtyFilesHolder, sources)) return false
+
+        if (!sources.logFiles()) {
+            return false
+        }

        val libraries = libraryFiles + dependenciesMetaFiles

@@ -104,7 +106,8 @@ class KotlinJsModuleBuildTarget(kotlinContext: KotlinCompileContext, jpsModuleBu
            module.k2JsCompilerArguments,
            module.kotlinCompilerSettings,
            environment,
-            sources,
+            sources.allFiles,
+            sources.crossCompiledFiles,
            sourceMapRoots,
            libraries,
            friendBuildTargetsMetaFiles,
@@ -74,6 +74,7 @@ class KotlinJvmModuleBuildTarget(kotlinContext: KotlinCompileContext, jpsModuleB
        with(builder) {
            register(
                IncrementalCompilationComponents::class.java,
+                @Suppress("UNCHECKED_CAST")
                IncrementalCompilationComponentsImpl(
                    incrementalCaches.mapKeys { it.key.targetId } as Map<TargetId, IncrementalCache>
                )
@@ -82,27 +83,28 @@ class KotlinJvmModuleBuildTarget(kotlinContext: KotlinCompileContext, jpsModuleB
    }

    override fun compileModuleChunk(
-        chunk: ModuleChunk,
        commonArguments: CommonCompilerArguments,
        dirtyFilesHolder: KotlinDirtySourceFilesHolder,
        environment: JpsCompilerEnvironment
    ): Boolean {
-        if (chunk.modules.size > 1) {
+        require(chunk.representativeTarget == this)
+
+        if (chunk.targets.size > 1) {
            environment.messageCollector.report(
                CompilerMessageSeverity.STRONG_WARNING,
-                "Circular dependencies are only partially supported. The following modules depend on each other: "
-                        + chunk.modules.joinToString(", ") { it.name } + " "
-                        + "Kotlin will compile them, but some strange effect may happen"
+                "Circular dependencies are only partially supported. " +
+                        "The following modules depend on each other: ${chunk.presentableShortName}. " +
+                        "Kotlin will compile them, but some strange effect may happen"
            )
        }

        val filesSet = dirtyFilesHolder.allDirtyFiles

-        val moduleFile = generateModuleDescription(chunk, dirtyFilesHolder)
+        val moduleFile = generateChunkModuleDescription(dirtyFilesHolder)
        if (moduleFile == null) {
            if (KotlinBuilder.LOG.isDebugEnabled) {
                KotlinBuilder.LOG.debug(
-                    "Not compiling, because no files affected: " + chunk.targets.joinToString { it.presentableName }
+                    "Not compiling, because no files affected: " + chunk.presentableShortName
                )
            }

@@ -110,13 +112,15 @@ class KotlinJvmModuleBuildTarget(kotlinContext: KotlinCompileContext, jpsModuleB
            return false
        }

-        val module = chunk.representativeTarget().module
+        val module = chunk.representativeTarget.module

        if (KotlinBuilder.LOG.isDebugEnabled) {
            val totalRemovedFiles = dirtyFilesHolder.allRemovedFilesFiles.size
-            KotlinBuilder.LOG.debug("Compiling to JVM ${filesSet.size} files"
-                                            + (if (totalRemovedFiles == 0) "" else " ($totalRemovedFiles removed files)")
-                                            + " in " + chunk.targets.joinToString { it.presentableName })
+            KotlinBuilder.LOG.debug(
+                "Compiling to JVM ${filesSet.size} files"
+                        + (if (totalRemovedFiles == 0) "" else " ($totalRemovedFiles removed files)")
+                        + " in " + chunk.presentableShortName
+            )
        }

        try {
@@ -137,35 +141,35 @@ class KotlinJvmModuleBuildTarget(kotlinContext: KotlinCompileContext, jpsModuleB
        return true
    }

-    private fun generateModuleDescription(chunk: ModuleChunk, dirtyFilesHolder: KotlinDirtySourceFilesHolder): File? {
+    private fun generateChunkModuleDescription(dirtyFilesHolder: KotlinDirtySourceFilesHolder): File? {
        val builder = KotlinModuleXmlBuilder()

        var hasDirtySources = false

-        val targets = chunk.targets.mapNotNull { kotlinContext.targetsBinding[it] as? KotlinJvmModuleBuildTarget }
+        val targets = chunk.targets

        val outputDirs = targets.map { it.outputDir }.toSet()

        for (target in targets) {
+            target as KotlinJvmModuleBuildTarget
+
            val outputDir = target.outputDir
            val friendDirs = target.friendOutputDirs

-            val moduleSources = collectSourcesToCompile(target, dirtyFilesHolder).toSet()
-            val commonSources = sources
-                .filter { it.value.isIncludedSourceRoot && it.key in moduleSources }
-                .map { it.key }
+            val sources = target.collectSourcesToCompile(dirtyFilesHolder)

-            val hasDirtyOrRemovedSources = checkShouldCompileAndLog(target, dirtyFilesHolder, moduleSources)
-            if (hasDirtyOrRemovedSources) hasDirtySources = true
+            if (sources.logFiles()) {
+                hasDirtySources = true
+            }

            val kotlinModuleId = target.targetId
            builder.addModule(
                kotlinModuleId.name,
                outputDir.absolutePath,
-                moduleSources,
+                sources.allFiles,
                target.findSourceRoots(dirtyFilesHolder.context),
                target.findClassPathRoots(),
-                commonSources,
+                sources.crossCompiledFiles,
                target.findModularJdkRoot(),
                kotlinModuleId.type,
                isTests,
@@ -177,15 +181,15 @@ class KotlinJvmModuleBuildTarget(kotlinContext: KotlinCompileContext, jpsModuleB

        if (!hasDirtySources) return null

-        val scriptFile = createTempFileForModuleDesc(chunk)
+        val scriptFile = createTempFileForChunkModuleDesc()
        FileUtil.writeToFile(scriptFile, builder.asText().toString())
        return scriptFile
    }

-    private fun createTempFileForModuleDesc(chunk: ModuleChunk): File {
+    private fun createTempFileForChunkModuleDesc(): File {
        val readableSuffix = buildString {
-            append(StringUtil.sanitizeJavaIdentifier(chunk.representativeTarget().module.name))
-            if (chunk.containsTests()) {
+            append(StringUtil.sanitizeJavaIdentifier(chunk.representativeTarget.module.name))
+            if (chunk.containsTests) {
                append("-test")
            }
        }
@@ -276,7 +280,7 @@ class KotlinJvmModuleBuildTarget(kotlinContext: KotlinCompileContext, jpsModuleB
        val targetDirtyFiles: Map<ModuleBuildTarget, Set<File>> = chunk.targets.keysToMap {
            val files = HashSet<File>()
            files.addAll(dirtyFilesHolder.getRemovedFiles(it))
-            files.addAll(dirtyFilesHolder.getDirtyFiles(it))
+            files.addAll(dirtyFilesHolder.getDirtyFiles(it).keys)
            files
        }

@@ -137,21 +137,25 @@ abstract class KotlinModuleBuildTarget<BuildMetaInfoType : BuildMetaInfo> intern

    /**
     * All sources of this target (including non dirty).
-     * Initialized lazily based on global context and will be updated on each round based on round local context.
     *
+     * Lazy initialization is required since value is required only in rare cases.
+     *
+     * Before first round initialized lazily based on global context.
+     * This is required for friend build targets, when friends are not compiled in this build run.
+     *
+     * Lazy value will be invalidated on each round (should be recalculated based on round local context).
     * Update required since source roots can be changed, for example groovy can provide new temporary source roots with stubs.
-     * Lazy initialization is required for friend build targets, when friends are not compiled in this build run.
+     *
+     * Ugly delegation to lazy is used to capture local compile context and reset calculated value.
     */
    val sources: Map<File, Source>
-        get() = _sources ?: synchronized(this) {
-            _sources ?: updateSourcesList(jpsGlobalContext)
-        }
+        get() = _sources.value

    @Volatile
-    private var _sources: Map<File, Source>? = null
+    private var _sources: Lazy<Map<File, Source>> = lazy { updateSourcesList(jpsGlobalContext) }

    fun nextRound(localContext: CompileContext) {
-        updateSourcesList(localContext)
+        _sources = lazy { updateSourcesList(localContext) }
    }

    private fun updateSourcesList(localContext: CompileContext): Map<File, Source> {
@@ -165,34 +169,33 @@ abstract class KotlinModuleBuildTarget<BuildMetaInfoType : BuildMetaInfo> intern
        val buildRootIndex = localContext.projectDescriptor.buildRootIndex
        val roots = buildRootIndex.getTargetRoots(jpsModuleBuildTarget, localContext)
        roots.forEach { rootDescriptor ->
-            val isIncludedSourceRoot = rootDescriptor is KotlinIncludedModuleSourceRoot
+            val isCrossCompiled = rootDescriptor is KotlinIncludedModuleSourceRoot

            rootDescriptor.root.walkTopDown()
                .onEnter { file -> file !in moduleExcludes }
                .forEach { file ->
                    if (!compilerExcludes.isExcluded(file) && file.isFile && file.isKotlinSourceFile) {
-                        result[file] = Source(file, isIncludedSourceRoot)
+                        result[file] = Source(file, isCrossCompiled)
                    }
                }

        }

-        this._sources = result
        return result
    }

    /**
-     * @property isIncludedSourceRoot sources that are cross-compiled to multiple targets
+     * @property isCrossCompiled sources that are cross-compiled to multiple targets
     */
    class Source(
        val file: File,
-        val isIncludedSourceRoot: Boolean
+        val isCrossCompiled: Boolean
    )

-    fun isFromIncludedSourceRoot(file: File): Boolean = sources[file]?.isIncludedSourceRoot == true
+    fun isFromIncludedSourceRoot(file: File): Boolean = sources[file]?.isCrossCompiled == true

    val sourceFiles: Collection<File>
-        get() = sources.values.map { it.file }
+        get() = sources.keys

    override fun toString() = jpsModuleBuildTarget.toString()

@@ -200,23 +203,19 @@ abstract class KotlinModuleBuildTarget<BuildMetaInfoType : BuildMetaInfo> intern
     * Called for `ModuleChunk.representativeTarget`
     */
    abstract fun compileModuleChunk(
-        chunk: ModuleChunk,
        commonArguments: CommonCompilerArguments,
        dirtyFilesHolder: KotlinDirtySourceFilesHolder,
        environment: JpsCompilerEnvironment
    ): Boolean

-    protected fun reportAndSkipCircular(
-        chunk: ModuleChunk,
-        environment: JpsCompilerEnvironment
-    ): Boolean {
-        if (chunk.modules.size > 1) {
+    protected fun reportAndSkipCircular(environment: JpsCompilerEnvironment): Boolean {
+        if (chunk.targets.size > 1) {
            // We do not support circular dependencies, but if they are present, we do our best should not break the build,
            // so we simply yield a warning and report NOTHING_DONE
            environment.messageCollector.report(
                CompilerMessageSeverity.STRONG_WARNING,
                "Circular dependencies are not supported. The following modules depend on each other: "
-                        + chunk.modules.joinToString(", ") { it.name } + " "
+                        + chunk.presentableShortName + " "
                        + "Kotlin is not compiled for these modules"
            )

@@ -273,46 +272,45 @@ abstract class KotlinModuleBuildTarget<BuildMetaInfoType : BuildMetaInfo> intern
        }
    }

-    protected fun collectSourcesToCompile(dirtyFilesHolder: KotlinDirtySourceFilesHolder) =
-        collectSourcesToCompile(this, dirtyFilesHolder)
-
    /**
     * Should be used only for particular target in chunk (jvm)
+     *
+     * Should not be cached since may be vary in different rounds.
     */
    protected fun collectSourcesToCompile(
-        target: KotlinModuleBuildTarget<BuildMetaInfoType>,
        dirtyFilesHolder: KotlinDirtySourceFilesHolder
-    ): Collection<File> {
-        // Should not be cached since may be vary in different rounds
+    ) = SourcesToCompile(
+        sources = when {
+            chunk.representativeTarget.isIncrementalCompilationEnabled ->
+                dirtyFilesHolder.getDirtyFiles(jpsModuleBuildTarget).values
+            else -> sources.values
+        },
+        removedFiles = dirtyFilesHolder.getRemovedFiles(jpsModuleBuildTarget)
+    )

-        val jpsModuleTarget = target.jpsModuleBuildTarget
-        return when {
-            isIncrementalCompilationEnabled -> dirtyFilesHolder.getDirtyFiles(jpsModuleTarget)
-            else -> target.sourceFiles
-        }
-    }
+    inner class SourcesToCompile(
+        sources: Collection<KotlinModuleBuildTarget.Source>,
+        val removedFiles: Collection<File>
+    ) {
+        val allFiles = sources.map { it.file }
+        val crossCompiledFiles = sources.filter { it.isCrossCompiled }.map { it.file }

-    protected fun checkShouldCompileAndLog(dirtyFilesHolder: KotlinDirtySourceFilesHolder, moduleSources: Collection<File>) =
-        checkShouldCompileAndLog(this, dirtyFilesHolder, moduleSources)
+        /**
+         * @return true, if there are removed files or files to compile
+         */
+        fun logFiles(): Boolean {
+            val hasRemovedSources = removedFiles.isNotEmpty()
+            val hasDirtyOrRemovedSources = allFiles.isNotEmpty() || hasRemovedSources

-    /**
-     * Should be used only for particular target in chunk (jvm)
-     */
-    protected fun checkShouldCompileAndLog(
-        target: KotlinModuleBuildTarget<BuildMetaInfoType>,
-        dirtyFilesHolder: KotlinDirtySourceFilesHolder,
-        moduleSources: Collection<File>
-    ): Boolean {
-        val hasRemovedSources = dirtyFilesHolder.getRemovedFiles(target.jpsModuleBuildTarget).isNotEmpty()
-        val hasDirtyOrRemovedSources = moduleSources.isNotEmpty() || hasRemovedSources
-        if (hasDirtyOrRemovedSources) {
-            val logger = jpsGlobalContext.loggingManager.projectBuilderLogger
-            if (logger.isEnabled) {
-                logger.logCompiledFiles(moduleSources, KotlinBuilder.KOTLIN_BUILDER_NAME, "Compiling files:")
+            if (hasDirtyOrRemovedSources) {
+                val logger = jpsGlobalContext.loggingManager.projectBuilderLogger
+                if (logger.isEnabled) {
+                    logger.logCompiledFiles(allFiles, KotlinBuilder.KOTLIN_BUILDER_NAME, "Compiling files:")
+                }
            }
-        }

-        return hasDirtyOrRemovedSources
+            return hasDirtyOrRemovedSources
+        }
    }

    abstract val buildMetaInfoFactory: BuildMetaInfoFactory<BuildMetaInfoType>