diff --git a/lsp/indexing/src/main/kotlin/org/appdevforall/codeonthego/indexing/InMemoryIndex.kt b/lsp/indexing/src/main/kotlin/org/appdevforall/codeonthego/indexing/InMemoryIndex.kt index 1f7d7f8982..f58ba835f9 100644 --- a/lsp/indexing/src/main/kotlin/org/appdevforall/codeonthego/indexing/InMemoryIndex.kt +++ b/lsp/indexing/src/main/kotlin/org/appdevforall/codeonthego/indexing/InMemoryIndex.kt @@ -155,6 +155,8 @@ class InMemoryIndex( override suspend fun sourceFingerprint(sourceId: String): String? = fingerprints[sourceId] + override suspend fun sourceFingerprints(): Map = HashMap(fingerprints) + override fun subpackages( parent: String, sourceIds: Collection?, diff --git a/lsp/indexing/src/main/kotlin/org/appdevforall/codeonthego/indexing/SQLiteIndex.kt b/lsp/indexing/src/main/kotlin/org/appdevforall/codeonthego/indexing/SQLiteIndex.kt index 4d380b8e22..0f2eca4146 100644 --- a/lsp/indexing/src/main/kotlin/org/appdevforall/codeonthego/indexing/SQLiteIndex.kt +++ b/lsp/indexing/src/main/kotlin/org/appdevforall/codeonthego/indexing/SQLiteIndex.kt @@ -354,6 +354,19 @@ class SQLiteIndex( } } + override suspend fun sourceFingerprints(): Map = + withContext(Dispatchers.IO) { + ifOpen(emptyMap()) { + db.query("SELECT _source_id, _fingerprint FROM $sourcesTableName").use { cursor -> + buildMap { + while (cursor.moveToNext()) { + put(cursor.getString(0), cursor.getString(1)) + } + } + } + } + } + override fun subpackages( parent: String, sourceIds: Collection?, diff --git a/lsp/indexing/src/main/kotlin/org/appdevforall/codeonthego/indexing/api/Index.kt b/lsp/indexing/src/main/kotlin/org/appdevforall/codeonthego/indexing/api/Index.kt index a730687e07..d3bbc85a63 100644 --- a/lsp/indexing/src/main/kotlin/org/appdevforall/codeonthego/indexing/api/Index.kt +++ b/lsp/indexing/src/main/kotlin/org/appdevforall/codeonthego/indexing/api/Index.kt @@ -138,6 +138,9 @@ interface Index : /** Returns the fingerprint recorded for [sourceId] by [insertSource], or `null` if there is none. */ suspend fun sourceFingerprint(sourceId: String): String? + /** Returns every fingerprint recorded by [insertSource], keyed by source id. */ + suspend fun sourceFingerprints(): Map + /** * Refreshes whatever the index keeps to plan its queries, after a bulk change to its entries. * Does nothing by default. diff --git a/lsp/java/src/test/java/com/itsaky/androidide/lsp/java/providers/completion/ImportCompletionProviderTest.kt b/lsp/java/src/test/java/com/itsaky/androidide/lsp/java/providers/completion/ImportCompletionProviderTest.kt index 7bf3999389..8739a2eabc 100644 --- a/lsp/java/src/test/java/com/itsaky/androidide/lsp/java/providers/completion/ImportCompletionProviderTest.kt +++ b/lsp/java/src/test/java/com/itsaky/androidide/lsp/java/providers/completion/ImportCompletionProviderTest.kt @@ -74,24 +74,11 @@ class ImportCompletionProviderTest { @Test fun `a package only the class index holds is completed`() { - val module = module() - module.compileClasspathClasses.append("com.trie.Widget") - - val items = complete("com.", module, FakeClasspathPackages(listOf("com.indexed.Widget"))) + val items = complete("com.", module(), FakeClasspathPackages(listOf("com.indexed.Widget"))) assertThat(items.labels()).contains("indexed") } - @Test - fun `a package only the classpath trie holds is not completed`() { - val module = module() - module.compileClasspathClasses.append("com.trie.Widget") - - val items = complete("com.", module, FakeClasspathPackages(listOf("com.indexed.Widget"))) - - assertThat(items.labels()).doesNotContain("trie") - } - @Test fun `classpath packages and classes are itemised as packages and classes`() { val items = diff --git a/lsp/jvm-symbol-index/src/main/kotlin/org/appdevforall/codeonthego/indexing/jvm/JarIndexingService.kt b/lsp/jvm-symbol-index/src/main/kotlin/org/appdevforall/codeonthego/indexing/jvm/JarIndexingService.kt index 564e92bace..2adcb41441 100644 --- a/lsp/jvm-symbol-index/src/main/kotlin/org/appdevforall/codeonthego/indexing/jvm/JarIndexingService.kt +++ b/lsp/jvm-symbol-index/src/main/kotlin/org/appdevforall/codeonthego/indexing/jvm/JarIndexingService.kt @@ -58,6 +58,7 @@ abstract class JarIndexingService( private val workspaceSupplier: () -> Workspace? = { ProjectManagerImpl.getInstance().workspace }, private val unreadableJarReporter: (List) -> Unit = ::reportUnreadableClasspathJars, private val unreadableJarFilter: (Collection) -> List = { it.toList() }, + private val indexFactory: (Context, String, String) -> JvmSymbolIndex = JvmSymbolIndex::createSqliteIndex, ) : IndexingService { companion object { private val log = LoggerFactory.getLogger(JarIndexingService::class.java) @@ -89,12 +90,7 @@ abstract class JarIndexingService( ) override suspend fun initialize(registry: IndexRegistry) { - val jvmIndex = - JvmSymbolIndex.createSqliteIndex( - context = context, - dbName = dbName, - indexName = indexName, - ) + val jvmIndex = indexFactory(context, dbName, indexName) this.jarIndex = jvmIndex registry.register(indexKey, jvmIndex) @@ -168,11 +164,16 @@ abstract class JarIndexingService( * Step 2: Index any JAR that is not cached, or was cached with a different fingerprint: * a JAR rebuilt at the same path (a snapshot, a local file dependency) is re-scanned. * Newly cached JARs are automatically visible because they're already in the active set. + * + * The fingerprints are read once, before this pass starts any job: a per-JAR read waits on + * the index lock behind every batch the jobs submitted so far are writing, which stretched + * the submission of 404 JARs to minutes and grew the progress total one JAR at a time. */ + val indexedFingerprints = index.sourceFingerprints() val jobs = mutableListOf() for (jarPath in jars) { val fingerprint = jarFingerprint(File(jarPath)) - if (index.sourceFingerprint(jarPath) != fingerprint) { + if (indexedFingerprints[jarPath] != fingerprint) { val job = index.indexSource(jarPath, skipIfExists = true, fingerprint = fingerprint) { sourceId -> scanTrackingUnreadable(jarPath, sourceId, unreadableJars) diff --git a/lsp/jvm-symbol-index/src/main/kotlin/org/appdevforall/codeonthego/indexing/jvm/JvmSymbolIndex.kt b/lsp/jvm-symbol-index/src/main/kotlin/org/appdevforall/codeonthego/indexing/jvm/JvmSymbolIndex.kt index b2172ad921..8673d108bc 100644 --- a/lsp/jvm-symbol-index/src/main/kotlin/org/appdevforall/codeonthego/indexing/jvm/JvmSymbolIndex.kt +++ b/lsp/jvm-symbol-index/src/main/kotlin/org/appdevforall/codeonthego/indexing/jvm/JvmSymbolIndex.kt @@ -116,6 +116,9 @@ open class JvmSymbolIndex( /** Returns the fingerprint [sourceId] was last indexed with, or `null` if it has none. */ suspend fun sourceFingerprint(sourceId: String): String? = backing.sourceFingerprint(sourceId) + /** Returns every source's last-indexed fingerprint, keyed by source id. */ + suspend fun sourceFingerprints(): Map = backing.sourceFingerprints() + /** * Find symbols matching the given prefix. * diff --git a/lsp/jvm-symbol-index/src/test/kotlin/org/appdevforall/codeonthego/indexing/jvm/ClasspathTrieDifferentialTest.kt b/lsp/jvm-symbol-index/src/test/kotlin/org/appdevforall/codeonthego/indexing/jvm/ClasspathTrieDifferentialTest.kt index 187cc8d34c..31b8172bb6 100644 --- a/lsp/jvm-symbol-index/src/test/kotlin/org/appdevforall/codeonthego/indexing/jvm/ClasspathTrieDifferentialTest.kt +++ b/lsp/jvm-symbol-index/src/test/kotlin/org/appdevforall/codeonthego/indexing/jvm/ClasspathTrieDifferentialTest.kt @@ -13,11 +13,11 @@ import java.io.File import java.nio.file.Paths /** - * The index must offer exactly the classes the classpath trie held. + * The index must offer exactly the classes that filled the per-module classpath trie. * - * The oracle is built from the **old** path: `JarFsClasspathReader` filtered to top-level classes is - * literally what filled `ModuleProject.compileClasspathClasses`. Building it from the index instead - * would let the new code agree with itself and pin nothing. + * The oracle is built independently: `JarFsClasspathReader` filtered to top-level classes is what + * `ModuleProject.indexClasspaths` used to fill. Building it from the index instead would let the new + * code agree with itself and pin nothing. * * It runs against real JARs because the cases that matter do not occur in a hand-built fixture: * Kotlin file facades, multi-file class parts and anonymous Kotlin classes in kotlin-stdlib, and diff --git a/lsp/jvm-symbol-index/src/test/kotlin/org/appdevforall/codeonthego/indexing/jvm/JarIndexingServiceProgressTest.kt b/lsp/jvm-symbol-index/src/test/kotlin/org/appdevforall/codeonthego/indexing/jvm/JarIndexingServiceProgressTest.kt index cb44d6513d..537eb1dbb4 100644 --- a/lsp/jvm-symbol-index/src/test/kotlin/org/appdevforall/codeonthego/indexing/jvm/JarIndexingServiceProgressTest.kt +++ b/lsp/jvm-symbol-index/src/test/kotlin/org/appdevforall/codeonthego/indexing/jvm/JarIndexingServiceProgressTest.kt @@ -9,11 +9,16 @@ import io.mockk.mockk import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.Job import kotlinx.coroutines.awaitCancellation +import kotlinx.coroutines.flow.first import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.withTimeoutOrNull +import org.appdevforall.codeonthego.indexing.InMemoryIndex +import org.appdevforall.codeonthego.indexing.api.Index import org.appdevforall.codeonthego.indexing.service.IndexKey import org.appdevforall.codeonthego.indexing.service.IndexRegistry import org.appdevforall.codeonthego.indexing.service.IndexingProgressTracker import org.appdevforall.codeonthego.indexing.service.IndexingState +import org.appdevforall.codeonthego.indexing.util.BackgroundIndexer import org.junit.After import org.junit.Rule import org.junit.Test @@ -46,6 +51,33 @@ class JarIndexingServiceProgressTest { assertThat((service.stateWhileCompleting as? IndexingState.Indexing)?.total).isEqualTo(2) } + @Test + fun `a refresh counts every changed JAR without waiting on a per-JAR fingerprint read`() { + val backing = GatedFingerprintIndex(InMemoryIndex(JvmSymbolDescriptor)) + val service = + TestService( + jars = setOf(jar("a.jar"), jar("b.jar"), jar("c.jar")), + indexFactory = { _, _, _ -> JvmSymbolIndex(backing, BackgroundIndexer(backing)) }, + ) + + val counted = + service.use { + runBlocking { + it.initialize(IndexRegistry()) + val pass = it.refresh() + val counted = + withTimeoutOrNull(5_000) { + tracker.state.first { state -> (state as? IndexingState.Indexing)?.total == 3 } + } + backing.gate.complete(Unit) + pass.join() + counted + } + } + + assertThat(counted).isNotNull() + } + @Test fun `the state is Idle once a refresh finishes`() { val service = TestService(jars = setOf(jar("a.jar"))) @@ -111,10 +143,26 @@ class JarIndexingServiceProgressTest { .apply { writeBytes(ByteArray(1)) } .absolutePath + /** + * Holds every single-source fingerprint read until [gate] completes, as the SQLite index's lock + * does while indexing jobs write. + */ + private class GatedFingerprintIndex( + private val backing: Index, + ) : Index by backing { + val gate = CompletableDeferred() + + override suspend fun sourceFingerprint(sourceId: String): String? { + gate.await() + return backing.sourceFingerprint(sourceId) + } + } + private inner class TestService( private val jars: Set, private val onComplete: suspend () -> Unit = {}, - ) : JarIndexingService(context, tracker, workspaceSupplier = { mockk() }) { + indexFactory: (Context, String, String) -> JvmSymbolIndex = JvmSymbolIndex::createSqliteIndex, + ) : JarIndexingService(context, tracker, workspaceSupplier = { mockk() }, indexFactory = indexFactory) { var stateWhileCompleting: IndexingState? = null override val id = "jar-indexing-service-progress-test" diff --git a/lsp/jvm-symbol-index/src/test/kotlin/org/appdevforall/codeonthego/indexing/jvm/ModuleClasspathLookupDifferentialTest.kt b/lsp/jvm-symbol-index/src/test/kotlin/org/appdevforall/codeonthego/indexing/jvm/ModuleClasspathLookupDifferentialTest.kt index 99a7b154f5..262fbe5432 100644 --- a/lsp/jvm-symbol-index/src/test/kotlin/org/appdevforall/codeonthego/indexing/jvm/ModuleClasspathLookupDifferentialTest.kt +++ b/lsp/jvm-symbol-index/src/test/kotlin/org/appdevforall/codeonthego/indexing/jvm/ModuleClasspathLookupDifferentialTest.kt @@ -23,8 +23,8 @@ import java.nio.file.Paths * A module's [ModuleClasspathLookup] answers every query its consumers asked of the module's * classpath trie the way the trie did. * - * The oracle is built the way `ModuleProject.indexClasspaths` fills the trie: `JarFsClasspathReader` - * filtered to top-level classes, appended to a [ClassTrie]. The lookup reads real SQLite-backed + * The oracle is built the way `ModuleProject.indexClasspaths` used to fill the trie: + * `JarFsClasspathReader` filtered to top-level classes, appended to a [ClassTrie]. The lookup reads real SQLite-backed * indexes filled by [CombinedJarScanner], so the scanner, the package table and the query layer are * all under test. kotlin-stdlib sits in the library index and, like `R.jar`, in the generated index * too, so every answer must be deduplicated; JUnit sits in the module-output index, standing in for diff --git a/subprojects/projects/src/main/java/com/itsaky/androidide/projects/api/ModuleProject.kt b/subprojects/projects/src/main/java/com/itsaky/androidide/projects/api/ModuleProject.kt index f54eda8814..27f5260a93 100644 --- a/subprojects/projects/src/main/java/com/itsaky/androidide/projects/api/ModuleProject.kt +++ b/subprojects/projects/src/main/java/com/itsaky/androidide/projects/api/ModuleProject.kt @@ -23,11 +23,9 @@ import com.itsaky.androidide.javac.services.fs.CacheFSInfoSingleton import com.itsaky.androidide.lookup.Lookup import com.itsaky.androidide.project.Common import com.itsaky.androidide.project.GradleModels -import com.itsaky.androidide.projects.classpath.JarFsClasspathReader import com.itsaky.androidide.projects.models.DEFAULT_COMPILER_SETTINGS import com.itsaky.androidide.projects.models.bootClassPaths import com.itsaky.androidide.projects.util.BootClasspathProvider -import com.itsaky.androidide.utils.ClassTrie import com.itsaky.androidide.utils.DocumentUtils import com.itsaky.androidide.utils.SourceClassTrie import com.itsaky.androidide.utils.SourceClassTrie.SourceNode @@ -74,9 +72,6 @@ abstract class ModuleProject( @JvmField val compileJavaSourceClasses = SourceClassTrie() - @JvmField - val compileClasspathClasses = ClassTrie() - /** * Get the source directories of this module (non-transitive i.e for this module only). * @@ -217,7 +212,7 @@ abstract class ModuleProject( */ fun findSourceRoot(file: File): Path? = getCompileSourceDirectories().find { file.path.startsWith(it.path) }?.toPath() - /** Finds the source files and classes from source directories and classpaths and indexes them. */ + /** Indexes the source files' classes and prepares the classpaths for the compiler. */ @RestrictTo(RestrictTo.Scope.LIBRARY_GROUP_PREFIX) fun indexSourcesAndClasspaths() { log.info("Indexing sources and classpaths for project: {}", path) @@ -227,9 +222,6 @@ abstract class ModuleProject( @RestrictTo(RestrictTo.Scope.LIBRARY_GROUP_PREFIX) fun indexClasspaths() { - this.compileClasspathClasses.clear() - - val watch = StopWatch("Indexing classpaths") val paths = getCompileClasspaths().filter { it.exists() } for (path in paths) { @@ -238,13 +230,6 @@ abstract class ModuleProject( CacheFSInfoSingleton.cache(CacheFSInfoSingleton.getCanonicalFile(path.toPath())) } - val reader = JarFsClasspathReader() - val topLevelClasses = reader.listClasses(paths).filter { it.isTopLevel } - topLevelClasses.forEach { this.compileClasspathClasses.append(it.name) } - - watch.log() - log.debug("Found {} classpaths.", topLevelClasses.size) - if (this is AndroidModule) { BootClasspathProvider.update(bootClassPaths.map { it.path }) }