From c39676869772922d7045dab4232b581a1b64bdd6 Mon Sep 17 00:00:00 2001 From: darken Date: Thu, 28 Nov 2024 18:31:47 +0100 Subject: [PATCH] Improve `filterDistinctRoots` performance By using a cheap path length comparison we can return faster from within `isAncestorOf` in some cases. --- .../sdmse/common/files/APathLookupExtensions.kt | 10 +++++++++- .../common/files/local/LocalPathExtensions.kt | 13 +++++++------ .../sdmse/common/files/APathLookupExtensionTest.kt | 14 ++++++++++++++ 3 files changed, 30 insertions(+), 7 deletions(-) diff --git a/app-common-io/src/main/java/eu/darken/sdmse/common/files/APathLookupExtensions.kt b/app-common-io/src/main/java/eu/darken/sdmse/common/files/APathLookupExtensions.kt index 4b6416caa..3e358b9a9 100644 --- a/app-common-io/src/main/java/eu/darken/sdmse/common/files/APathLookupExtensions.kt +++ b/app-common-io/src/main/java/eu/darken/sdmse/common/files/APathLookupExtensions.kt @@ -1,5 +1,7 @@ package eu.darken.sdmse.common.files +import eu.darken.sdmse.common.debug.logging.Logging.Priority.VERBOSE +import eu.darken.sdmse.common.debug.logging.log import kotlinx.coroutines.flow.Flow import okio.FileHandle @@ -91,8 +93,14 @@ fun APathLookup<*>.removePrefix(prefix: APath, overlap: Int = 0) = lookedUp.removePrefix(prefix, overlap) fun Collection>.filterDistinctRoots(): Set> { + log(VERBOSE) { "Creating lookup map..." } val lookupMap = this.associateBy { it.lookedUp } - return lookupMap.keys.filterDistinctRoots().map { lookupMap.getValue(it) }.toSet() + log(VERBOSE) { "Lookup map created with ${lookupMap.size} entries, now filtering..." } + return lookupMap.keys + .filterDistinctRoots() + .map { lookupMap.getValue(it) } + .toSet() + .also { log(VERBOSE) { "After filtering we got ${it.size} distinct roots" } } } val APathLookup<*>.extension: String? diff --git a/app-common-io/src/main/java/eu/darken/sdmse/common/files/local/LocalPathExtensions.kt b/app-common-io/src/main/java/eu/darken/sdmse/common/files/local/LocalPathExtensions.kt index 5e0b8c15f..1dd30341e 100644 --- a/app-common-io/src/main/java/eu/darken/sdmse/common/files/local/LocalPathExtensions.kt +++ b/app-common-io/src/main/java/eu/darken/sdmse/common/files/local/LocalPathExtensions.kt @@ -83,13 +83,14 @@ fun LocalPath.performLookupExtended( } fun LocalPath.isAncestorOf(child: LocalPath): Boolean { - val parentPath = this.asFile().absolutePath - val childPath = child.asFile().absolutePath + val parentPath = this.asFile().path + val childPath = child.asFile().path - return when (parentPath) { - childPath -> false - File.separator -> childPath.startsWith(parentPath) - else -> childPath.startsWith(parentPath + File.separator) + return when { + parentPath.length >= childPath.length -> false + !childPath.startsWith(parentPath) -> false + parentPath == File.separator -> true + else -> childPath[parentPath.length] == File.separatorChar } } diff --git a/app-common-io/src/test/java/eu/darken/sdmse/common/files/APathLookupExtensionTest.kt b/app-common-io/src/test/java/eu/darken/sdmse/common/files/APathLookupExtensionTest.kt index 835691a7d..bbc207565 100644 --- a/app-common-io/src/test/java/eu/darken/sdmse/common/files/APathLookupExtensionTest.kt +++ b/app-common-io/src/test/java/eu/darken/sdmse/common/files/APathLookupExtensionTest.kt @@ -142,4 +142,18 @@ class APathLookupExtensionTest : BaseTest() { lookup4 ) } + + @Test fun `filterDistinctRoots performance`() { + val targets = mutableSetOf() + (1..8000).forEach { + LocalPathLookup( + lookedUp = LocalPath.build("parentA", "parentB", "file$it"), + fileType = FileType.FILE, + size = 0, + modifiedAt = Instant.EPOCH, + target = null, + ).run { targets.add(this) } + } + targets.filterDistinctRoots().size shouldBe 8000 + } }