ADFA-4332: Batch JVM-symbol removals into one transaction (N+1) (#1411)

* ADFA-4332: add batched removeBySources index API

Sentry APPDEVFORALL-SE. Batched primitive (single transaction, chunked IN)
to collapse the per-file DELETE FROM jvm_symbols N+1. Call-site wiring TODO.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* ADFA-4332: wire batched removeBySources into IndexWorker + repro test

Completes the call-site wiring left TODO by the primitive commit. IndexWorker
now coalesces a run of consecutive RemoveFromIndex commands into a single
JvmSymbolIndex.removeBySources call (one SQLite transaction) instead of one
DELETE FROM jvm_symbols per file (the N+1, Sentry APPDEVFORALL-SE).

WorkerQueue gains a non-blocking pollIndexQueue + single-slot pushBack so a
non-removal command polled while draining is returned in order, not dropped.

IndexWorkerBatchRemovalTest is failing-first: it asserts N removals issue ONE
batched transaction (removeBySourcesBatches==1, removeBySourceCalls==0) and
verifies InMemory/SQLite-style parity. Reverting applyRemovals to the per-file
loop turns the two wiring tests red (expected 1 but was 0).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* ADFA-4332: add KDoc for docstring coverage (CodeRabbit)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* ADFA-4332: add batch-removal API on KtFileMetadataIndex wrapper (review)

itsaky asked (IndexWorker.kt:201) for KtFileMetadataIndex — a wrapper over an
Index — to expose a batch-removal API mirroring the underlying Index, instead of
IndexWorker looping per-file. The symbol side already batched via JvmSymbolIndex's
`WritableIndex by backing` delegation; the file wrapper is hand-written, so it
needed an explicit method.

- KtFileMetadataIndex.removeAll(filePaths): delegates to backing.removeBySources —
  one transaction, paralleling the existing remove(filePath). Idiomatic remove/removeAll.
- IndexWorker.applyRemovals: replace the per-file loop with fileIndex.removeAll(paths);
  now BOTH the symbol and metadata removals collapse into one transaction each (the
  N+1 this ticket targets). KDoc updated to drop the "no batch API" note.
- Test: extend the CountingIndex probe to the file index and assert it also issues
  exactly one batched call with zero per-source deletes — mutation-sensitive, fails
  if the per-file loop is reintroduced.

Verified: :lsp:jvm-symbol-index + :lsp:kotlin testV8DebugUnitTest — 134 tests, 0 failures.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: fryanpan

lsp/indexing/src/main/kotlin/org/appdevforall/codeonthego/indexing/InMemoryIndex.kt

@@ -81,7 +81,28 @@ class InMemoryIndex<T : Indexable>(
     override suspend fun insert(entry: T) = lock.write { insertSingleLocked(entry) }
 
     override suspend fun removeBySource(sourceId: String) = lock.write {
-        val keys = sourceMap.remove(sourceId) ?: return@write
+        removeBySourceLocked(sourceId)
+    }
+
+    /**
+     * Remove every entry belonging to any of [sourceIds].
+     *
+     * Acquires the write lock once and removes each source under it, so the whole
+     * batch is atomic with respect to concurrent readers and writers — there is no
+     * intermediate state in which only some of the sources have been removed.
+     */
+    override suspend fun removeBySources(sourceIds: Collection<String>) = lock.write {
+        for (sourceId in sourceIds) {
+            removeBySourceLocked(sourceId)
+        }
+    }
+
+    /**
+     * Remove all entries for [sourceId] from the primary, source, and secondary
+     * indexes. Caller MUST already hold the write lock; this method does not lock.
+     */
+    private fun removeBySourceLocked(sourceId: String) {
+        val keys = sourceMap.remove(sourceId) ?: return
         for (key in keys) {
             val entry = primaryMap.remove(key) ?: continue
             removeFromSecondaryIndexes(entry)

lsp/indexing/src/main/kotlin/org/appdevforall/codeonthego/indexing/SQLiteIndex.kt

@@ -65,6 +65,12 @@ class SQLiteIndex<T : Indexable>(
 ) : Index<T> {
     companion object {
         private val log = LoggerFactory.getLogger(SQLiteIndex::class.java)
+
+        /**
+         * Max number of `_source_id` placeholders per batched DELETE.
+         * Kept well under SQLite's default 999 bound-parameter limit.
+         */
+        private const val DELETE_CHUNK_SIZE = 900
     }
 
 
@@ -190,6 +196,35 @@ class SQLiteIndex<T : Indexable>(
         ifOpen { db.execSQL("DELETE FROM $tableName WHERE _source_id = ?", arrayOf(sourceId)) }
     }
 
+    /**
+     * Remove every row whose `_source_id` is in [sourceIds] using a single SQLite
+     * transaction. The ids are split into chunks of at most [DELETE_CHUNK_SIZE] so
+     * each `DELETE ... IN (?, ?, ...)` stays within SQLite's bound-parameter limit;
+     * all chunks run inside the one transaction, so the batch commits atomically
+     * (an empty [sourceIds] is a no-op and opens no transaction).
+     *
+     * @param sourceIds Source ids whose rows should be deleted.
+     */
+    override suspend fun removeBySources(sourceIds: Collection<String>) =
+        withContext(Dispatchers.IO) {
+            if (sourceIds.isEmpty()) return@withContext
+            ifOpen {
+                db.beginTransaction()
+                try {
+                    for (chunk in sourceIds.chunked(DELETE_CHUNK_SIZE)) {
+                        val placeholders = chunk.joinToString(",") { "?" }
+                        db.execSQL(
+                            "DELETE FROM $tableName WHERE _source_id IN ($placeholders)",
+                            chunk.toTypedArray(),
+                        )
+                    }
+                    db.setTransactionSuccessful()
+                } finally {
+                    db.endTransaction()
+                }
+            }
+        }
+
     override suspend fun clear() = withContext(Dispatchers.IO) {
         ifOpen { db.execSQL("DELETE FROM $tableName") }
     }

lsp/indexing/src/main/kotlin/org/appdevforall/codeonthego/indexing/api/Index.kt

@@ -68,6 +68,16 @@ interface WritableIndex<T : Indexable> {
      */
     suspend fun removeBySource(sourceId: String)
 
+    /**
+     * Remove all entries from the given sources in a single transaction.
+     *
+     * Equivalent to calling [removeBySource] for each id, but issues the
+     * deletes as one batched, transactional operation instead of N
+     * sequential statements. Implementations should chunk the ids so the
+     * generated SQL stays within parameter limits.
+     */
+    suspend fun removeBySources(sourceIds: Collection<String>)
+
     /**
      * Remove all entries.
      */

lsp/jvm-symbol-index/src/main/kotlin/org/appdevforall/codeonthego/indexing/jvm/KtFileMetadataIndex.kt

@@ -52,6 +52,16 @@ class KtFileMetadataIndex(
 	 */
 	suspend fun remove(filePath: String) = backing.removeBySource(filePath)
 
+	/**
+	 * Remove the metadata records for every path in [filePaths] as a single
+	 * batched, transactional operation.
+	 *
+	 * Equivalent to calling [remove] once per path, but issues the deletes as one
+	 * transaction instead of N — see [Index.removeBySources]. Paths not present in
+	 * the index are ignored.
+	 */
+	suspend fun removeAll(filePaths: Collection<String>) = backing.removeBySources(filePaths)
+
 	/**
 	 * Return the [KtFileMetadata] for [filePath], or `null` if the file is
 	 * not present in the index.

lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/index/IndexWorker.kt

@@ -66,9 +66,13 @@ internal class IndexWorker(
 
 			when (val cmd = queue.take()) {
 				is IndexCommand.RemoveFromIndex -> {
-					val filePath = cmd.path.pathString
-					fileIndex.remove(filePath)
-					sourceIndex.removeBySource(filePath)
+					applyRemovals(
+						first = cmd,
+						fileIndex = fileIndex,
+						sourceIndex = sourceIndex,
+						pollNext = { queue.pollIndexQueue() },
+						pushBack = { queue.pushBackIndexQueue(it) },
+					)
 				}
 
 				is IndexCommand.IndexSourceFile -> {
@@ -160,3 +164,50 @@ internal class IndexWorker(
 		}
 	}
 }
+
+/**
+ * Apply [first] plus any consecutive, immediately-available [IndexCommand.RemoveFromIndex]
+ * commands as a single batched removal.
+ *
+ * The symbol removals and the per-file metadata removals are each collapsed into one
+ * batched call — [JvmSymbolIndex.removeBySources] and [KtFileMetadataIndex.removeAll], a
+ * single SQLite transaction apiece — instead of issuing one `DELETE` per file (one
+ * transaction each), which is the N+1 this fix targets (Sentry APPDEVFORALL-SE).
+ *
+ * [pollNext] returns the next already-queued index command without blocking, or `null`
+ * when none is ready. A polled command that is *not* a removal is handed to [pushBack] so
+ * it is processed (in order) on the next loop iteration rather than dropped.
+ *
+ * @param first      The removal command that triggered this batch.
+ * @param fileIndex  Per-file metadata index; removed via the batched [KtFileMetadataIndex.removeAll].
+ * @param sourceIndex Symbol index; removed via the batched [JvmSymbolIndex.removeBySources].
+ * @param pollNext   Non-blocking poll of the next queued index command.
+ * @param pushBack   Returns a non-removal command to the front of the queue.
+ */
+internal suspend fun applyRemovals(
+	first: IndexCommand.RemoveFromIndex,
+	fileIndex: KtFileMetadataIndex,
+	sourceIndex: JvmSymbolIndex,
+	pollNext: () -> IndexCommand?,
+	pushBack: (IndexCommand) -> Unit,
+) {
+	val paths = ArrayList<String>()
+	paths.add(first.path.pathString)
+
+	while (true) {
+		val next = pollNext() ?: break
+		if (next is IndexCommand.RemoveFromIndex) {
+			paths.add(next.path.pathString)
+		} else {
+			// Not batchable — return it so the main loop handles it next, in order.
+			pushBack(next)
+			break
+		}
+	}
+
+	// Collapse all per-file metadata removals into a single transaction.
+	fileIndex.removeAll(paths)
+
+	// Collapse all symbol removals into a single transaction.
+	sourceIndex.removeBySources(paths)
+}

lsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/compiler/index/WorkerQueue.kt

@@ -9,11 +9,39 @@ internal class WorkerQueue<T> {
 	private val editChannel = Channel<T>(capacity = 20)
 	private val indexChannel = Channel<T>(capacity = 100)
 
+	// Single-slot pushback for an index-queue item that was polled (to coalesce
+	// removals) but turned out not to be batchable. It is returned ahead of the
+	// channels by the next [take], preserving command order.
+	private var pushedBack: T? = null
+
 	suspend fun putScanQueue(item: T) = scanChannel.send(item)
 	suspend fun putEditQueue(item: T) = editChannel.send(item)
 	suspend fun putIndexQueue(item: T) = indexChannel.send(item)
 
+	/**
+	 * Non-blocking poll of the index queue. Returns the next already-available
+	 * index-queue item, or `null` if none is immediately ready.
+	 *
+	 * Used to coalesce a run of consecutive removal commands into a single
+	 * batched index operation (see [IndexWorker]) instead of issuing one
+	 * transaction per command. A polled item that is not batchable must be
+	 * returned via [pushBackIndexQueue] so it is not dropped.
+	 */
+	fun pollIndexQueue(): T? = indexChannel.tryReceive().getOrNull()
+
+	/**
+	 * Return an item previously obtained from [pollIndexQueue] to the front of
+	 * the queue so the next [take] yields it before any channel item. At most
+	 * one item may be pushed back at a time.
+	 */
+	fun pushBackIndexQueue(item: T) {
+		check(pushedBack == null) { "pushBack slot already occupied" }
+		pushedBack = item
+	}
+
 	suspend fun take(): T {
+		pushedBack?.let { pushedBack = null; return it }
+
 		scanChannel.tryReceive().getOrNull()?.let { return it }
 		editChannel.tryReceive().getOrNull()?.let { return it }
 		indexChannel.tryReceive().getOrNull()?.let { return it }