From de4542061e9f02db885d8e9c82128cbfdca385af Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Sat, 8 Aug 2026 03:09:41 -0700 Subject: [PATCH 1/2] fix: Bound live build output memory Fixes #1367 --- .../fragments/output/BuildOutputBuffer.kt | 146 ++++++++++++ .../fragments/output/BuildOutputFragment.kt | 225 ++++++++++-------- .../viewmodel/BuildOutputViewModel.kt | 28 ++- .../fragments/output/BuildOutputBufferTest.kt | 196 +++++++++++++++ 4 files changed, 492 insertions(+), 103 deletions(-) create mode 100644 app/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputBuffer.kt create mode 100644 app/src/test/java/com/itsaky/androidide/fragments/output/BuildOutputBufferTest.kt diff --git a/app/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputBuffer.kt b/app/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputBuffer.kt new file mode 100644 index 0000000000..793f83aa58 --- /dev/null +++ b/app/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputBuffer.kt @@ -0,0 +1,146 @@ +/* + * This file is part of AndroidIDE. + * + * AndroidIDE is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * AndroidIDE is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with AndroidIDE. If not, see . + */ + +package com.itsaky.androidide.fragments.output + +import kotlinx.coroutines.channels.Channel + +/** + * Thread-safe pending build output with fixed memory and batch budgets. + * + * Inputs are indivisible: one input larger than [maxBatchChars] is emitted alone, while one larger + * than [maxPendingChars] is omitted. Other inputs are never split or reordered. + */ +internal class BuildOutputBuffer( + private val maxPendingChars: Int = DEFAULT_MAX_PENDING_CHARS, + private val maxBatchChars: Int = DEFAULT_MAX_BATCH_CHARS, +) { + data class Batch( + val text: String, + val sessionGeneration: Int, + ) + + private sealed interface Entry { + val sessionGeneration: Int + + data class Text( + val value: String, + override val sessionGeneration: Int, + ) : Entry + + data class Omission( + var lineCount: Long, + override val sessionGeneration: Int, + ) : Entry + } + + private val entries = ArrayDeque() + private val available = Channel(Channel.CONFLATED) + private val lock = Any() + private var retainedChars = 0 + private var omission: Entry.Omission? = null + + internal val pendingChars: Int + get() = synchronized(lock) { retainedChars } + + init { + require(maxPendingChars > 0) { "maxPendingChars must be positive" } + require(maxBatchChars > 0) { "maxBatchChars must be positive" } + } + + fun offer( + text: String, + sessionGeneration: Int, + ) { + if (text.isEmpty()) return + val needsNewline = !text.endsWith('\n') + val normalizedLength = text.length.toLong() + if (needsNewline) 1 else 0 + val lineCount = text.count { it == '\n' }.toLong() + if (needsNewline) 1 else 0 + synchronized(lock) { + if ( + normalizedLength > maxPendingChars.toLong() || + normalizedLength > (maxPendingChars - retainedChars).toLong() + ) { + val existingOmission = omission + if ( + existingOmission?.sessionGeneration == sessionGeneration && + entries.lastOrNull() === existingOmission + ) { + existingOmission.lineCount += lineCount + } else { + val marker = Entry.Omission(lineCount, sessionGeneration) + omission = marker + entries.addLast(marker) + } + } else { + val normalized = if (needsNewline) "$text\n" else text + entries.addLast(Entry.Text(normalized, sessionGeneration)) + retainedChars += normalizedLength.toInt() + } + available.trySend(Unit) + } + } + + suspend fun takeBatch(): Batch { + while (true) { + available.receive() + val batch = synchronized(lock) { takeAvailableBatch() } + if (batch != null) return batch + } + } + + fun clear() { + synchronized(lock) { + entries.clear() + retainedChars = 0 + omission = null + while (available.tryReceive().isSuccess) { + // Discard stale wakeups from the cleared build session. + } + } + } + + private fun takeAvailableBatch(): Batch? { + if (entries.isEmpty()) return null + val sessionGeneration = entries.first().sessionGeneration + val batch = StringBuilder(minOf(retainedChars, maxBatchChars)) + while (entries.isNotEmpty()) { + val entry = entries.first() + if (entry.sessionGeneration != sessionGeneration) break + val value = + when (entry) { + is Entry.Text -> entry.value + is Entry.Omission -> omissionMarker(entry.lineCount) + } + if (batch.isNotEmpty() && batch.length + value.length > maxBatchChars) break + + entries.removeFirst() + batch.append(value) + if (entry is Entry.Text) retainedChars -= entry.value.length + if (entry is Entry.Omission && omission === entry) omission = null + } + if (entries.isNotEmpty()) available.trySend(Unit) + return Batch(batch.toString(), sessionGeneration) + } + + private fun omissionMarker(lineCount: Long): String = "[$lineCount build output lines omitted]\n" + + companion object { + private const val DEFAULT_MAX_PENDING_CHARS = 256 * 1024 + private const val DEFAULT_MAX_BATCH_CHARS = 32 * 1024 + } +} diff --git a/app/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputFragment.kt b/app/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputFragment.kt index 6c5e4c42dc..497c42c91b 100644 --- a/app/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputFragment.kt +++ b/app/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputFragment.kt @@ -37,8 +37,6 @@ import com.itsaky.androidide.utils.dpToPx import com.itsaky.androidide.utils.flashInfo import com.itsaky.androidide.viewmodel.BuildOutputViewModel import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.channels.Channel -import kotlinx.coroutines.channels.ReceiveChannel import kotlinx.coroutines.flow.collectLatest import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.drop @@ -60,7 +58,7 @@ class BuildOutputFragment : override val currentEditor: IDEEditor? get() = editor - private val logChannel = Channel(Channel.UNLIMITED) + private val outputBuffer = BuildOutputBuffer() private var searchLayout: EditorSearchLayout? = null private var filterBar: LogFilterBarController? = null @@ -78,6 +76,11 @@ class BuildOutputFragment : // in-flight batch flush drained before the replacement can detect it and drop itself. @Volatile private var editorContentGeneration = 0 + + @Volatile + private var visibleEditorChars = 0 + @Volatile + private var editorSourceChars = 0 private val noMatchTracker = FilterNoMatchTracker() // Reads view state (bar visibility), so evaluate it on the main thread. @@ -98,7 +101,7 @@ class BuildOutputFragment : launch { restoreWindowFromViewModel() } launch(Dispatchers.Default) { processLogs() } launch { - val content = buildOutputViewModel.getFullContent() + val content = withContext(Dispatchers.IO) { buildOutputViewModel.getWindowForEditor() } buildOutputViewModel.setCachedSnapshot(content) } launch { @@ -126,10 +129,14 @@ class BuildOutputFragment : val window = withContext(Dispatchers.IO) { buildOutputViewModel.getWindowForEditor() } val filtered = withContext(Dispatchers.Default) { - BuildOutputViewModel.filterLines(window, query, showTimestamps, showDeltas) + BuildOutputViewModel.fitEditorWindow( + BuildOutputViewModel.filterLines(window, query, showTimestamps, showDeltas), + ) } withContext(Dispatchers.Main) { editor?.setText(filtered) + visibleEditorChars = filtered.length + editorSourceChars = window.length val isSourceEmpty = window.isBlank() updateEmptyState(isSourceEmpty = isSourceEmpty, isFilterActive = isFilterActive) if (noMatchTracker.onRender(isSourceEmpty = isSourceEmpty, isFilteredEmpty = filtered.isBlank())) { @@ -287,57 +294,79 @@ class BuildOutputFragment : }.also { filterBar = it } } - private suspend fun restoreWindowFromViewModel() { - val window = withContext(Dispatchers.IO) { buildOutputViewModel.getWindowForEditor() } - val content = - BuildOutputViewModel.filterLines( - window, - buildOutputViewModel.filterText.value, - buildOutputViewModel.showTimestamps.value, - buildOutputViewModel.showDeltas.value, - ) - val query = buildOutputViewModel.filterText.value - val isSourceEmpty = window.isBlank() - val isFilteredEmpty = content.isBlank() - - withContext(Dispatchers.Main) { - updateEmptyState(isSourceEmpty = isSourceEmpty, isFilterActive = isFilterActive) - noMatchTracker.prime(isFilteredEmpty) - if (!isSourceEmpty && isFilteredEmpty) { - editor?.setText("") - onContentReplaced() - } - } + private suspend fun restoreWindowFromViewModel() = + withContext(Dispatchers.Default) { + val window = withContext(Dispatchers.IO) { buildOutputViewModel.getWindowForEditor() } + val content = + BuildOutputViewModel.fitEditorWindow( + BuildOutputViewModel.filterLines( + window, + buildOutputViewModel.filterText.value, + buildOutputViewModel.showTimestamps.value, + buildOutputViewModel.showDeltas.value, + ), + ) + val generationAtRestore = editorContentGeneration + val visibleCharsAtRestore = visibleEditorChars + val sourceCharsAtRestore = editorSourceChars + fun isRestoreCurrent() = + editorContentGeneration == generationAtRestore && + visibleEditorChars == visibleCharsAtRestore && + editorSourceChars == sourceCharsAtRestore + val isSourceEmpty = window.isBlank() + val isFilteredEmpty = content.isBlank() - if (content.isEmpty()) return - withContext(Dispatchers.Main) { - val editor = this@BuildOutputFragment.editor ?: return@withContext - val layoutCompleted = - withTimeoutOrNull(LAYOUT_TIMEOUT_MS) { - editor.awaitLayout(onForceVisible = { updateEmptyState(isSourceEmpty = false, isFilterActive = isFilterActive) }) + withContext(Dispatchers.Main) { + updateEmptyState(isSourceEmpty = isSourceEmpty, isFilterActive = isFilterActive) + noMatchTracker.prime(isFilteredEmpty) + if (!isSourceEmpty && isFilteredEmpty) { + editorContentMutex.withLock { + if (isRestoreCurrent()) { + editor?.setText("") + visibleEditorChars = 0 + editorSourceChars = window.length + onContentReplaced() + } + } } - if (layoutCompleted != null) { - editor.appendBatch(content) - updateEmptyState(isSourceEmpty = false, isFilterActive = isFilterActive) - } else { - // Timeout: defer append until layout is ready so content is not lost - val generationAtRestore = editorContentGeneration - val job = - viewLifecycleOwner.lifecycleScope.launch(Dispatchers.Main) { - editor.run { - awaitLayout(onForceVisible = { updateEmptyState(isSourceEmpty = false, isFilterActive = isFilterActive) }) - editorContentMutex.withLock { - if (editorContentGeneration == generationAtRestore) { - appendBatch(content) - updateEmptyState(isSourceEmpty = false, isFilterActive = isFilterActive) + } + + if (content.isEmpty()) return@withContext + withContext(Dispatchers.Main) { + val editor = this@BuildOutputFragment.editor ?: return@withContext + val layoutCompleted = + withTimeoutOrNull(LAYOUT_TIMEOUT_MS) { + editor.awaitLayout(onForceVisible = { updateEmptyState(isSourceEmpty = false, isFilterActive = isFilterActive) }) + } + if (layoutCompleted != null) { + editorContentMutex.withLock { + if (isRestoreCurrent()) { + editor.appendBatch(content) + visibleEditorChars += content.length + editorSourceChars = window.length + updateEmptyState(isSourceEmpty = false, isFilterActive = isFilterActive) + } + } + } else { + // Timeout: defer append until layout is ready so content is not lost + val job = + viewLifecycleOwner.lifecycleScope.launch(Dispatchers.Main) { + editor.run { + awaitLayout(onForceVisible = { updateEmptyState(isSourceEmpty = false, isFilterActive = isFilterActive) }) + editorContentMutex.withLock { + if (isRestoreCurrent()) { + appendBatch(content) + visibleEditorChars += content.length + editorSourceChars = window.length + updateEmptyState(isSourceEmpty = false, isFilterActive = isFilterActive) + } } } } - } - job.join() + job.join() + } } } - } override fun onDestroyView() { searchLayout = null @@ -351,13 +380,13 @@ class BuildOutputFragment : // Avoid forcing the activityViewModels lazy init (which calls requireActivity()) // when the fragment is detached, otherwise an IllegalStateException is thrown. if (!isAdded || activity == null) return - while (logChannel.tryReceive().isSuccess) { - // Discard: these lines belong to the session being cleared. - } // Invalidate in-flight flushes before deleting content, so a batch drained from the - // channel earlier cannot re-seed the cleared session. + // buffer earlier cannot re-seed the cleared session. sessionGeneration++ + outputBuffer.clear() editorContentGeneration++ + visibleEditorChars = 0 + editorSourceChars = 0 noMatchTracker.reset() buildOutputViewModel.clear() super.clearOutput() @@ -377,55 +406,22 @@ class BuildOutputFragment : fun appendOutput(output: String?) { if (!output.isNullOrEmpty()) { - logChannel.trySend(output) - } - } - - /** - * Ensures the string ends with a newline character (`\n`). - * Useful for maintaining correct formatting when concatenating log lines. - */ - private fun String.ensureNewline(): String = if (endsWith('\n')) this else "$this\n" - - /** - * Immediately drains (consumes) all available messages from the channel into the [buffer]. - * - * This is a **non-blocking** operation that enables batching, grouping hundreds of pending lines - * into a single memory operation to avoid saturating the UI queue. - */ - private fun ReceiveChannel.drainTo(buffer: StringBuilder) { - var result = tryReceive() - while (result.isSuccess) { - val line = result.getOrNull() - if (!line.isNullOrEmpty()) { - buffer.append(line.ensureNewline()) - } - result = tryReceive() + outputBuffer.offer(output, sessionGeneration) } } /** * Main log orchestrator: Consumes, Batches, and Dispatches. * - * 1. Suspends (zero CPU usage) until the first log arrives. - * 2. Wakes up and drains the entire queue (Batching). - * 3. Sends the complete block to the UI in a single pass. + * Suspends until bounded output is available, then sends one bounded batch to the UI. */ - private suspend fun processLogs() = - with(StringBuilder()) { - for (firstLine in logChannel) { - val sessionGenAtDrain = sessionGeneration - val editorGenAtDrain = editorContentGeneration - append(firstLine.ensureNewline()) - logChannel.drainTo(this) - - if (isNotEmpty()) { - val batchText = toString() - clear() - flushToEditor(batchText, sessionGenAtDrain, editorGenAtDrain) - } - } + private suspend fun processLogs() { + while (true) { + val batch = outputBuffer.takeBatch() + val editorGenAtDrain = editorContentGeneration + flushToEditor(batch.text, batch.sessionGeneration, editorGenAtDrain) } + } /** * Performs the safe UI update on the Main Thread. @@ -443,7 +439,10 @@ class BuildOutputFragment : // A clear (new build) after this batch was drained invalidates session append. if (sessionGen != sessionGeneration) return - buildOutputViewModel.append(text) + buildOutputViewModel.append(text) { sessionGen == sessionGeneration } + if (sessionGen != sessionGeneration) return + val refreshEditorWindow = + BuildOutputViewModel.wouldExceedEditorWindow(editorSourceChars, text.length) // The session file always gets the full text; the editor only shows matching lines val visibleText = @@ -453,13 +452,41 @@ class BuildOutputFragment : buildOutputViewModel.showTimestamps.value, buildOutputViewModel.showDeltas.value, ) - if (visibleText.isEmpty()) { + if (visibleText.isEmpty() && !refreshEditorWindow) { + withContext(Dispatchers.Main) { + if (sessionGen == sessionGeneration) editorSourceChars += text.length + } return } + val refreshedWindow = + if (refreshEditorWindow) { + val window = withContext(Dispatchers.IO) { buildOutputViewModel.getWindowForEditor() } + withContext(Dispatchers.Default) { + Pair( + BuildOutputViewModel.fitEditorWindow( + BuildOutputViewModel.filterLines( + window, + buildOutputViewModel.filterText.value, + buildOutputViewModel.showTimestamps.value, + buildOutputViewModel.showDeltas.value, + ), + ), + window.length, + ) + } + } else { + null + } withContext(Dispatchers.Main) { updateEmptyState(isSourceEmpty = false, isFilterActive = isFilterActive) - if (visibleText.isEmpty()) { + if (editorGen != editorContentGeneration) return@withContext + if (refreshedWindow != null) { + editorContentGeneration++ + editor?.setText(refreshedWindow.first) + visibleEditorChars = refreshedWindow.first.length + editorSourceChars = refreshedWindow.second + onContentReplaced() return@withContext } editor?.run { @@ -471,6 +498,8 @@ class BuildOutputFragment : // clearOutput() or renderFiltered() may have run since the file append. if (editorGen == editorContentGeneration) { appendBatch(visibleText) + visibleEditorChars += visibleText.length + editorSourceChars += text.length updateEmptyState(isSourceEmpty = false, isFilterActive = isFilterActive) } } else { @@ -481,6 +510,8 @@ class BuildOutputFragment : editorContentMutex.withLock { if (editorGen == editorContentGeneration) { appendBatch(visibleText) + visibleEditorChars += visibleText.length + editorSourceChars += text.length updateEmptyState(isSourceEmpty = false, isFilterActive = isFilterActive) } } diff --git a/app/src/main/java/com/itsaky/androidide/viewmodel/BuildOutputViewModel.kt b/app/src/main/java/com/itsaky/androidide/viewmodel/BuildOutputViewModel.kt index dc94377062..6065a05afd 100644 --- a/app/src/main/java/com/itsaky/androidide/viewmodel/BuildOutputViewModel.kt +++ b/app/src/main/java/com/itsaky/androidide/viewmodel/BuildOutputViewModel.kt @@ -90,10 +90,14 @@ class BuildOutputViewModel( * Appends text to the session file. File I/O is performed on a background dispatcher; call from * any thread. Prefer calling before switching to Main so disk write does not block the UI. */ - suspend fun append(text: String) { + suspend fun append( + text: String, + isCurrentSession: () -> Boolean = { true }, + ) { if (text.isEmpty()) return withContext(Dispatchers.IO) { lock.withLock { + if (!isCurrentSession()) return@withLock try { FileOutputStream(sessionFile, true).use { it.write(text.toByteArray(StandardCharsets.UTF_8)) @@ -108,12 +112,12 @@ class BuildOutputViewModel( } /** - * Returns the last [WINDOW_SIZE_CHARS] characters from the session file for the editor to + * Returns the last [EDITOR_WINDOW_MAX_CHARS] characters from the session file for the editor to * display (e.g. initial view or after rotation). Returns empty string if no content. */ fun getWindowForEditor(): String = lock.withLock { - readTailFromFile(sessionFile, WINDOW_SIZE_CHARS) + readTailFromFile(sessionFile, EDITOR_WINDOW_MAX_CHARS) } /** @@ -194,6 +198,20 @@ class BuildOutputViewModel( } companion object { + internal const val EDITOR_WINDOW_MAX_CHARS = 512 * 1024 + + internal fun wouldExceedEditorWindow( + currentChars: Int, + incomingChars: Int, + ): Boolean = currentChars > EDITOR_WINDOW_MAX_CHARS - incomingChars + + internal fun fitEditorWindow(content: String): String = + if (content.length <= EDITOR_WINDOW_MAX_CHARS) { + content + } else { + content.takeLast(EDITOR_WINDOW_MAX_CHARS) + } + // Must mirror formatLinePrefix exactly; the round-trip is covered by BuildOutputFilterTest. // Anchored to line start so timestamp-shaped text inside a message is never stripped. private val PREFIX_REGEX = @@ -261,10 +279,8 @@ class BuildOutputViewModel( } private const val SESSION_FILE_NAME = "build_output_session.txt" - private const val WINDOW_SIZE_CHARS = 512 * 1024 - /** Max length of [cachedContentSnapshot] to bound memory. */ - private const val CACHE_SNAPSHOT_MAX_CHARS = WINDOW_SIZE_CHARS + private const val CACHE_SNAPSHOT_MAX_CHARS = EDITOR_WINDOW_MAX_CHARS private val log = org.slf4j.LoggerFactory.getLogger(BuildOutputViewModel::class.java) } } diff --git a/app/src/test/java/com/itsaky/androidide/fragments/output/BuildOutputBufferTest.kt b/app/src/test/java/com/itsaky/androidide/fragments/output/BuildOutputBufferTest.kt new file mode 100644 index 0000000000..8139d44caa --- /dev/null +++ b/app/src/test/java/com/itsaky/androidide/fragments/output/BuildOutputBufferTest.kt @@ -0,0 +1,196 @@ +/* + * This file is part of AndroidIDE. + * + * AndroidIDE is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * AndroidIDE is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with AndroidIDE. If not, see . + */ + +package com.itsaky.androidide.fragments.output + +import com.itsaky.androidide.viewmodel.BuildOutputViewModel +import kotlinx.coroutines.test.runTest +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test + +class BuildOutputBufferTest { + private fun BuildOutputBuffer.offer(text: String) { + offer(text, sessionGeneration = 0) + } + + @Test + fun `output below limits is emitted in order with one trailing newline`() = + runTest { + val buffer = BuildOutputBuffer(maxPendingChars = 64, maxBatchChars = 64) + + buffer.offer("first") + buffer.offer("second\n") + + assertEquals("first\nsecond\n", buffer.takeBatch().text) + } + + @Test + fun `output is split into bounded batches without reordering`() = + runTest { + val buffer = BuildOutputBuffer(maxPendingChars = 64, maxBatchChars = 6) + + buffer.offer("aa") + buffer.offer("bb") + buffer.offer("cc") + + val first = buffer.takeBatch().text + val second = buffer.takeBatch().text + assertEquals("aa\nbb\n", first) + assertEquals("cc\n", second) + assertTrue(first.length <= 6) + assertTrue(second.length <= 6) + } + + @Test + fun `one indivisible input may exceed the batch limit`() = + runTest { + val buffer = BuildOutputBuffer(maxPendingChars = 64, maxBatchChars = 4) + + buffer.offer("oversized") + + assertEquals("oversized\n", buffer.takeBatch().text) + } + + @Test + fun `overflow is coalesced before retained output resumes`() = + runTest { + val buffer = BuildOutputBuffer(maxPendingChars = 8, maxBatchChars = 128) + + buffer.offer("one") + buffer.offer("two") + buffer.offer("dropped one") + buffer.offer("dropped two\nand three") + + assertEquals( + "one\ntwo\n[3 build output lines omitted]\n", + buffer.takeBatch().text, + ) + assertTrue(buffer.pendingChars <= 8) + } + + @Test + fun `overflow after resumed output starts a new omission marker`() = + runTest { + val buffer = BuildOutputBuffer(maxPendingChars = 8, maxBatchChars = 4) + + buffer.offer("one") + buffer.offer("two") + buffer.offer("first dropped") + assertEquals("one\n", buffer.takeBatch().text) + + buffer.offer("new") + buffer.offer("second dropped") + + assertEquals("two\n", buffer.takeBatch().text) + assertEquals("[1 build output lines omitted]\n", buffer.takeBatch().text) + assertEquals("new\n", buffer.takeBatch().text) + assertEquals("[1 build output lines omitted]\n", buffer.takeBatch().text) + } + + @Test + fun `clear resets pending output and overflow accounting`() = + runTest { + val buffer = BuildOutputBuffer(maxPendingChars = 4, maxBatchChars = 64) + + buffer.offer("kept") + buffer.offer("dropped") + buffer.clear() + buffer.offer("new") + + assertEquals("new\n", buffer.takeBatch().text) + } + + @Test + fun `in-flight batch keeps the session generation from its producer`() = + runTest { + val buffer = BuildOutputBuffer(maxPendingChars = 64, maxBatchChars = 64) + + buffer.offer("old", sessionGeneration = 3) + val inFlight = buffer.takeBatch() + buffer.clear() + buffer.offer("new", sessionGeneration = 4) + + assertEquals(3, inFlight.sessionGeneration) + assertEquals(4, buffer.takeBatch().sessionGeneration) + } + + @Test + fun `repeated live batches refresh to the newest bounded editor tail`() { + val chunk = "x".repeat(200 * 1024) + val newest = "newest build output\n" + var session = "" + var visible = "" + var refreshCount = 0 + + for (batch in listOf(chunk, chunk, chunk + newest)) { + session += batch + visible = + if (BuildOutputViewModel.wouldExceedEditorWindow(visible.length, batch.length)) { + refreshCount++ + session.takeLast(BuildOutputViewModel.EDITOR_WINDOW_MAX_CHARS) + } else { + visible + batch + } + } + + assertEquals(1, refreshCount) + assertTrue(visible.length <= BuildOutputViewModel.EDITOR_WINDOW_MAX_CHARS) + assertTrue(visible.endsWith(newest)) + } + + @Test + fun `filtered editor refreshes when hidden source output advances the window`() { + val oldMatch = "old match\n" + val hidden = "x".repeat(BuildOutputViewModel.EDITOR_WINDOW_MAX_CHARS) + var session = oldMatch + var sourceChars = session.length + var visible = oldMatch + + session += hidden + if (BuildOutputViewModel.wouldExceedEditorWindow(sourceChars, hidden.length)) { + val window = session.takeLast(BuildOutputViewModel.EDITOR_WINDOW_MAX_CHARS) + visible = + BuildOutputViewModel.filterLines( + window, + query = "match", + showTimestamps = true, + showDeltas = true, + ) + sourceChars = window.length + } + + assertEquals("", visible) + assertEquals(BuildOutputViewModel.EDITOR_WINDOW_MAX_CHARS, sourceChars) + } + + @Test + fun `refreshed tail applies filtering and timing visibility`() { + val prefix = BuildOutputViewModel.formatLinePrefix(1_722_000_000_000L, 42L) + val tail = prefix + "ignored\n" + prefix + "newest output\n" + + val visible = + BuildOutputViewModel.filterLines( + tail, + query = "newest", + showTimestamps = false, + showDeltas = false, + ) + + assertEquals("newest output\n", visible) + } +} From b258106d4086080462788e81cad9e1a116a38def Mon Sep 17 00:00:00 2001 From: Matt Van Horn Date: Wed, 2 Sep 2026 06:35:47 -0700 Subject: [PATCH 2/2] fix: address review findings on bounded build output F01: the window refresh had no hysteresis, so once the session file passed the threshold every subsequent batch did a full disk read, re-filter and setText. The source-char counter is now reset to a base value after each refresh instead of being left parked at the threshold. F05/F06: overflow dropped the newest input and could strand an oversized one entirely. Eviction now removes the oldest pending output first and retains the newest tail, and the omission marker is folded into a single leading entry that tracks both line and character counts. F07: session invalidation is owned by the ViewModel as a token (currentSessionToken / isCurrentSession) rather than a caller-supplied predicate, so a stale writer cannot outlive clear(). F08: fitEditorWindow could never truncate at any of its three call sites, since every caller passed an already-filtered window. Removed. F09: the omission marker pluralizes rather than rendering "1 build output lines omitted". Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01TeuFQUkLJAMt4KTLvSF7hh --- .../fragments/output/BuildOutputBuffer.kt | 93 +++--- .../fragments/output/BuildOutputFragment.kt | 313 ++++++++++-------- .../viewmodel/BuildOutputViewModel.kt | 36 +- .../fragments/output/BuildOutputBufferTest.kt | 125 +++---- 4 files changed, 313 insertions(+), 254 deletions(-) diff --git a/app/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputBuffer.kt b/app/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputBuffer.kt index 793f83aa58..934e92bd6e 100644 --- a/app/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputBuffer.kt +++ b/app/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputBuffer.kt @@ -22,8 +22,8 @@ import kotlinx.coroutines.channels.Channel /** * Thread-safe pending build output with fixed memory and batch budgets. * - * Inputs are indivisible: one input larger than [maxBatchChars] is emitted alone, while one larger - * than [maxPendingChars] is omitted. Other inputs are never split or reordered. + * Inputs are indivisible unless one exceeds [maxPendingChars], in which case only its newest tail + * is retained. Older pending output is evicted first, while batches preserve the retained order. */ internal class BuildOutputBuffer( private val maxPendingChars: Int = DEFAULT_MAX_PENDING_CHARS, @@ -31,20 +31,22 @@ internal class BuildOutputBuffer( ) { data class Batch( val text: String, - val sessionGeneration: Int, + val sessionToken: Int, + val sourceChars: Int, ) private sealed interface Entry { - val sessionGeneration: Int + val sessionToken: Int data class Text( val value: String, - override val sessionGeneration: Int, + override val sessionToken: Int, ) : Entry data class Omission( var lineCount: Long, - override val sessionGeneration: Int, + var sourceChars: Int, + override val sessionToken: Int, ) : Entry } @@ -52,7 +54,6 @@ internal class BuildOutputBuffer( private val available = Channel(Channel.CONFLATED) private val lock = Any() private var retainedChars = 0 - private var omission: Entry.Omission? = null internal val pendingChars: Int get() = synchronized(lock) { retainedChars } @@ -64,33 +65,35 @@ internal class BuildOutputBuffer( fun offer( text: String, - sessionGeneration: Int, + sessionToken: Int, ) { if (text.isEmpty()) return - val needsNewline = !text.endsWith('\n') - val normalizedLength = text.length.toLong() + if (needsNewline) 1 else 0 - val lineCount = text.count { it == '\n' }.toLong() + if (needsNewline) 1 else 0 + val normalized = if (text.endsWith('\n')) text else "$text\n" synchronized(lock) { - if ( - normalizedLength > maxPendingChars.toLong() || - normalizedLength > (maxPendingChars - retainedChars).toLong() - ) { - val existingOmission = omission - if ( - existingOmission?.sessionGeneration == sessionGeneration && - entries.lastOrNull() === existingOmission - ) { - existingOmission.lineCount += lineCount - } else { - val marker = Entry.Omission(lineCount, sessionGeneration) - omission = marker - entries.addLast(marker) - } - } else { - val normalized = if (needsNewline) "$text\n" else text - entries.addLast(Entry.Text(normalized, sessionGeneration)) - retainedChars += normalizedLength.toInt() + val existingOmission = entries.firstOrNull() as? Entry.Omission + if (existingOmission != null) entries.removeFirst() + + var retained = normalized + var omittedLines = existingOmission?.lineCount ?: 0 + var omittedChars = existingOmission?.sourceChars ?: 0 + if (retained.length > maxPendingChars) { + val droppedPrefix = retained.dropLast(maxPendingChars) + omittedLines += lineCount(droppedPrefix) + omittedChars = saturatedAdd(omittedChars, droppedPrefix.length) + retained = retained.takeLast(maxPendingChars) + } + while (retainedChars > maxPendingChars - retained.length) { + val evicted = entries.removeFirst() as Entry.Text + retainedChars -= evicted.value.length + omittedLines += lineCount(evicted.value) + omittedChars = saturatedAdd(omittedChars, evicted.value.length) } + + if (omittedLines > 0) { + entries.addFirst(Entry.Omission(omittedLines, omittedChars, sessionToken)) + } + entries.addLast(Entry.Text(retained, sessionToken)) + retainedChars += retained.length available.trySend(Unit) } } @@ -107,7 +110,6 @@ internal class BuildOutputBuffer( synchronized(lock) { entries.clear() retainedChars = 0 - omission = null while (available.tryReceive().isSuccess) { // Discard stale wakeups from the cleared build session. } @@ -116,11 +118,12 @@ internal class BuildOutputBuffer( private fun takeAvailableBatch(): Batch? { if (entries.isEmpty()) return null - val sessionGeneration = entries.first().sessionGeneration + val sessionToken = entries.first().sessionToken val batch = StringBuilder(minOf(retainedChars, maxBatchChars)) + var sourceChars = 0 while (entries.isNotEmpty()) { val entry = entries.first() - if (entry.sessionGeneration != sessionGeneration) break + if (entry.sessionToken != sessionToken) break val value = when (entry) { is Entry.Text -> entry.value @@ -130,14 +133,30 @@ internal class BuildOutputBuffer( entries.removeFirst() batch.append(value) - if (entry is Entry.Text) retainedChars -= entry.value.length - if (entry is Entry.Omission && omission === entry) omission = null + when (entry) { + is Entry.Text -> { + retainedChars -= entry.value.length + sourceChars = saturatedAdd(sourceChars, entry.value.length) + } + is Entry.Omission -> sourceChars = saturatedAdd(sourceChars, entry.sourceChars) + } } if (entries.isNotEmpty()) available.trySend(Unit) - return Batch(batch.toString(), sessionGeneration) + return Batch(batch.toString(), sessionToken, sourceChars) } - private fun omissionMarker(lineCount: Long): String = "[$lineCount build output lines omitted]\n" + private fun lineCount(text: String): Long = + text.count { it == '\n' }.toLong() + if (text.endsWith('\n')) 0 else 1 + + private fun saturatedAdd( + left: Int, + right: Int, + ): Int = (left.toLong() + right).coerceAtMost(Int.MAX_VALUE.toLong()).toInt() + + private fun omissionMarker(lineCount: Long): String { + val noun = if (lineCount == 1L) "line" else "lines" + return "[$lineCount build output $noun omitted]\n" + } companion object { private const val DEFAULT_MAX_PENDING_CHARS = 256 * 1024 diff --git a/app/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputFragment.kt b/app/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputFragment.kt index 497c42c91b..c0eb0a6506 100644 --- a/app/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputFragment.kt +++ b/app/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputFragment.kt @@ -36,6 +36,7 @@ import com.itsaky.androidide.utils.BasicBuildInfo import com.itsaky.androidide.utils.dpToPx import com.itsaky.androidide.utils.flashInfo import com.itsaky.androidide.viewmodel.BuildOutputViewModel +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.flow.collectLatest import kotlinx.coroutines.flow.combine @@ -52,10 +53,6 @@ class BuildOutputFragment : ViewOptionsOutputFragment { private val buildOutputViewModel: BuildOutputViewModel by activityViewModels() - companion object { - private const val LAYOUT_TIMEOUT_MS = 2000L - } - override val currentEditor: IDEEditor? get() = editor private val outputBuffer = BuildOutputBuffer() @@ -67,18 +64,15 @@ class BuildOutputFragment : // so a re-render never misses or duplicates a concurrently flushed batch. private val editorContentMutex = Mutex() - // Bumped only when a build session is cleared (new build) so live streaming logs - // are never dropped from the disk session file during filter re-renders. - @Volatile - private var sessionGeneration = 0 + // Keeps producer-side disk appends ordered and provides an atomic restore snapshot boundary. + private val appendMutex = Mutex() // Bumped on every wholesale content replacement (filtered re-render or clear) so an // in-flight batch flush drained before the replacement can detect it and drop itself. @Volatile private var editorContentGeneration = 0 - @Volatile - private var visibleEditorChars = 0 + // Written on Main and read by the background batch processor. @Volatile private var editorSourceChars = 0 private val noMatchTracker = FilterNoMatchTracker() @@ -98,11 +92,9 @@ class BuildOutputFragment : setupSearchLayout() viewLifecycleOwner.lifecycleScope.launch { - launch { restoreWindowFromViewModel() } - launch(Dispatchers.Default) { processLogs() } launch { - val content = withContext(Dispatchers.IO) { buildOutputViewModel.getWindowForEditor() } - buildOutputViewModel.setCachedSnapshot(content) + restoreWindowFromViewModel() + withContext(Dispatchers.Default) { processLogs() } } launch { combine( @@ -124,19 +116,23 @@ class BuildOutputFragment : showTimestamps: Boolean = buildOutputViewModel.showTimestamps.value, showDeltas: Boolean = buildOutputViewModel.showDeltas.value, ) { - editorContentMutex.withLock { - editorContentGeneration++ - val window = withContext(Dispatchers.IO) { buildOutputViewModel.getWindowForEditor() } - val filtered = - withContext(Dispatchers.Default) { - BuildOutputViewModel.fitEditorWindow( - BuildOutputViewModel.filterLines(window, query, showTimestamps, showDeltas), - ) - } + val renderGeneration = withContext(Dispatchers.Main) { - editor?.setText(filtered) - visibleEditorChars = filtered.length - editorSourceChars = window.length + editorContentGeneration++ + editorContentGeneration + } + val window = + snapshotEditorWindow() + val filtered = + withContext(Dispatchers.Default) { + BuildOutputViewModel.filterLines(window, query, showTimestamps, showDeltas) + } + withContext(Dispatchers.Main) { + editorContentMutex.withLock { + if (renderGeneration != editorContentGeneration) return@withLock + val editor = editor ?: return@withLock + editor.setText(filtered) + editorSourceChars = BuildOutputViewModel.editorSourceCharsAfterRefresh(window.length) val isSourceEmpty = window.isBlank() updateEmptyState(isSourceEmpty = isSourceEmpty, isFilterActive = isFilterActive) if (noMatchTracker.onRender(isSourceEmpty = isSourceEmpty, isFilteredEmpty = filtered.isBlank())) { @@ -296,23 +292,16 @@ class BuildOutputFragment : private suspend fun restoreWindowFromViewModel() = withContext(Dispatchers.Default) { - val window = withContext(Dispatchers.IO) { buildOutputViewModel.getWindowForEditor() } + val generationAtRestore = editorContentGeneration + val window = snapshotEditorWindow() val content = - BuildOutputViewModel.fitEditorWindow( - BuildOutputViewModel.filterLines( - window, - buildOutputViewModel.filterText.value, - buildOutputViewModel.showTimestamps.value, - buildOutputViewModel.showDeltas.value, - ), + BuildOutputViewModel.filterLines( + window, + buildOutputViewModel.filterText.value, + buildOutputViewModel.showTimestamps.value, + buildOutputViewModel.showDeltas.value, ) - val generationAtRestore = editorContentGeneration - val visibleCharsAtRestore = visibleEditorChars - val sourceCharsAtRestore = editorSourceChars - fun isRestoreCurrent() = - editorContentGeneration == generationAtRestore && - visibleEditorChars == visibleCharsAtRestore && - editorSourceChars == sourceCharsAtRestore + fun isRestoreCurrent() = editorContentGeneration == generationAtRestore val isSourceEmpty = window.isBlank() val isFilteredEmpty = content.isBlank() @@ -322,10 +311,12 @@ class BuildOutputFragment : if (!isSourceEmpty && isFilteredEmpty) { editorContentMutex.withLock { if (isRestoreCurrent()) { - editor?.setText("") - visibleEditorChars = 0 - editorSourceChars = window.length - onContentReplaced() + editor?.run { + setText("") + editorSourceChars = + BuildOutputViewModel.editorSourceCharsAfterRefresh(window.length) + onContentReplaced() + } } } } @@ -340,30 +331,22 @@ class BuildOutputFragment : } if (layoutCompleted != null) { editorContentMutex.withLock { - if (isRestoreCurrent()) { - editor.appendBatch(content) - visibleEditorChars += content.length - editorSourceChars = window.length + if (isRestoreCurrent() && editor.appendBatchIfReady(content)) { + editorSourceChars = + BuildOutputViewModel.editorSourceCharsAfterRefresh(window.length) updateEmptyState(isSourceEmpty = false, isFilterActive = isFilterActive) } } } else { - // Timeout: defer append until layout is ready so content is not lost - val job = - viewLifecycleOwner.lifecycleScope.launch(Dispatchers.Main) { - editor.run { - awaitLayout(onForceVisible = { updateEmptyState(isSourceEmpty = false, isFilterActive = isFilterActive) }) - editorContentMutex.withLock { - if (isRestoreCurrent()) { - appendBatch(content) - visibleEditorChars += content.length - editorSourceChars = window.length - updateEmptyState(isSourceEmpty = false, isFilterActive = isFilterActive) - } - } - } + // Layout timed out; keep waiting so the restored content is not lost. + editor.awaitLayout(onForceVisible = { updateEmptyState(isSourceEmpty = false, isFilterActive = isFilterActive) }) + editorContentMutex.withLock { + if (isRestoreCurrent() && editor.appendBatchIfReady(content)) { + editorSourceChars = + BuildOutputViewModel.editorSourceCharsAfterRefresh(window.length) + updateEmptyState(isSourceEmpty = false, isFilterActive = isFilterActive) } - job.join() + } } } } @@ -371,6 +354,8 @@ class BuildOutputFragment : override fun onDestroyView() { searchLayout = null filterBar = null + editorContentGeneration++ + editorSourceChars = 0 editor?.release() super.onDestroyView() } @@ -380,12 +365,8 @@ class BuildOutputFragment : // Avoid forcing the activityViewModels lazy init (which calls requireActivity()) // when the fragment is detached, otherwise an IllegalStateException is thrown. if (!isAdded || activity == null) return - // Invalidate in-flight flushes before deleting content, so a batch drained from the - // buffer earlier cannot re-seed the cleared session. - sessionGeneration++ outputBuffer.clear() editorContentGeneration++ - visibleEditorChars = 0 editorSourceChars = 0 noMatchTracker.reset() buildOutputViewModel.clear() @@ -405,11 +386,34 @@ class BuildOutputFragment : } fun appendOutput(output: String?) { - if (!output.isNullOrEmpty()) { - outputBuffer.offer(output, sessionGeneration) + val text = output ?: return + if (text.isEmpty()) return + val sessionToken = buildOutputViewModel.currentSessionToken + lifecycleScope.launch { + appendMutex.withLock { + val normalized = + withContext(Dispatchers.Default) { + if (text.endsWith('\n')) text else "$text\n" + } + if ( + buildOutputViewModel.append(normalized, sessionToken) && + buildOutputViewModel.isCurrentSession(sessionToken) + ) { + outputBuffer.offer(normalized, sessionToken) + } + } } } + private suspend fun snapshotEditorWindow(): String = + appendMutex.withLock { + val snapshot = withContext(Dispatchers.IO) { buildOutputViewModel.getWindowForEditor() } + // The snapshot already contains everything persisted before this boundary. + outputBuffer.clear() + buildOutputViewModel.setCachedSnapshot(snapshot) + snapshot + } + /** * Main log orchestrator: Consumes, Batches, and Dispatches. * @@ -419,107 +423,122 @@ class BuildOutputFragment : while (true) { val batch = outputBuffer.takeBatch() val editorGenAtDrain = editorContentGeneration - flushToEditor(batch.text, batch.sessionGeneration, editorGenAtDrain) + try { + flushToEditor( + batch.text, + batch.sourceChars, + batch.sessionToken, + editorGenAtDrain, + ) + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + log.error("Failed to flush a build output batch to the editor", e) + } } } /** * Performs the safe UI update on the Main Thread. * - * Appends to the session file on a background dispatcher before switching to Main. + * Applies output already persisted by the producer before switching to Main. * Uses [IDEEditor.awaitLayout] to guarantee the editor has physical dimensions (width > 0) * before attempting to insert text, preventing the Sora library's `ArrayIndexOutOfBoundsException`. */ private suspend fun flushToEditor( text: String, - sessionGen: Int, + sourceChars: Int, + sessionToken: Int, editorGen: Int, ) { - editorContentMutex.withLock { - // A clear (new build) after this batch was drained invalidates session append. - if (sessionGen != sessionGeneration) return - - buildOutputViewModel.append(text) { sessionGen == sessionGeneration } - if (sessionGen != sessionGeneration) return - val refreshEditorWindow = - BuildOutputViewModel.wouldExceedEditorWindow(editorSourceChars, text.length) - - // The session file always gets the full text; the editor only shows matching lines - val visibleText = - BuildOutputViewModel.filterLines( - text, - buildOutputViewModel.filterText.value, - buildOutputViewModel.showTimestamps.value, - buildOutputViewModel.showDeltas.value, - ) - if (visibleText.isEmpty() && !refreshEditorWindow) { - withContext(Dispatchers.Main) { - if (sessionGen == sessionGeneration) editorSourceChars += text.length - } - return - } - val refreshedWindow = - if (refreshEditorWindow) { - val window = withContext(Dispatchers.IO) { buildOutputViewModel.getWindowForEditor() } - withContext(Dispatchers.Default) { - Pair( - BuildOutputViewModel.fitEditorWindow( - BuildOutputViewModel.filterLines( - window, - buildOutputViewModel.filterText.value, - buildOutputViewModel.showTimestamps.value, - buildOutputViewModel.showDeltas.value, - ), - ), - window.length, - ) + if (!buildOutputViewModel.isCurrentSession(sessionToken)) return + val refreshEditorWindow = + BuildOutputViewModel.wouldExceedEditorWindow(editorSourceChars, sourceChars) + val visibleText = + BuildOutputViewModel.filterLines( + text, + buildOutputViewModel.filterText.value, + buildOutputViewModel.showTimestamps.value, + buildOutputViewModel.showDeltas.value, + ) + val refreshedWindow = + if (refreshEditorWindow) { + val window = + appendMutex.withLock { + val snapshot = buildOutputViewModel.getCachedContentSnapshot() + outputBuffer.clear() + snapshot } - } else { - null + withContext(Dispatchers.Default) { + Pair( + BuildOutputViewModel.filterLines( + window, + buildOutputViewModel.filterText.value, + buildOutputViewModel.showTimestamps.value, + buildOutputViewModel.showDeltas.value, + ), + window.length, + ) } + } else { + null + } - withContext(Dispatchers.Main) { + withContext(Dispatchers.Main) { + editorContentMutex.withLock { + if ( + editorGen != editorContentGeneration || + !buildOutputViewModel.isCurrentSession(sessionToken) + ) { + return@withLock + } + val editor = editor ?: return@withLock updateEmptyState(isSourceEmpty = false, isFilterActive = isFilterActive) - if (editorGen != editorContentGeneration) return@withContext if (refreshedWindow != null) { editorContentGeneration++ - editor?.setText(refreshedWindow.first) - visibleEditorChars = refreshedWindow.first.length - editorSourceChars = refreshedWindow.second + editor.setText(refreshedWindow.first) + editorSourceChars = + BuildOutputViewModel.editorSourceCharsAfterRefresh(refreshedWindow.second) onContentReplaced() - return@withContext + return@withLock } - editor?.run { - val layoutCompleted = - withTimeoutOrNull(LAYOUT_TIMEOUT_MS) { - awaitLayout(onForceVisible = { updateEmptyState(isSourceEmpty = false, isFilterActive = isFilterActive) }) - } - if (layoutCompleted != null) { - // clearOutput() or renderFiltered() may have run since the file append. - if (editorGen == editorContentGeneration) { - appendBatch(visibleText) - visibleEditorChars += visibleText.length - editorSourceChars += text.length - updateEmptyState(isSourceEmpty = false, isFilterActive = isFilterActive) - } - } else { - // Timeout: defer append until layout is ready (same as restoreWindowFromViewModel) - viewLifecycleOwner.lifecycleScope.launch(Dispatchers.Main) { - editor?.run { - awaitLayout(onForceVisible = { updateEmptyState(isSourceEmpty = false, isFilterActive = isFilterActive) }) - editorContentMutex.withLock { - if (editorGen == editorContentGeneration) { - appendBatch(visibleText) - visibleEditorChars += visibleText.length - editorSourceChars += text.length - updateEmptyState(isSourceEmpty = false, isFilterActive = isFilterActive) - } - } - } - } + if (visibleText.isEmpty()) { + editorSourceChars += sourceChars + return@withLock + } + + val layoutCompleted = + withTimeoutOrNull(LAYOUT_TIMEOUT_MS) { + editor.awaitLayout(onForceVisible = { updateEmptyState(isSourceEmpty = false, isFilterActive = isFilterActive) }) + } + if (layoutCompleted != null) { + if (editor.appendBatchIfReady(visibleText)) { + editorSourceChars += sourceChars + } + } else { + editor.awaitLayout(onForceVisible = { updateEmptyState(isSourceEmpty = false, isFilterActive = isFilterActive) }) + if ( + editorGen == editorContentGeneration && + buildOutputViewModel.isCurrentSession(sessionToken) && + editor.appendBatchIfReady(visibleText) + ) { + editorSourceChars += sourceChars + updateEmptyState(isSourceEmpty = false, isFilterActive = isFilterActive) } } } } } + + private fun IDEEditor.appendBatchIfReady(text: String): Boolean { + if (!isReadyToAppend) return false + val previousLength = this.text.length + appendBatch(text) + return this.text.length == previousLength + text.length + } + + companion object { + private const val LAYOUT_TIMEOUT_MS = 2000L + private val log = org.slf4j.LoggerFactory.getLogger(BuildOutputFragment::class.java) + } } diff --git a/app/src/main/java/com/itsaky/androidide/viewmodel/BuildOutputViewModel.kt b/app/src/main/java/com/itsaky/androidide/viewmodel/BuildOutputViewModel.kt index 6065a05afd..f3b89f21d4 100644 --- a/app/src/main/java/com/itsaky/androidide/viewmodel/BuildOutputViewModel.kt +++ b/app/src/main/java/com/itsaky/androidide/viewmodel/BuildOutputViewModel.kt @@ -47,6 +47,16 @@ class BuildOutputViewModel( ) : AndroidViewModel(application) { private val lock = ReentrantLock() + @Volatile + private var sessionGeneration = 0 + + /** Token for output produced by the current build session. */ + val currentSessionToken: Int + get() = sessionGeneration + + /** Returns whether [token] still belongs to the current build session. */ + fun isCurrentSession(token: Int): Boolean = token == sessionGeneration + /** * Case-insensitive line filter applied to the *editor view* of the build output. * The session file always receives the unfiltered text. @@ -88,24 +98,26 @@ class BuildOutputViewModel( /** * Appends text to the session file. File I/O is performed on a background dispatcher; call from - * any thread. Prefer calling before switching to Main so disk write does not block the UI. + * any thread. [sessionToken] values invalidated by [clear] are rejected inside the file lock. */ suspend fun append( text: String, - isCurrentSession: () -> Boolean = { true }, - ) { - if (text.isEmpty()) return - withContext(Dispatchers.IO) { + sessionToken: Int, + ): Boolean { + if (text.isEmpty()) return false + return withContext(Dispatchers.IO) { lock.withLock { - if (!isCurrentSession()) return@withLock + if (!isCurrentSession(sessionToken)) return@withLock false try { FileOutputStream(sessionFile, true).use { it.write(text.toByteArray(StandardCharsets.UTF_8)) } cachedContentSnapshot = (cachedContentSnapshot + text).takeLast(CACHE_SNAPSHOT_MAX_CHARS) + true } catch (e: Exception) { log.error("Failed to append build output to session file", e) + false } } } @@ -163,6 +175,7 @@ class BuildOutputViewModel( */ fun clear() { lock.withLock { + sessionGeneration++ cachedContentSnapshot = "" try { if (sessionFile.exists()) { @@ -199,18 +212,17 @@ class BuildOutputViewModel( companion object { internal const val EDITOR_WINDOW_MAX_CHARS = 512 * 1024 + private const val EDITOR_WINDOW_REFRESH_CHARS = 128 * 1024 + private const val EDITOR_WINDOW_REFRESH_BASE_CHARS = + EDITOR_WINDOW_MAX_CHARS - EDITOR_WINDOW_REFRESH_CHARS internal fun wouldExceedEditorWindow( currentChars: Int, incomingChars: Int, ): Boolean = currentChars > EDITOR_WINDOW_MAX_CHARS - incomingChars - internal fun fitEditorWindow(content: String): String = - if (content.length <= EDITOR_WINDOW_MAX_CHARS) { - content - } else { - content.takeLast(EDITOR_WINDOW_MAX_CHARS) - } + internal fun editorSourceCharsAfterRefresh(windowChars: Int): Int = + windowChars.coerceAtMost(EDITOR_WINDOW_REFRESH_BASE_CHARS) // Must mirror formatLinePrefix exactly; the round-trip is covered by BuildOutputFilterTest. // Anchored to line start so timestamp-shaped text inside a message is never stripped. diff --git a/app/src/test/java/com/itsaky/androidide/fragments/output/BuildOutputBufferTest.kt b/app/src/test/java/com/itsaky/androidide/fragments/output/BuildOutputBufferTest.kt index 8139d44caa..30ef509ab2 100644 --- a/app/src/test/java/com/itsaky/androidide/fragments/output/BuildOutputBufferTest.kt +++ b/app/src/test/java/com/itsaky/androidide/fragments/output/BuildOutputBufferTest.kt @@ -17,15 +17,19 @@ package com.itsaky.androidide.fragments.output +import android.app.Application +import androidx.test.core.app.ApplicationProvider +import com.google.common.truth.Truth.assertThat import com.itsaky.androidide.viewmodel.BuildOutputViewModel import kotlinx.coroutines.test.runTest -import org.junit.Assert.assertEquals -import org.junit.Assert.assertTrue import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +@RunWith(RobolectricTestRunner::class) class BuildOutputBufferTest { private fun BuildOutputBuffer.offer(text: String) { - offer(text, sessionGeneration = 0) + offer(text, sessionToken = 0) } @Test @@ -36,7 +40,7 @@ class BuildOutputBufferTest { buffer.offer("first") buffer.offer("second\n") - assertEquals("first\nsecond\n", buffer.takeBatch().text) + assertThat(buffer.takeBatch().text).isEqualTo("first\nsecond\n") } @Test @@ -50,10 +54,10 @@ class BuildOutputBufferTest { val first = buffer.takeBatch().text val second = buffer.takeBatch().text - assertEquals("aa\nbb\n", first) - assertEquals("cc\n", second) - assertTrue(first.length <= 6) - assertTrue(second.length <= 6) + assertThat(first).isEqualTo("aa\nbb\n") + assertThat(second).isEqualTo("cc\n") + assertThat(first.length).isAtMost(6) + assertThat(second.length).isAtMost(6) } @Test @@ -63,43 +67,36 @@ class BuildOutputBufferTest { buffer.offer("oversized") - assertEquals("oversized\n", buffer.takeBatch().text) + assertThat(buffer.takeBatch().text).isEqualTo("oversized\n") } @Test - fun `overflow is coalesced before retained output resumes`() = + fun `overflow evicts oldest output and keeps newest output`() = runTest { - val buffer = BuildOutputBuffer(maxPendingChars = 8, maxBatchChars = 128) + val buffer = BuildOutputBuffer(maxPendingChars = 11, maxBatchChars = 128) buffer.offer("one") buffer.offer("two") - buffer.offer("dropped one") - buffer.offer("dropped two\nand three") + buffer.offer("three") + buffer.offer("four") - assertEquals( - "one\ntwo\n[3 build output lines omitted]\n", - buffer.takeBatch().text, - ) - assertTrue(buffer.pendingChars <= 8) + val batch = buffer.takeBatch() + assertThat(batch.text).isEqualTo("[2 build output lines omitted]\nthree\nfour\n") + assertThat(batch.sourceChars).isEqualTo(19) + assertThat(buffer.pendingChars).isAtMost(11) } @Test - fun `overflow after resumed output starts a new omission marker`() = + fun `oversized input retains its newest bounded tail`() = runTest { - val buffer = BuildOutputBuffer(maxPendingChars = 8, maxBatchChars = 4) - - buffer.offer("one") - buffer.offer("two") - buffer.offer("first dropped") - assertEquals("one\n", buffer.takeBatch().text) + val buffer = BuildOutputBuffer(maxPendingChars = 8, maxBatchChars = 128) - buffer.offer("new") - buffer.offer("second dropped") + buffer.offer("0123456789") - assertEquals("two\n", buffer.takeBatch().text) - assertEquals("[1 build output lines omitted]\n", buffer.takeBatch().text) - assertEquals("new\n", buffer.takeBatch().text) - assertEquals("[1 build output lines omitted]\n", buffer.takeBatch().text) + val batch = buffer.takeBatch() + assertThat(batch.text).isEqualTo("[1 build output line omitted]\n3456789\n") + assertThat(batch.sourceChars).isEqualTo(11) + assertThat(buffer.pendingChars).isEqualTo(0) } @Test @@ -112,45 +109,57 @@ class BuildOutputBufferTest { buffer.clear() buffer.offer("new") - assertEquals("new\n", buffer.takeBatch().text) + assertThat(buffer.takeBatch().text).isEqualTo("new\n") } @Test - fun `in-flight batch keeps the session generation from its producer`() = + fun `in-flight batch keeps the session token from its producer`() = runTest { val buffer = BuildOutputBuffer(maxPendingChars = 64, maxBatchChars = 64) - buffer.offer("old", sessionGeneration = 3) + buffer.offer("old", sessionToken = 3) val inFlight = buffer.takeBatch() buffer.clear() - buffer.offer("new", sessionGeneration = 4) + buffer.offer("new", sessionToken = 4) - assertEquals(3, inFlight.sessionGeneration) - assertEquals(4, buffer.takeBatch().sessionGeneration) + assertThat(inFlight.sessionToken).isEqualTo(3) + assertThat(buffer.takeBatch().sessionToken).isEqualTo(4) } @Test - fun `repeated live batches refresh to the newest bounded editor tail`() { - val chunk = "x".repeat(200 * 1024) - val newest = "newest build output\n" - var session = "" - var visible = "" + fun `clear invalidates stale view model session tokens`() = + runTest { + val viewModel = + BuildOutputViewModel(ApplicationProvider.getApplicationContext()) + viewModel.clear() + val staleToken = viewModel.currentSessionToken + + assertThat(viewModel.append("old\n", staleToken)).isTrue() + viewModel.clear() + + assertThat(viewModel.append("stale\n", staleToken)).isFalse() + assertThat(viewModel.getFullContent()).isEmpty() + } + + @Test + fun `window refresh uses hysteresis after reaching the editor limit`() { + val batchChars = 32 * 1024 + var sourceChars = BuildOutputViewModel.EDITOR_WINDOW_MAX_CHARS var refreshCount = 0 - for (batch in listOf(chunk, chunk, chunk + newest)) { - session += batch - visible = - if (BuildOutputViewModel.wouldExceedEditorWindow(visible.length, batch.length)) { - refreshCount++ - session.takeLast(BuildOutputViewModel.EDITOR_WINDOW_MAX_CHARS) - } else { - visible + batch - } + repeat(6) { + if (BuildOutputViewModel.wouldExceedEditorWindow(sourceChars, batchChars)) { + refreshCount++ + sourceChars = + BuildOutputViewModel.editorSourceCharsAfterRefresh( + BuildOutputViewModel.EDITOR_WINDOW_MAX_CHARS, + ) + } else { + sourceChars += batchChars + } } - assertEquals(1, refreshCount) - assertTrue(visible.length <= BuildOutputViewModel.EDITOR_WINDOW_MAX_CHARS) - assertTrue(visible.endsWith(newest)) + assertThat(refreshCount).isEqualTo(2) } @Test @@ -171,11 +180,11 @@ class BuildOutputBufferTest { showTimestamps = true, showDeltas = true, ) - sourceChars = window.length + sourceChars = BuildOutputViewModel.editorSourceCharsAfterRefresh(window.length) } - assertEquals("", visible) - assertEquals(BuildOutputViewModel.EDITOR_WINDOW_MAX_CHARS, sourceChars) + assertThat(visible).isEmpty() + assertThat(sourceChars).isLessThan(BuildOutputViewModel.EDITOR_WINDOW_MAX_CHARS) } @Test @@ -191,6 +200,6 @@ class BuildOutputBufferTest { showDeltas = false, ) - assertEquals("newest output\n", visible) + assertThat(visible).isEqualTo("newest output\n") } }