From 722ddc9f9109425ca4532546cf05b74dee2de5b2 Mon Sep 17 00:00:00 2001 From: Dave MacLachlan Date: Fri, 22 Sep 2023 09:14:14 -0700 Subject: [PATCH] Use `fileKey` instead of `canonicalPath` when deduping files `resolveDependencies` showed up as a hotspot when profiling due to `canonicalPath` doing a lot of file system calls. `fileKey` is more efficient and should guarantee uniqueness (`canonicalPath` may fail in the case of hardlinks). The new code is 3x faster in our use case. --- .../src/org/jetbrains/kotlin/konan/file/File.kt | 12 ++++++++++++ .../resolver/impl/KotlinLibraryResolverImpl.kt | 14 ++++++-------- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/compiler/util-io/src/org/jetbrains/kotlin/konan/file/File.kt b/compiler/util-io/src/org/jetbrains/kotlin/konan/file/File.kt index d33ca7b19c8..c88ce3b9833 100644 --- a/compiler/util-io/src/org/jetbrains/kotlin/konan/file/File.kt +++ b/compiler/util-io/src/org/jetbrains/kotlin/konan/file/File.kt @@ -56,6 +56,18 @@ data class File constructor(internal val javaPath: Path) { val listFilesOrEmpty: List get() = if (exists) listFiles else emptyList() + // A fileKey is an object that uniquely identifies the given file. + val fileKey: Any + get() { + // It is not guaranteed that all filesystems have fileKey. If not we fall + // back on canonicalPath which can be significantly slower to get. + var key = Files.readAttributes(javaPath, BasicFileAttributes::class.java).fileKey() + if (key == null) { + key = this.canonicalPath + } + return key + } + fun child(name: String) = File(this, name) fun startsWith(another: File) = javaPath.startsWith(another.javaPath) diff --git a/compiler/util-klib-metadata/src/org/jetbrains/kotlin/library/metadata/resolver/impl/KotlinLibraryResolverImpl.kt b/compiler/util-klib-metadata/src/org/jetbrains/kotlin/library/metadata/resolver/impl/KotlinLibraryResolverImpl.kt index ea4af23988d..deedd5147d5 100644 --- a/compiler/util-klib-metadata/src/org/jetbrains/kotlin/library/metadata/resolver/impl/KotlinLibraryResolverImpl.kt +++ b/compiler/util-klib-metadata/src/org/jetbrains/kotlin/library/metadata/resolver/impl/KotlinLibraryResolverImpl.kt @@ -101,15 +101,14 @@ class KotlinLibraryResolverImpl internal constructor( * 3. Creates resulting [KotlinLibraryResolveResult] object. */ override fun List.resolveDependencies(): KotlinLibraryResolveResult { - val rootLibraries = this.map { KotlinResolvedLibraryImpl(it) } // As far as the list of root libraries is known from the very beginning, the result can be // constructed from the very beginning as well. val result = KotlinLibraryResolverResultImpl(rootLibraries) - val cache = mutableMapOf() - cache.putAll(rootLibraries.map { it.library.libraryFile.canonicalFile to it }) + val cache = mutableMapOf() + cache.putAll(rootLibraries.map { it.library.libraryFile.fileKey to it }) var newDependencies = rootLibraries do { @@ -119,12 +118,12 @@ class KotlinLibraryResolverImpl internal constructor( .filterNot { searchPathResolver.isProvidedByDefault(it) } .mapNotNull { searchPathResolver.resolve(it)?.let(::KotlinResolvedLibraryImpl) } .map { resolved -> - val canonicalFile = resolved.library.libraryFile.canonicalFile - if (canonicalFile in cache) { - library.addDependency(cache[canonicalFile]!!) + val fileKey = resolved.library.libraryFile.fileKey + if (fileKey in cache) { + library.addDependency(cache[fileKey]!!) null } else { - cache.put(canonicalFile, resolved) + cache.put(fileKey, resolved) library.addDependency(resolved) resolved } @@ -133,7 +132,6 @@ class KotlinLibraryResolverImpl internal constructor( .toList() }.flatten() } while (newDependencies.isNotEmpty()) - return result } }