Make lookup storage addAll order independent

Lookup storage output files could differ for projects
with different absolute paths.
This happened because, paths for lookups were
relativized only before writing to the underlying storage.
Storing absolute paths in a hash table could
result in different order of adding files to the lookup storage.
This commit fixes the issue by sorting lookups and files in
LookupStorage#addAll

    #KT-32674 Fixed

build-common/src/org/jetbrains/kotlin/incremental/LookupStorage.kt

@@ -75,12 +75,13 @@ open class LookupStorage(
     }
 
     @Synchronized
-    fun addAll(lookups: Set<Map.Entry<LookupSymbol, Collection<String>>>, allPaths: Set<String>) {
-        val pathToId = allPaths.keysToMap { addFileIfNeeded(File(it)) }
+    fun addAll(lookups: MultiMap<LookupSymbol, String>, allPaths: Set<String>) {
+        val pathToId = allPaths.sorted().keysToMap { addFileIfNeeded(File(it)) }
 
-        for ((lookupSymbol, paths) in lookups) {
+        for (lookupSymbol in lookups.keySet().sorted()) {
             val key = LookupSymbolKey(lookupSymbol.name, lookupSymbol.scope)
-            val fileIds = paths.mapTo(HashSet<Int>()) { pathToId[it]!! }
+            val paths = lookups[lookupSymbol]!!
+            val fileIds = paths.mapTo(TreeSet()) { pathToId[it]!! }
             fileIds.addAll(lookupMap[key] ?: emptySet())
             lookupMap[key] = fileIds
         }
@@ -227,4 +228,11 @@ class LookupTrackerImpl(private val delegate: LookupTracker) : LookupTracker {
     }
 }
 
-data class LookupSymbol(val name: String, val scope: String)
+data class LookupSymbol(val name: String, val scope: String) : Comparable<LookupSymbol> {
+    override fun compareTo(other: LookupSymbol): Int {
+        val scopeCompare = scope.compareTo(other.scope)
+        if (scopeCompare != 0) return scopeCompare
+
+        return name.compareTo(other.name)
+    }
+}

build-common/src/org/jetbrains/kotlin/incremental/buildUtil.kt

@@ -30,6 +30,7 @@ import org.jetbrains.kotlin.name.FqName
 import org.jetbrains.kotlin.progress.CompilationCanceledStatus
 import org.jetbrains.kotlin.synthetic.SAM_LOOKUP_NAME
 import org.jetbrains.kotlin.utils.addToStdlib.flattenTo
+import org.jetbrains.kotlin.utils.keysToMap
 import java.io.File
 import java.util.*
 import kotlin.collections.HashSet
@@ -126,7 +127,7 @@ fun LookupStorage.update(
 
     removeLookupsFrom(filesToCompile.asSequence() + removedFiles.asSequence())
 
-    addAll(lookupTracker.lookups.entrySet(), lookupTracker.pathInterner.values)
+    addAll(lookupTracker.lookups, lookupTracker.pathInterner.values)
 }
 
 data class DirtyData(

build-common/src/org/jetbrains/kotlin/incremental/storage/RelativeFileToPathConverter.kt

@@ -0,0 +1,34 @@
+/*
+ * Copyright 2010-2019 JetBrains s.r.o. and Kotlin Programming Language contributors.
+ * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file.
+ */
+
+package org.jetbrains.kotlin.incremental.storage
+
+import java.io.File
+
+open class RelativeFileToPathConverter(baseDirFile: File?) : FileToPathConverter {
+    private val baseDirPath = baseDirFile?.canonicalFile?.invariantSeparatorsPath
+
+    override fun toPath(file: File): String {
+        val path = file.canonicalFile.invariantSeparatorsPath
+        return when {
+            baseDirPath != null && path.startsWith(baseDirPath) ->
+                PROJECT_DIR_PLACEHOLDER + path.substring(baseDirPath.length)
+            else -> path
+        }
+    }
+
+    override fun toFile(path: String): File =
+        when {
+            path.startsWith(PROJECT_DIR_PLACEHOLDER) -> {
+                val basePath = baseDirPath ?: error("Could not get project root dir")
+                File(basePath + path.substring(PROJECT_DIR_PLACEHOLDER.length))
+            }
+            else -> File(path)
+        }
+
+    private companion object {
+        private const val PROJECT_DIR_PLACEHOLDER = "${'$'}PROJECT_DIR$"
+    }
+}

build-common/test/org/jetbrains/kotlin/incremental/storage/RelocatableCachesTest.kt

@@ -0,0 +1,90 @@
+/*
+ * Copyright 2010-2019 JetBrains s.r.o. and Kotlin Programming Language contributors.
+ * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file.
+ */
+
+package org.jetbrains.kotlin.incremental.storage
+
+import com.intellij.util.containers.MultiMap
+import org.jetbrains.kotlin.TestWithWorkingDir
+import org.jetbrains.kotlin.incremental.LookupStorage
+import org.jetbrains.kotlin.incremental.LookupSymbol
+import org.jetbrains.kotlin.incremental.testingUtils.assertEqualDirectories
+import org.junit.Test
+import java.io.File
+
+class RelocatableCachesTest : TestWithWorkingDir() {
+    @Test
+    fun testLookupStorageAddAllReversedFiles() {
+        val originalRoot = workingDir.resolve("original")
+        fillLookupStorage(originalRoot, reverseFiles = false, reverseLookups = false)
+        val reversedFilesOrderRoot = workingDir.resolve("reversedFiles")
+        fillLookupStorage(reversedFilesOrderRoot, reverseFiles = true, reverseLookups = false)
+        assertEqualDirectories(originalRoot, reversedFilesOrderRoot, forgiveExtraFiles = false)
+    }
+
+    @Test
+    fun testLookupStorageAddAllReversedLookups() {
+        val originalRoot = workingDir.resolve("original")
+        fillLookupStorage(originalRoot, reverseFiles = false, reverseLookups = false)
+        val reversedLookupsOrderRoot = workingDir.resolve("reversedLookups")
+        fillLookupStorage(reversedLookupsOrderRoot, reverseFiles = false, reverseLookups = true)
+        assertEqualDirectories(originalRoot, reversedLookupsOrderRoot, forgiveExtraFiles = false)
+    }
+
+    @Test
+    fun testLookupStorageAddAllReversedFilesReversedLookups() {
+        val originalRoot = workingDir.resolve("original")
+        fillLookupStorage(originalRoot, reverseFiles = false, reverseLookups = false)
+        val reversedFilesReversedLookupsOrderRoot = workingDir.resolve("reversedFilesReversedLookupsOrderRoot")
+        fillLookupStorage(reversedFilesReversedLookupsOrderRoot, reverseFiles = true, reverseLookups = true)
+        assertEqualDirectories(originalRoot, reversedFilesReversedLookupsOrderRoot, forgiveExtraFiles = false)
+    }
+
+    /**
+     * Fills lookup storage in [projectRoot] with N fq-names,
+     * where i_th fq-name myscope_i.MyClass_i has lookups for previous fq-names (from 0 to i-1)
+     */
+    private fun fillLookupStorage(projectRoot: File, reverseFiles: Boolean, reverseLookups: Boolean) {
+        val storageRoot = projectRoot.storageRoot
+        val fileToPathConverter = RelativeFileToPathConverter(projectRoot)
+        val lookupStorage = LookupStorage(storageRoot, fileToPathConverter)
+        val files = LinkedHashSet<String>()
+        val symbols = LinkedHashSet<LookupSymbol>()
+        val lookups = MultiMap.createOrderedSet<LookupSymbol, String>()
+
+        for (i in 0..10) {
+            val newSymbol = LookupSymbol(name = "MyClass_$i", scope = "myscope_$i")
+            val newSourcePath = projectRoot.resolve("src/${newSymbol.asRelativePath()}").canonicalFile.invariantSeparatorsPath
+            symbols.add(newSymbol)
+
+            for (lookedUpSymbol in symbols) {
+                lookups.putValue(lookedUpSymbol, newSourcePath)
+            }
+
+            files.add(newSourcePath)
+        }
+
+        val filesToAdd = if (reverseFiles) files.reversedSet() else files
+        val lookupsToAdd = if (reverseLookups) lookups.reversedMultiMap() else lookups
+        lookupStorage.addAll(lookupsToAdd, filesToAdd)
+        lookupStorage.flush(memoryCachesOnly = false)
+    }
+
+    private val File.storageRoot: File
+        get() = resolve("storage")
+
+    private fun <K, V> MultiMap<K, V>.reversedMultiMap(): MultiMap<K, V> {
+        val newMap = MultiMap.createOrderedSet<K, V>()
+        for ((key, values) in entrySet().reversedSet()) {
+            newMap.putValues(key, values.reversed())
+        }
+        return newMap
+    }
+
+    private fun <T> Set<T>.reversedSet(): LinkedHashSet<T> =
+        reversed().toCollection(LinkedHashSet(size))
+
+    private fun LookupSymbol.asRelativePath(): String =
+        if (scope.isBlank()) name else scope.replace('.', '/') + '/' + name
+}

compiler/daemon/src/org/jetbrains/kotlin/daemon/experimental/RemoteLookupTrackerClient.kt

@@ -8,11 +8,8 @@ package org.jetbrains.kotlin.daemon.experimental
 import com.intellij.util.containers.StringInterner
 import kotlinx.coroutines.runBlocking
 import org.jetbrains.kotlin.daemon.EventManager
+import org.jetbrains.kotlin.daemon.common.*
 import org.jetbrains.kotlin.daemon.common.experimental.CompilerCallbackServicesFacadeClientSide
-import org.jetbrains.kotlin.daemon.common.DummyProfiler
-import org.jetbrains.kotlin.daemon.common.Profiler
-import org.jetbrains.kotlin.daemon.common.withMeasure
-import org.jetbrains.kotlin.daemon.common.withMeasureBlocking
 import org.jetbrains.kotlin.incremental.components.LookupInfo
 import org.jetbrains.kotlin.incremental.components.LookupTracker
 import org.jetbrains.kotlin.incremental.components.Position
@@ -20,7 +17,7 @@ import org.jetbrains.kotlin.incremental.components.ScopeKind
 
 
 class RemoteLookupTrackerClient(
-    val facade: CompilerCallbackServicesFacadeClientSide,
+    val facade: CompilerCallbackServicesFacadeAsync,
     eventManager: EventManager,
     val profiler: Profiler = DummyProfiler()
 ) : LookupTracker {

jps-plugin/src/org/jetbrains/kotlin/jps/build/JpsFileToPathConverter.kt

@@ -7,30 +7,8 @@ package org.jetbrains.kotlin.jps.build
 
 import org.jetbrains.jps.model.JpsProject
 import org.jetbrains.jps.model.serialization.JpsModelSerializationDataService
-import org.jetbrains.kotlin.incremental.storage.FileToPathConverter
-import java.io.File
+import org.jetbrains.kotlin.incremental.storage.RelativeFileToPathConverter
 
-private const val PROJECT_DIR_PLACEHOLDER = "${'$'}PROJECT_DIR$"
-
-internal class JpsFileToPathConverter(jpsProject: JpsProject) : FileToPathConverter {
-    private val baseDirPath = JpsModelSerializationDataService
-        .getBaseDirectory(jpsProject)?.canonicalFile?.invariantSeparatorsPath
-
-    override fun toPath(file: File): String {
-        val path = file.canonicalFile.invariantSeparatorsPath
-        return when {
-            baseDirPath != null && path.startsWith(baseDirPath) ->
-                PROJECT_DIR_PLACEHOLDER + path.substring(baseDirPath.length)
-            else -> path
-        }
-    }
-
-    override fun toFile(path: String): File =
-        when {
-            path.startsWith(PROJECT_DIR_PLACEHOLDER) -> {
-                val basePath = baseDirPath ?: error("Could not get project root dir")
-                File(basePath + path.substring(PROJECT_DIR_PLACEHOLDER.length))
-            }
-            else -> File(path)
-        }
-}
+internal class JpsFileToPathConverter(
+    jpsProject: JpsProject
+) : RelativeFileToPathConverter(JpsModelSerializationDataService.getBaseDirectory(jpsProject))

jps-plugin/src/org/jetbrains/kotlin/jps/build/KotlinBuilder.kt

@@ -648,7 +648,7 @@ class KotlinBuilder : ModuleLevelBuilder(BuilderCategory.SOURCE_PROCESSOR) {
 
         lookupStorageManager.withLookupStorage { lookupStorage ->
             lookupStorage.removeLookupsFrom(dirtyFilesHolder.allDirtyFiles.asSequence() + dirtyFilesHolder.allRemovedFilesFiles.asSequence())
-            lookupStorage.addAll(lookupTracker.lookups.entrySet(), lookupTracker.pathInterner.values)
+            lookupStorage.addAll(lookupTracker.lookups, lookupTracker.pathInterner.values)
         }
     }
 }