From 51a2b24b24f3077e63b770034e2c69d40931dce9 Mon Sep 17 00:00:00 2001 From: David Schachter Date: Mon, 7 Sep 2026 01:38:58 -0700 Subject: [PATCH 1/6] ADFA-5526: copy a history into a destination, and retain fewer samples Two changes that pay for each other. copyInto writes a ring buffer into an array the caller already owns. toLongArray now delegates to it. Snapshotting the watchers otherwise takes eleven fresh arrays, and the next commit needs a caller that allocates nothing at all -- a crash handler must not ask for memory, because the crash it is reporting may be the heap running out. It requires an exact-length destination: a short one truncates the history and a long one leaves a stale tail behind it, and both read as data. MAX_USAGE_ENTRIES drops from 10,000 to 3,600. Ten thousand samples is nearly three hours at the default rate, of which the chart shows sixty at a time, and eleven buffers of it is 859KB held for the life of the process. At 3,600 the live buffers plus the pre-allocated destinations cost 619KB together -- less than the live buffers alone did before. Note what the constant is not: it counts samples, not time, so at the fastest offered rate of 100ms it is six minutes rather than an hour. If the fast rates are meant to give hours of history, this is the wrong number, and it is one constant. Also relaxes readUsages to internal so a test can drive one sample without starting the sampling loop, matching what ADFA-5514 does to it. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_017CCQUU7tBzZL61EmQhJP8j --- .../androidide/utils/MemoryUsageWatcher.kt | 44 ++++++++++- .../androidide/utils/NetworkUsageWatcher.kt | 37 +++++++-- .../androidide/utils/PowerUsageWatcher.kt | 33 ++++++-- .../androidide/utils/ShiftedLongArray.kt | 21 ++++- .../utils/ShiftedLongArrayCopyIntoTest.kt | 79 +++++++++++++++++++ 5 files changed, 197 insertions(+), 17 deletions(-) create mode 100644 app/src/test/java/com/itsaky/androidide/utils/ShiftedLongArrayCopyIntoTest.kt diff --git a/app/src/main/java/com/itsaky/androidide/utils/MemoryUsageWatcher.kt b/app/src/main/java/com/itsaky/androidide/utils/MemoryUsageWatcher.kt index eeb47d03ce..567d11d670 100644 --- a/app/src/main/java/com/itsaky/androidide/utils/MemoryUsageWatcher.kt +++ b/app/src/main/java/com/itsaky/androidide/utils/MemoryUsageWatcher.kt @@ -157,11 +157,18 @@ class MemoryUsageWatcher } /** - * Samples retained per series: nearly three hours at [DEFAULT_UPDATE_INTERVAL] (ADFA-5486). - * About 80KB of longs per series, so the cost is in drawing rather than holding -- - * see MetricsChartRenderer, which shows a window of this rather than all of it. + * Samples retained per series. + * + * An hour at [DEFAULT_UPDATE_INTERVAL], and the chart shows sixty of them at a time + * (ADFA-5486). It was 10,000, which is nearly three hours nobody was looking at -- and + * eleven buffers of that is 859KB held for the life of the process, doubled by the + * pre-allocated snapshot destinations [MetricsScratch] adds so a crash handler never has + * to allocate. At 3,600 the two together cost less than the one did (ADFA-5526). + * + * A count of samples, not a duration: at the fastest offered rate of 100ms it is six + * minutes rather than an hour. */ - const val MAX_USAGE_ENTRIES = 10000 + const val MAX_USAGE_ENTRIES = 3600 const val DEFAULT_UPDATE_INTERVAL = 1000L private val log = LoggerFactory.getLogger(MemoryUsageWatcher::class.java) } @@ -336,6 +343,32 @@ class MemoryUsageWatcher ) } + /** + * [history], into destinations the caller owns (ADFA-5526). + * + * Processes beyond the destinations given are dropped rather than allocated for -- the caller + * sized itself for [MetricsCsv.MEMORY_COLUMNS], which is every process the chart can plot. + */ + fun copyHistoryInto( + timesDest: LongArray, + destinations: List, + ): MemoryHistory = + synchronized(historyLock) { + MemoryHistory( + times = sampleTimes.copyInto(timesDest), + processes = + memoryUsage.values.take(destinations.size).mapIndexed { index, proc -> + ProcessHistory( + pid = proc.pid, + pname = proc.pname, + usage = proc._history.copyInto(destinations[index]), + watchedSinceMillis = proc.watchedSinceMillis, + ) + }, + ) + } + } + /** * Returns the memory usage of all the registered processes. */ @@ -412,6 +445,9 @@ class MemoryUsageWatcher /** * One process's retained samples, detached from the watcher. * + * Deliberately not [ProcessMemoryInfo], which carries a MemoryInfo and a ring buffer of its + * own and is what [getMemoryUsages] allocates. + * * @property usage The samples, oldest first, in bytes. * @property watchedSinceMillis When this process started being watched. Its buffer reaches * back to the start of the session however late in it the process appeared, and this is what diff --git a/app/src/main/java/com/itsaky/androidide/utils/NetworkUsageWatcher.kt b/app/src/main/java/com/itsaky/androidide/utils/NetworkUsageWatcher.kt index 2166a87d74..0194ee904a 100644 --- a/app/src/main/java/com/itsaky/androidide/utils/NetworkUsageWatcher.kt +++ b/app/src/main/java/com/itsaky/androidide/utils/NetworkUsageWatcher.kt @@ -155,8 +155,27 @@ class NetworkUsageWatcher * the sampler thread is midway through appending, and the chart renderer reads all 30 entries. */ fun getUsage(): NetworkUsage = + copyUsageInto(LongArray(received.size), LongArray(transmitted.size), LongArray(sampleTimes.size)) + + /** + * [getUsage], into destinations the caller owns (ADFA-5526). + * + * The times come back with the values because they are read in the same critical section: + * asking separately let a sample land between the calls and shifted every value one index + * against its timestamp (ADFA-5531). + */ + fun copyUsageInto( + receivedDest: LongArray, + transmittedDest: LongArray, + timesDest: LongArray, + ): NetworkUsage = synchronized(historyLock) { - NetworkUsage(received.toLongArray(), transmitted.toLongArray(), sampleTimes.toLongArray()) + NetworkUsage( + received.copyInto(receivedDest), + transmitted.copyInto(transmittedDest), + sampleTimes.copyInto(timesDest), + ) + } } /** @@ -356,12 +375,18 @@ class NetworkUsageWatcher companion object { /** - * Samples retained per series (ADFA-5486). The span this covers depends on the interval: - * under three hours at one second, about seventeen minutes at the 0.1s minimum. 80KB of - * longs per series, so the cost is in drawing rather than holding -- see - * MetricsChartRenderer, which shows a window of this rather than all of it. + * Samples retained per series. + * + * An hour at [DEFAULT_UPDATE_INTERVAL], and the chart shows sixty of them at a time + * (ADFA-5486). It was 10,000, which is nearly three hours nobody was looking at -- and + * eleven buffers of that is 859KB held for the life of the process, doubled by the + * pre-allocated snapshot destinations [MetricsScratch] adds so a crash handler never has + * to allocate. At 3,600 the two together cost less than the one did (ADFA-5526). + * + * A count of samples, not a duration: at the fastest offered rate of 100ms it is six + * minutes rather than an hour. */ - const val MAX_USAGE_ENTRIES = 10000 + const val MAX_USAGE_ENTRIES = 3600 const val DEFAULT_UPDATE_INTERVAL = 1000L /** [TrafficStats.UNSUPPORTED] widened to [Long], which is what the getters return. */ diff --git a/app/src/main/java/com/itsaky/androidide/utils/PowerUsageWatcher.kt b/app/src/main/java/com/itsaky/androidide/utils/PowerUsageWatcher.kt index 3c38817837..4d94b102a7 100644 --- a/app/src/main/java/com/itsaky/androidide/utils/PowerUsageWatcher.kt +++ b/app/src/main/java/com/itsaky/androidide/utils/PowerUsageWatcher.kt @@ -140,14 +140,35 @@ class PowerUsageWatcher * live ring buffers would let a reader see them mid-append. */ fun getUsage(): PowerUsage = + copyUsageInto( + LongArray(temperature.size), + LongArray(power.size), + LongArray(thermal.size), + LongArray(sampleTimes.size), + ) + + /** + * [getUsage], into destinations the caller owns (ADFA-5526). + * + * The times come back with the values because they are read in the same critical section: + * asking separately let a sample land between the calls and shifted every value one index + * against its timestamp (ADFA-5531). + */ + fun copyUsageInto( + temperatureDest: LongArray, + powerDest: LongArray, + thermalDest: LongArray, + timesDest: LongArray, + ): PowerUsage = synchronized(historyLock) { PowerUsage( - temperature.toLongArray(), - power.toLongArray(), - thermal.toLongArray(), - sampleTimes.toLongArray(), + temperature.copyInto(temperatureDest), + power.copyInto(powerDest), + thermal.copyInto(thermalDest), + sampleTimes.copyInto(timesDest), ) } + } fun clearHistory() { synchronized(historyLock) { @@ -316,8 +337,8 @@ class PowerUsageWatcher } companion object { - /** Samples retained per series, matching the other watchers. */ - const val MAX_USAGE_ENTRIES = 10000 + /** Samples retained per series, matching the other watchers (ADFA-5526). */ + const val MAX_USAGE_ENTRIES = 3600 const val DEFAULT_UPDATE_INTERVAL = 1000L /** A reading the device does not provide. */ diff --git a/app/src/main/java/com/itsaky/androidide/utils/ShiftedLongArray.kt b/app/src/main/java/com/itsaky/androidide/utils/ShiftedLongArray.kt index 6392e21f7c..36cf8ba013 100644 --- a/app/src/main/java/com/itsaky/androidide/utils/ShiftedLongArray.kt +++ b/app/src/main/java/com/itsaky/androidide/utils/ShiftedLongArray.kt @@ -134,4 +134,23 @@ open class ShiftedLongArray( * Shared so the watchers' snapshots cannot drift from [ShiftedLongArray]'s shift semantics; each * of them had its own private copy of this one line. */ -internal fun ShiftedLongArray.toLongArray(): LongArray = LongArray(size) { this[it] } +internal fun ShiftedLongArray.toLongArray(): LongArray = copyInto(LongArray(size)) + +/** + * Copies this ring buffer into [dest] in logical order, oldest first, and returns it. + * + * For a caller that owns its destination already. A crash handler must not allocate -- the crash it + * is reporting may be the heap running out -- so ADFA-5526 pre-allocates one set of destinations at + * startup and fills them here instead of taking eleven fresh arrays per snapshot. + * + * @throws IllegalArgumentException when [dest] is not exactly this buffer's length. A short + * destination would silently truncate the history and a long one would leave a stale tail behind + * it, and both read as data. + */ +internal fun ShiftedLongArray.copyInto(dest: LongArray): LongArray { + require(dest.size == size) { "Destination is ${dest.size} long, buffer is $size" } + for (i in 0 until size) { + dest[i] = this[i] + } + return dest +} diff --git a/app/src/test/java/com/itsaky/androidide/utils/ShiftedLongArrayCopyIntoTest.kt b/app/src/test/java/com/itsaky/androidide/utils/ShiftedLongArrayCopyIntoTest.kt new file mode 100644 index 0000000000..2ba1d776cc --- /dev/null +++ b/app/src/test/java/com/itsaky/androidide/utils/ShiftedLongArrayCopyIntoTest.kt @@ -0,0 +1,79 @@ +/* + * 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.utils + +import com.google.common.truth.Truth.assertThat +import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.JUnit4 + +/** Copying a ring buffer into a destination the caller owns (ADFA-5526). */ +@RunWith(JUnit4::class) +class ShiftedLongArrayCopyIntoTest { + private fun buffer(): MutableShiftedLongArray { + val buffer = MutableShiftedLongArray(4) + // Appended the way the watchers append: newest in at 0, then shift. + listOf(10L, 20L, 30L).forEach { value -> + buffer[0] = value + buffer.shift(1) + } + return buffer + } + + @Test + fun `it writes the same order toLongArray produces`() { + val buffer = buffer() + + val dest = LongArray(buffer.size) + assertThat(buffer.copyInto(dest).toList()).isEqualTo(buffer.toLongArray().toList()) + } + + @Test + fun `it returns the destination it was given, not a copy`() { + val buffer = buffer() + val dest = LongArray(buffer.size) + + // The whole point: the caller pre-allocated this, so nothing new may be handed back. + assertThat(buffer.copyInto(dest)).isSameInstanceAs(dest) + } + + @Test + fun `a destination of the wrong length is refused`() { + val buffer = buffer() + + // A short destination truncates the history and a long one leaves a stale tail behind it, + // and both read as data. Better to fail where the mistake is than to file a wrong graph. + listOf(LongArray(buffer.size - 1), LongArray(buffer.size + 1)).forEach { wrong -> + val failure = runCatching { buffer.copyInto(wrong) }.exceptionOrNull() + assertThat(failure).isInstanceOf(IllegalArgumentException::class.java) + } + } + + @Test + fun `a second copy overwrites the first, leaving nothing of it`() { + val dest = LongArray(4) + buffer().copyInto(dest) + + val fresh = MutableShiftedLongArray(4) + fresh.copyInto(dest) + + // The scratch is reused across snapshots, so a stale value surviving into the next one + // would be reported as a measurement. + assertThat(dest.toList()).containsExactly(0L, 0L, 0L, 0L) + } +} From e4c43d8be7f9a677638bc80f2c7f34919da3fa6f Mon Sep 17 00:00:00 2001 From: David Schachter Date: Mon, 7 Sep 2026 01:39:23 -0700 Subject: [PATCH 2/6] ADFA-5526: pre-allocate the snapshot destinations, and make them reachable A crash handler is the wrong place to ask for memory. The crash being reported may be the heap running out, and a handler that throws replaces a useful report with no report. So the eleven destinations a snapshot needs are taken at startup, where failing to get them is survivable and obvious, and filled rather than allocated when a snapshot is taken. MetricsScratch is single-use at a time rather than thread-confined: claim hands it to one caller and release gives it back. A caller that cannot claim it allocates for itself instead of waiting, because two writers into one array is a scrambled file and a crash must not block on an export. withSnapshot scopes the claim to a block, because the snapshot points into the scratch and so the scratch has to outlive whatever reads it -- which is a file write, not the call that assembled it. MetricsSource is the other half: the watchers live in an activity-scoped ViewModel, which is right for the carousel and no use to a crash handler that has no activity and arrives on whatever thread threw. The ViewModel registers itself and clears on onCleared, conditionally, since an activity recreation can register the replacement before the outgoing one is cleared. The assembler's @UiThread comes off. Every read it makes takes the watcher's own history lock, so it was always safe from any thread; the annotation was conservative rather than load-bearing, and a crash does not get to choose its thread. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_017CCQUU7tBzZL61EmQhJP8j --- .../itsaky/androidide/utils/MetricsScratch.kt | 95 +++++++++++++++++++ .../utils/MetricsSnapshotAssembler.kt | 67 +++++++++++-- .../itsaky/androidide/utils/MetricsSource.kt | 59 ++++++++++++ .../androidide/viewmodel/MetricsViewModel.kt | 18 +++- .../androidide/utils/MetricsScratchTest.kt | 92 ++++++++++++++++++ 5 files changed, 319 insertions(+), 12 deletions(-) create mode 100644 app/src/main/java/com/itsaky/androidide/utils/MetricsScratch.kt create mode 100644 app/src/main/java/com/itsaky/androidide/utils/MetricsSource.kt create mode 100644 app/src/test/java/com/itsaky/androidide/utils/MetricsScratchTest.kt diff --git a/app/src/main/java/com/itsaky/androidide/utils/MetricsScratch.kt b/app/src/main/java/com/itsaky/androidide/utils/MetricsScratch.kt new file mode 100644 index 0000000000..df7a145159 --- /dev/null +++ b/app/src/main/java/com/itsaky/androidide/utils/MetricsScratch.kt @@ -0,0 +1,95 @@ +/* + * 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.utils + +import androidx.annotation.VisibleForTesting +import java.util.concurrent.atomic.AtomicBoolean + +/** + * Destinations for one metrics snapshot, allocated once so that taking one needs no memory. + * + * A crash handler is the wrong place to ask for memory: the crash being reported may be the heap + * running out, and a handler that throws replaces a useful report with a useless one. Snapshotting + * the watchers otherwise takes eleven fresh arrays -- around 300KB at the retained length -- so the + * arrays are taken at startup instead, when failing to get them is survivable and obvious. + * + * Held for the life of the process, which is the trade: this is memory reserved against a crash that + * may never come, in a process that is already a fat target for the low-memory killer. It is paid + * for by [MemoryUsageWatcher.MAX_USAGE_ENTRIES] coming down at the same time -- the live buffers plus + * these cost less than the live buffers alone did before (ADFA-5526). + * + * Not thread-confined but single-use at a time: [claim] hands it to one caller and [release] gives it + * back. A caller that cannot claim it allocates for itself rather than waiting or sharing, because + * two writers into one array is a scrambled file and a crash must not block on an export. + */ +class MetricsScratch( + @VisibleForTesting internal val entries: Int, + memorySeries: Int, +) { + private val inUse = AtomicBoolean(false) + + val memoryTimes = LongArray(entries) + val memoryValues: List = List(memorySeries) { LongArray(entries) } + val networkTimes = LongArray(entries) + val networkReceived = LongArray(entries) + val networkTransmitted = LongArray(entries) + val powerTimes = LongArray(entries) + val temperature = LongArray(entries) + val power = LongArray(entries) + val thermal = LongArray(entries) + + /** Takes this scratch, or returns false if something else already has it. */ + fun claim(): Boolean = inUse.compareAndSet(false, true) + + fun release() { + inUse.set(false) + } + + companion object { + /** + * The process-wide scratch, or `null` before [install] or if it could not be allocated. + * + * A crash arrives on whatever thread threw, from anywhere in the process, so this cannot + * live on an activity-scoped ViewModel the way the watchers do. + */ + @Volatile + var instance: MetricsScratch? = null + private set + + /** + * Allocates the process-wide scratch. Call once, from application startup. + * + * Failure is not fatal and not worth retrying: the crash path simply allocates for itself, + * which is what it did before this existed. + */ + fun install( + entries: Int = MemoryUsageWatcher.MAX_USAGE_ENTRIES, + memorySeries: Int = MetricsCsv.MEMORY_COLUMNS.size, + ) { + if (instance != null) { + return + } + instance = runCatching { MetricsScratch(entries, memorySeries) }.getOrNull() + } + + @VisibleForTesting + internal fun resetForTesting() { + instance = null + } + } +} diff --git a/app/src/main/java/com/itsaky/androidide/utils/MetricsSnapshotAssembler.kt b/app/src/main/java/com/itsaky/androidide/utils/MetricsSnapshotAssembler.kt index 31c50a0bcf..9d37c36566 100644 --- a/app/src/main/java/com/itsaky/androidide/utils/MetricsSnapshotAssembler.kt +++ b/app/src/main/java/com/itsaky/androidide/utils/MetricsSnapshotAssembler.kt @@ -19,7 +19,7 @@ package com.itsaky.androidide.utils import android.content.Context import android.os.SystemClock -import androidx.annotation.UiThread +import androidx.annotation.AnyThread /** * Reads the watchers' buffers into a [MetricsCsv.Snapshot]. @@ -29,7 +29,9 @@ import androidx.annotation.UiThread * the file: the feedback FAB can be tapped with the strip closed (ADFA-5534), and a crash report is * assembled with no UI at all (ADFA-5526). * - * All of it must be read on the UI thread; formatting and writing must not be. + * Every read here takes the watcher's own history lock, so this is safe from any thread -- which it + * has to be, because a crash arrives on whatever thread threw (ADFA-5526). Formatting and writing + * are a different matter and must stay off the main thread. */ object MetricsSnapshotAssembler { /** @@ -44,21 +46,72 @@ object MetricsSnapshotAssembler { * @param context resolves an annotation's label, which a build outcome carries as a string id * so its marker follows the system language. */ - @UiThread + @AnyThread fun assemble( context: Context, memory: MemoryUsageWatcher, network: NetworkUsageWatcher, power: PowerUsageWatcher, annotations: MetricsAnnotationStore?, + ): MetricsCsv.Snapshot = assemble(context, memory, network, power, annotations, scratch = null) + + /** + * Assembles a snapshot, hands it to [block], and only then gives the scratch back. + * + * The snapshot points *into* the scratch, so the scratch cannot be released when this returns -- + * it has to outlive whatever reads the snapshot, which is a file write. Scoping it to a block is + * how that is made hard to get wrong. + * + * Falls back to allocating when the scratch is already taken. A crash must not wait on an export, + * and two writers into one array is a scrambled file. + */ + @AnyThread + fun withSnapshot( + context: Context, + memory: MemoryUsageWatcher, + network: NetworkUsageWatcher, + power: PowerUsageWatcher, + annotations: MetricsAnnotationStore?, + block: (MetricsCsv.Snapshot) -> T, + ): T { + val scratch = MetricsScratch.instance?.takeIf { it.claim() } + return try { + block(assemble(context, memory, network, power, annotations, scratch)) + } finally { + scratch?.release() + } + } + + private fun assemble( + context: Context, + memory: MemoryUsageWatcher, + network: NetworkUsageWatcher, + power: PowerUsageWatcher, + annotations: MetricsAnnotationStore?, + scratch: MetricsScratch?, ): MetricsCsv.Snapshot { // One call per watcher, not one per array. Each hands back its times and its values from a // single critical section, which is what keeps a row of the file a single moment: asking // separately let a sample land between the two calls, and every value came out one row off - // its own timestamp. - val memoryHistory = memory.history() - val networkUsage = network.getUsage() - val powerUsage = power.getUsage() + // its own timestamp (ADFA-5531). + val memoryHistory = + if (scratch == null) { + memory.history() + } else { + memory.copyHistoryInto(scratch.memoryTimes, scratch.memoryValues) + } + val networkUsage = + if (scratch == null) { + network.getUsage() + } else { + network.copyUsageInto(scratch.networkReceived, scratch.networkTransmitted, scratch.networkTimes) + } + val powerUsage = + if (scratch == null) { + power.getUsage() + } else { + power.copyUsageInto(scratch.temperature, scratch.power, scratch.thermal, scratch.powerTimes) + } return MetricsCsv.Snapshot( rowTimes = memoryHistory.times, diff --git a/app/src/main/java/com/itsaky/androidide/utils/MetricsSource.kt b/app/src/main/java/com/itsaky/androidide/utils/MetricsSource.kt new file mode 100644 index 0000000000..d84e9e19f3 --- /dev/null +++ b/app/src/main/java/com/itsaky/androidide/utils/MetricsSource.kt @@ -0,0 +1,59 @@ +/* + * 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.utils + +/** + * Where a process-wide caller finds the live metrics watchers (ADFA-5526). + * + * The watchers belong to an activity-scoped ViewModel, which is right for the carousel and no use to + * a crash handler: a crash arrives on any thread, from anywhere, with no activity in hand. This is + * the one indirection that lets the handler reach them. + * + * Deliberately thin, and deliberately nullable. There is no source before the editor has run -- a + * crash during onboarding, in the project chooser, or in direct boot has no history to report -- and + * a caller that cannot find one attaches nothing rather than inventing something. + */ +object MetricsSource { + /** What a crash handler needs to build a snapshot. */ + interface Metrics { + val memoryUsageWatcher: MemoryUsageWatcher + val networkUsageWatcher: NetworkUsageWatcher + val powerUsageWatcher: PowerUsageWatcher + val annotations: MetricsAnnotationStore + } + + @Volatile + var current: Metrics? = null + private set + + fun register(metrics: Metrics) { + current = metrics + } + + /** + * Clears [current] if [metrics] is still the registered one. + * + * Conditional because an activity recreation can register the replacement before the outgoing + * one is cleared, and an unconditional clear would then drop the live source. + */ + fun unregister(metrics: Metrics) { + if (current === metrics) { + current = null + } + } +} diff --git a/app/src/main/java/com/itsaky/androidide/viewmodel/MetricsViewModel.kt b/app/src/main/java/com/itsaky/androidide/viewmodel/MetricsViewModel.kt index 2400fe4cc5..3a511114d5 100644 --- a/app/src/main/java/com/itsaky/androidide/viewmodel/MetricsViewModel.kt +++ b/app/src/main/java/com/itsaky/androidide/viewmodel/MetricsViewModel.kt @@ -22,6 +22,7 @@ import androidx.lifecycle.AndroidViewModel import com.itsaky.androidide.utils.DevicePowerSource import com.itsaky.androidide.utils.MemoryUsageWatcher import com.itsaky.androidide.utils.MetricsAnnotationStore +import com.itsaky.androidide.utils.MetricsSource import com.itsaky.androidide.utils.NetworkUsageWatcher import com.itsaky.androidide.utils.PowerUsageWatcher @@ -39,22 +40,29 @@ import com.itsaky.androidide.utils.PowerUsageWatcher */ class MetricsViewModel( application: Application, -) : AndroidViewModel(application) { - val memoryUsageWatcher = MemoryUsageWatcher() +) : AndroidViewModel(application), + MetricsSource.Metrics { + override val memoryUsageWatcher = MemoryUsageWatcher() - val networkUsageWatcher = NetworkUsageWatcher() + override val networkUsageWatcher = NetworkUsageWatcher() /** * Temperature and power (ADFA-5499). Needs a Context for the battery broadcast, which is why * this is an AndroidViewModel. */ - val powerUsageWatcher = PowerUsageWatcher(source = DevicePowerSource(application)) + override val powerUsageWatcher = PowerUsageWatcher(source = DevicePowerSource(application)) /** Significant events for the charts to annotate (ADFA-5486). */ - val annotations = MetricsAnnotationStore() + override val annotations = MetricsAnnotationStore() + + init { + // So a crash handler can reach the history (ADFA-5526). It has no activity to ask. + MetricsSource.register(this) + } override fun onCleared() { super.onCleared() + MetricsSource.unregister(this) // close(), not stopWatching(): this is the terminal teardown, and each watcher holds a // dedicated sampling thread that newSingleThreadContext keeps alive until it is closed. memoryUsageWatcher.close() diff --git a/app/src/test/java/com/itsaky/androidide/utils/MetricsScratchTest.kt b/app/src/test/java/com/itsaky/androidide/utils/MetricsScratchTest.kt new file mode 100644 index 0000000000..9bbbed0a13 --- /dev/null +++ b/app/src/test/java/com/itsaky/androidide/utils/MetricsScratchTest.kt @@ -0,0 +1,92 @@ +/* + * 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.utils + +import com.google.common.truth.Truth.assertThat +import org.junit.After +import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.JUnit4 + +/** + * The destinations a crash handler snapshots into, so that it never has to allocate (ADFA-5526). + */ +@RunWith(JUnit4::class) +class MetricsScratchTest { + @After + fun tearDown() = MetricsScratch.resetForTesting() + + @Test + fun `one claim at a time`() { + val scratch = MetricsScratch(entries = 4, memorySeries = 2) + + assertThat(scratch.claim()).isTrue() + // Two writers into one array is a scrambled file, so the second caller is refused and + // allocates for itself rather than waiting -- a crash must not block on an export. + assertThat(scratch.claim()).isFalse() + + scratch.release() + assertThat(scratch.claim()).isTrue() + } + + @Test + fun `every destination is the retained length`() { + val scratch = MetricsScratch(entries = 7, memorySeries = 3) + + // copyInto requires an exact-length destination, so a mismatch here is a crash-time failure. + val all = + listOf( + scratch.memoryTimes, + scratch.networkTimes, + scratch.networkReceived, + scratch.networkTransmitted, + scratch.powerTimes, + scratch.temperature, + scratch.power, + scratch.thermal, + ) + scratch.memoryValues + all.forEach { assertThat(it.size).isEqualTo(7) } + assertThat(scratch.memoryValues).hasSize(3) + } + + @Test + fun `installing is idempotent, so a second call keeps the first arrays`() { + MetricsScratch.install(entries = 4, memorySeries = 1) + val first = MetricsScratch.instance + + MetricsScratch.install(entries = 99, memorySeries = 1) + + // Replacing it would hand a second set of destinations to whoever already held the first. + assertThat(MetricsScratch.instance).isSameInstanceAs(first) + assertThat(MetricsScratch.instance!!.entries).isEqualTo(4) + } + + @Test + fun `there is no scratch until it is installed`() { + assertThat(MetricsScratch.instance).isNull() + } + + @Test + fun `the default size matches the retained history`() { + MetricsScratch.install() + + // If these drift apart, copyInto throws at crash time -- exactly when nothing may throw. + assertThat(MetricsScratch.instance!!.entries).isEqualTo(MemoryUsageWatcher.MAX_USAGE_ENTRIES) + assertThat(MetricsScratch.instance!!.memoryValues).hasSize(MetricsCsv.MEMORY_COLUMNS.size) + } +} From 98141c73a25fa4e01a03c4b347afd1356517fa45 Mon Sep 17 00:00:00 2001 From: David Schachter Date: Mon, 7 Sep 2026 01:39:23 -0700 Subject: [PATCH 3/6] ADFA-5526: attach the metrics to every report A crash arrives with a stack and no idea what the machine was doing. The minutes of memory, network, temperature and power before it are what turn "it died" into a diagnosis, and for an out-of-memory kill they are most of the answer. Registered as a Sentry EventProcessor beside GlitchTipDiagnosticsContext rather than on the uncaught exception handler, so it also covers the non-fatal captureException calls the IDE makes deliberately, and events raised from anywhere rather than only uncaught throws. Everything is inside runCatching, which in Kotlin catches Throwable. That is deliberate: this runs while the process is dying, and an OutOfMemoryError raised in here would cost the whole report rather than just the attachment. Losing the attachment is the right way to fail. Attaches nothing when the editor has never run -- onboarding, the project chooser and direct boot have no history, and direct boot has no credential-protected cache to write to either -- and nothing when nothing has been sampled, since a header-only file on every early crash would be noise rather than context. This does not need ADFA-5494, despite having been blocked on it. A crash is the one loss cause with a hookable moment, which is why it can be served alone; the low-memory kill 5494 exists for produces no report at all, because nothing runs on a SIGKILL. Verified by unit test: an attachment appears with history and gunzips to a valid CSV, nothing is attached without a source or without samples, and the event comes back unchanged when the attachment throws. NOT verified on device: `am crash` aborts the WebView renderer natively, which never reaches the Java uncaught handler, and I did not find another reachable path that raises a reported Java exception in the main process. The device leg of this is open. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_017CCQUU7tBzZL61EmQhJP8j --- .../app/DeviceProtectedApplicationLoader.kt | 8 ++ .../handlers/MetricsCrashAttachment.kt | 96 +++++++++++++ .../handlers/MetricsCrashAttachmentTest.kt | 135 ++++++++++++++++++ 3 files changed, 239 insertions(+) create mode 100644 app/src/main/java/com/itsaky/androidide/handlers/MetricsCrashAttachment.kt create mode 100644 app/src/test/java/com/itsaky/androidide/handlers/MetricsCrashAttachmentTest.kt diff --git a/app/src/main/java/com/itsaky/androidide/app/DeviceProtectedApplicationLoader.kt b/app/src/main/java/com/itsaky/androidide/app/DeviceProtectedApplicationLoader.kt index a5a1ed921c..96c4430352 100644 --- a/app/src/main/java/com/itsaky/androidide/app/DeviceProtectedApplicationLoader.kt +++ b/app/src/main/java/com/itsaky/androidide/app/DeviceProtectedApplicationLoader.kt @@ -18,6 +18,7 @@ import com.itsaky.androidide.events.LspJavaEventsIndex import com.itsaky.androidide.events.ProjectsApiEventsIndex import com.itsaky.androidide.handlers.CrashEventSubscriber import com.itsaky.androidide.handlers.GlitchTipDiagnosticsContext +import com.itsaky.androidide.handlers.MetricsCrashAttachment import com.itsaky.androidide.logging.provider.IdeLogRouter import com.itsaky.androidide.preferences.internal.StatPreferences import com.itsaky.androidide.preferences.internal.TelemetryConsent @@ -25,6 +26,7 @@ import com.itsaky.androidide.syntax.colorschemes.SchemeAndroidIDE import com.itsaky.androidide.ui.themes.IThemeManager import com.itsaky.androidide.utils.Environment import com.itsaky.androidide.utils.FeatureFlags +import com.itsaky.androidide.utils.MetricsScratch import com.termux.shared.reflection.ReflectionUtils import io.github.rosemoe.sora.widget.schemes.EditorColorScheme import io.sentry.Breadcrumb @@ -126,6 +128,12 @@ internal object DeviceProtectedApplicationLoader : // Enrich every GlitchTip event with app-specific diagnostic context. GlitchTipDiagnosticsContext.install(options) + + // And with what the machine was doing in the minutes before it (ADFA-5526). The + // destinations that snapshot writes into are taken now, while failing to get them is + // survivable -- a crash handler is the wrong place to ask for memory. + MetricsScratch.install() + MetricsCrashAttachment.install(options, app) } // Forward INFO+ logs to GlitchTip as breadcrumbs (never as events; crash events are diff --git a/app/src/main/java/com/itsaky/androidide/handlers/MetricsCrashAttachment.kt b/app/src/main/java/com/itsaky/androidide/handlers/MetricsCrashAttachment.kt new file mode 100644 index 0000000000..153b953947 --- /dev/null +++ b/app/src/main/java/com/itsaky/androidide/handlers/MetricsCrashAttachment.kt @@ -0,0 +1,96 @@ +/* + * 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.handlers + +import android.content.Context +import com.itsaky.androidide.utils.MetricsCsvFile +import com.itsaky.androidide.utils.MetricsSnapshotAssembler +import com.itsaky.androidide.utils.MetricsSource +import io.sentry.Attachment +import io.sentry.EventProcessor +import io.sentry.Hint +import io.sentry.SentryEvent +import io.sentry.SentryOptions +import org.slf4j.LoggerFactory +import java.io.File + +/** + * Attaches the carousel's metrics to every report the IDE sends (ADFA-5526). + * + * A crash arrives with a stack and no idea what the machine was doing. The minutes of memory, + * network, temperature and power leading up to it are what turn "it died" into a diagnosis -- and + * for an out-of-memory kill they are most of the answer. + * + * Registered as a Sentry [EventProcessor] beside [GlitchTipDiagnosticsContext], not on the uncaught + * exception handler, so it also covers the non-fatal `Sentry.captureException` calls the IDE makes + * deliberately. + * + * ADFA-5494 will keep this history across process death; this does not need it. A crash is the one + * loss cause with a hookable moment, which is exactly why it can be served on its own. The kill that + * 5494 exists for produces no report at all -- nothing runs on a SIGKILL -- so it was never this + * ticket's case. + */ +class MetricsCrashAttachment( + private val context: Context, +) : EventProcessor { + override fun process( + event: SentryEvent, + hint: Hint, + ): SentryEvent { + // Everything, including Errors. This runs while the process is dying, and an OutOfMemoryError + // raised in here would replace a useful report with no report -- losing the attachment is the + // right way to fail. runCatching is what makes that true: it catches Throwable. + runCatching { attach(hint) } + .onFailure { failure -> log.warn("Could not attach the metrics file to the report", failure) } + return event + } + + private fun attach(hint: Hint) { + // No source before the editor has run: a crash in onboarding, in the project chooser or in + // direct boot has no history to report, and direct boot has no credential-protected cache to + // write it to either. + val metrics = MetricsSource.current ?: return + val file = writeSnapshot(metrics) ?: return + hint.addAttachment(Attachment(file.absolutePath, file.name, MetricsCsvFile.COMPRESSED_MIME_TYPE)) + } + + private fun writeSnapshot(metrics: MetricsSource.Metrics): File? = + MetricsSnapshotAssembler.withSnapshot( + context = context, + memory = metrics.memoryUsageWatcher, + network = metrics.networkUsageWatcher, + power = metrics.powerUsageWatcher, + annotations = metrics.annotations, + ) { snapshot -> + // Nothing sampled yet is nothing to say. A header-only attachment on every early crash + // would be noise in the reports rather than context. + if (!snapshot.hasRows) null else MetricsCsvFile.writeForReport(context, snapshot) + } + + companion object { + private val log = LoggerFactory.getLogger(MetricsCrashAttachment::class.java) + + /** Registers this processor. Call once, from within `SentryAndroid.init`. */ + fun install( + options: SentryOptions, + context: Context, + ) { + options.addEventProcessor(MetricsCrashAttachment(context)) + } + } +} diff --git a/app/src/test/java/com/itsaky/androidide/handlers/MetricsCrashAttachmentTest.kt b/app/src/test/java/com/itsaky/androidide/handlers/MetricsCrashAttachmentTest.kt new file mode 100644 index 0000000000..41f5be08e6 --- /dev/null +++ b/app/src/test/java/com/itsaky/androidide/handlers/MetricsCrashAttachmentTest.kt @@ -0,0 +1,135 @@ +/* + * 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.handlers + +import android.content.Context +import androidx.test.core.app.ApplicationProvider +import com.google.common.truth.Truth.assertThat +import com.itsaky.androidide.utils.MemoryUsageWatcher +import com.itsaky.androidide.utils.MetricsAnnotationStore +import com.itsaky.androidide.utils.MetricsCsvFile +import com.itsaky.androidide.utils.MetricsScratch +import com.itsaky.androidide.utils.MetricsSource +import com.itsaky.androidide.utils.NetworkUsageWatcher +import com.itsaky.androidide.utils.PowerUsageWatcher +import io.sentry.Hint +import io.sentry.SentryEvent +import org.junit.After +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import java.io.File +import java.util.zip.GZIPInputStream + +/** + * What a report carries about the machine that produced it (ADFA-5526). + * + * The failure modes matter more than the happy path here: this runs while the process is dying, so + * anything it throws costs the whole report rather than just the attachment. + */ +@RunWith(RobolectricTestRunner::class) +class MetricsCrashAttachmentTest { + private val context = ApplicationProvider.getApplicationContext() + + private val watchers = mutableListOf() + + @After + fun tearDown() { + watchers.forEach { it.stopWatching() } + watchers.clear() + MetricsSource.current?.let(MetricsSource::unregister) + MetricsScratch.resetForTesting() + } + + private class FakeMetrics( + override val memoryUsageWatcher: MemoryUsageWatcher, + override val networkUsageWatcher: NetworkUsageWatcher = NetworkUsageWatcher(uid = 0), + override val powerUsageWatcher: PowerUsageWatcher = + PowerUsageWatcher( + source = { + PowerUsageWatcher.PowerReading( + temperatureMilliCelsius = 29_700L, + powerMicroWatts = -3_400_000L, + thermalStatus = 0, + battery = PowerUsageWatcher.BatteryState.UNKNOWN, + ) + }, + ), + override val annotations: MetricsAnnotationStore = MetricsAnnotationStore(), + ) : MetricsSource.Metrics + + private fun sampledWatcher(): MemoryUsageWatcher = + MemoryUsageWatcher().also { watcher -> + watchers += watcher + watcher.watchProcess(android.os.Process.myPid(), "IDE") + watcher.readUsages() + } + + private fun process(): Hint { + val hint = Hint() + MetricsCrashAttachment(context).process(SentryEvent(), hint) + return hint + } + + @Test + fun `a report from a session with history carries it, gzipped`() { + MetricsSource.register(FakeMetrics(sampledWatcher())) + + val attachments = process().attachments + + assertThat(attachments).hasSize(1) + val attachment = attachments.single() + assertThat(attachment.contentType).isEqualTo(MetricsCsvFile.COMPRESSED_MIME_TYPE) + assertThat(attachment.filename).endsWith(".csv.gz") + // A named file that does not unzip is worse than none: it looks like context and is not. + val unzipped = GZIPInputStream(File(attachment.pathname!!).inputStream()).bufferedReader().use { it.readText() } + assertThat(unzipped.lineSequence().first()).startsWith("\"timestamp\"") + assertThat(unzipped.lineSequence().count()).isAtLeast(2) + } + + @Test + fun `a crash before the editor ran attaches nothing`() { + // Onboarding, the project chooser, direct boot: no watchers exist, and direct boot has no + // credential-protected cache to write to either. + assertThat(MetricsSource.current).isNull() + + assertThat(process().attachments).isEmpty() + } + + @Test + fun `a session that has sampled nothing attaches nothing`() { + // A header-only file on every early crash would be noise in the reports, not context. + MetricsSource.register(FakeMetrics(MemoryUsageWatcher().also(watchers::add))) + + assertThat(process().attachments).isEmpty() + } + + @Test + fun `the event is returned unchanged even when the attachment fails`() { + // The whole point of the guard: a report with no metrics beats no report. A watcher whose + // buffers are a different length than the scratch makes copyInto throw, which is the + // closest stand-in for the crash-time failures this has to survive. + MetricsScratch.install(entries = MemoryUsageWatcher.MAX_USAGE_ENTRIES + 1, memorySeries = 3) + MetricsSource.register(FakeMetrics(sampledWatcher())) + + val event = SentryEvent() + val returned = MetricsCrashAttachment(context).process(event, Hint()) + + assertThat(returned).isSameInstanceAs(event) + } +} From 020459c7d9bdba99e55654d2dde685153df91e3a Mon Sep 17 00:00:00 2001 From: David Schachter Date: Mon, 7 Sep 2026 01:54:40 -0700 Subject: [PATCH 4/6] ADFA-5526: prove the attachment reaches the envelope The processor putting an Attachment on the Hint is our half of the hop; whether Sentry carries it into the envelope it sends is the SDK's, and asserting on our own call says nothing about that. So this runs the real SDK against a transport that keeps what it is handed, captures an exception, and reads the envelope back: an attachment item named *.csv.gz, content type application/gzip, whose bytes gunzip to a CSV with the expected header and at least one data row. Confirmed to fail when the addAttachment call is removed. The negative case is pinned too -- a session with no samples produces an envelope with no metrics attachment. Why this rather than a device run. A crash on device does reach the processor: verified on a Pixel 6 Pro with a build carrying a non-resolvable DSN, where `am crash` on the app's main pid wrote 2026_09_07_01_48_52_523.csv.gz at crash time. But no event envelope survives to disk to read back, because the IDE's own uncaught handler captures and then calls exitProcess without waiting for Sentry to flush. Session and log envelopes cache fine, so the cache works; the crash event never gets written. That looks like ordinary crash reports being lost whatever this ticket does, and it is worth its own ticket rather than a change here. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_017CCQUU7tBzZL61EmQhJP8j --- .../MetricsCrashAttachmentEnvelopeTest.kt | 177 ++++++++++++++++++ 1 file changed, 177 insertions(+) create mode 100644 app/src/test/java/com/itsaky/androidide/handlers/MetricsCrashAttachmentEnvelopeTest.kt diff --git a/app/src/test/java/com/itsaky/androidide/handlers/MetricsCrashAttachmentEnvelopeTest.kt b/app/src/test/java/com/itsaky/androidide/handlers/MetricsCrashAttachmentEnvelopeTest.kt new file mode 100644 index 0000000000..f5b3b05960 --- /dev/null +++ b/app/src/test/java/com/itsaky/androidide/handlers/MetricsCrashAttachmentEnvelopeTest.kt @@ -0,0 +1,177 @@ +/* + * 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.handlers + +import android.content.Context +import androidx.test.core.app.ApplicationProvider +import com.google.common.truth.Truth.assertThat +import com.itsaky.androidide.utils.MemoryUsageWatcher +import com.itsaky.androidide.utils.MetricsAnnotationStore +import com.itsaky.androidide.utils.MetricsScratch +import com.itsaky.androidide.utils.MetricsSource +import com.itsaky.androidide.utils.NetworkUsageWatcher +import com.itsaky.androidide.utils.PowerUsageWatcher +import io.sentry.Hint +import io.sentry.ITransportFactory +import io.sentry.Sentry +import io.sentry.SentryEnvelope +import io.sentry.SentryItemType +import io.sentry.SentryOptions +import io.sentry.transport.ITransport +import io.sentry.transport.RateLimiter +import org.junit.After +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import java.util.zip.GZIPInputStream + +/** + * The last hop: that the attachment this handler puts on a [Hint] reaches the envelope Sentry sends. + * + * Everything up to the Hint is covered by [MetricsCrashAttachmentTest]. This runs the real SDK with + * a transport that keeps what it is handed, because the hop itself is the SDK's to make and asserting + * on our own call proves nothing about it. + * + * Why not on a device: a crash there does reach this processor -- verified, it writes its file -- but + * the IDE's own uncaught handler calls exitProcess straight after capturing, so no event envelope + * survives to disk to be read back. That is worth its own ticket and is not this hop. + */ +@RunWith(RobolectricTestRunner::class) +class MetricsCrashAttachmentEnvelopeTest { + private val context = ApplicationProvider.getApplicationContext() + + private val sent = mutableListOf() + + private val watchers = mutableListOf() + + @After + fun tearDown() { + Sentry.close() + watchers.forEach { it.stopWatching() } + watchers.clear() + MetricsSource.current?.let(MetricsSource::unregister) + MetricsScratch.resetForTesting() + sent.clear() + } + + private inner class RecordingTransport : ITransport { + override fun send( + envelope: SentryEnvelope, + hint: Hint, + ) { + sent += envelope + } + + override fun flush(timeoutMillis: Long) = Unit + + override fun getRateLimiter(): RateLimiter? = null + + override fun close(isRestarting: Boolean) = Unit + + override fun close() = Unit + } + + private fun sampledMetrics(): MetricsSource.Metrics { + val memory = + MemoryUsageWatcher().also { watcher -> + watchers += watcher + watcher.watchProcess(android.os.Process.myPid(), "IDE") + watcher.readUsages() + } + return object : MetricsSource.Metrics { + override val memoryUsageWatcher = memory + override val networkUsageWatcher = NetworkUsageWatcher(uid = 0) + override val powerUsageWatcher = + PowerUsageWatcher( + source = { + PowerUsageWatcher.PowerReading( + temperatureMilliCelsius = 29_700L, + powerMicroWatts = -3_400_000L, + thermalStatus = 0, + battery = PowerUsageWatcher.BatteryState.UNKNOWN, + ) + }, + ) + override val annotations = MetricsAnnotationStore() + } + } + + private fun startSentry() { + Sentry.init { options: SentryOptions -> + // A DSN that cannot resolve, and a transport that never touches the network anyway. + options.dsn = "https://0123456789abcdef0123456789abcdef@sentry.invalid/1" + options.isEnableUncaughtExceptionHandler = false + options.setTransportFactory { _, _ -> RecordingTransport() } + MetricsCrashAttachment.install(options, context) + } + } + + private fun attachmentsOf(envelope: SentryEnvelope) = envelope.items.filter { it.header.type == SentryItemType.Attachment } + + @Test + fun `the metrics file arrives in the envelope Sentry sends`() { + MetricsSource.register(sampledMetrics()) + startSentry() + + Sentry.captureException(RuntimeException("boom")) + + assertThat(sent).isNotEmpty() + val attachments = sent.flatMap(::attachmentsOf) + val metrics = + attachments.single { + it.header.fileName + .orEmpty() + .endsWith(".csv.gz") + } + assertThat(metrics.header.contentType).isEqualTo("application/gzip") + // The bytes have to survive the trip, not just the filename: an envelope carrying a name and + // no readable payload would look like context and be none. + val csv = GZIPInputStream(metrics.data.inputStream()).bufferedReader().use { it.readText() } + assertThat(csv.lineSequence().first()).startsWith("\"timestamp\"") + assertThat(csv.lineSequence().count()).isAtLeast(2) + } + + @Test + fun `an envelope from a session with no samples carries no metrics attachment`() { + MetricsSource.register( + object : MetricsSource.Metrics { + override val memoryUsageWatcher = MemoryUsageWatcher().also(watchers::add) + override val networkUsageWatcher = NetworkUsageWatcher(uid = 0) + override val powerUsageWatcher = + PowerUsageWatcher( + source = { + PowerUsageWatcher.PowerReading(0L, 0L, 0, PowerUsageWatcher.BatteryState.UNKNOWN) + }, + ) + override val annotations = MetricsAnnotationStore() + }, + ) + startSentry() + + Sentry.captureException(RuntimeException("boom")) + + assertThat(sent).isNotEmpty() + assertThat( + sent.flatMap(::attachmentsOf).filter { + it.header.fileName + .orEmpty() + .endsWith(".csv.gz") + }, + ).isEmpty() + } +} From f89e523311e11c262424388c8e19852dc6fc7769 Mon Sep 17 00:00:00 2001 From: David Schachter Date: Mon, 7 Sep 2026 05:34:18 -0700 Subject: [PATCH 5/6] ADFA-5526: close the blocks the rebase resolution left open Three stray braces from resolving this branch onto the row-alignment fix, where copyUsageInto and copyHistoryInto grew a times destination. No behaviour: the files did not parse before this. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_017CCQUU7tBzZL61EmQhJP8j --- .../java/com/itsaky/androidide/utils/MemoryUsageWatcher.kt | 1 - .../java/com/itsaky/androidide/utils/NetworkUsageWatcher.kt | 4 +--- .../java/com/itsaky/androidide/utils/PowerUsageWatcher.kt | 1 - 3 files changed, 1 insertion(+), 5 deletions(-) diff --git a/app/src/main/java/com/itsaky/androidide/utils/MemoryUsageWatcher.kt b/app/src/main/java/com/itsaky/androidide/utils/MemoryUsageWatcher.kt index 567d11d670..b32ebd5980 100644 --- a/app/src/main/java/com/itsaky/androidide/utils/MemoryUsageWatcher.kt +++ b/app/src/main/java/com/itsaky/androidide/utils/MemoryUsageWatcher.kt @@ -367,7 +367,6 @@ class MemoryUsageWatcher }, ) } - } /** * Returns the memory usage of all the registered processes. diff --git a/app/src/main/java/com/itsaky/androidide/utils/NetworkUsageWatcher.kt b/app/src/main/java/com/itsaky/androidide/utils/NetworkUsageWatcher.kt index 0194ee904a..2132ba3d0e 100644 --- a/app/src/main/java/com/itsaky/androidide/utils/NetworkUsageWatcher.kt +++ b/app/src/main/java/com/itsaky/androidide/utils/NetworkUsageWatcher.kt @@ -154,8 +154,7 @@ class NetworkUsageWatcher * The arrays are copies. Handing out the live ring buffers would let the caller read them while * the sampler thread is midway through appending, and the chart renderer reads all 30 entries. */ - fun getUsage(): NetworkUsage = - copyUsageInto(LongArray(received.size), LongArray(transmitted.size), LongArray(sampleTimes.size)) + fun getUsage(): NetworkUsage = copyUsageInto(LongArray(received.size), LongArray(transmitted.size), LongArray(sampleTimes.size)) /** * [getUsage], into destinations the caller owns (ADFA-5526). @@ -176,7 +175,6 @@ class NetworkUsageWatcher sampleTimes.copyInto(timesDest), ) } - } /** * Discards every recorded sample and drops the cumulative baseline, so the next sample diff --git a/app/src/main/java/com/itsaky/androidide/utils/PowerUsageWatcher.kt b/app/src/main/java/com/itsaky/androidide/utils/PowerUsageWatcher.kt index 4d94b102a7..94a6136b82 100644 --- a/app/src/main/java/com/itsaky/androidide/utils/PowerUsageWatcher.kt +++ b/app/src/main/java/com/itsaky/androidide/utils/PowerUsageWatcher.kt @@ -168,7 +168,6 @@ class PowerUsageWatcher sampleTimes.copyInto(timesDest), ) } - } fun clearHistory() { synchronized(historyLock) { From 489cf63f5c9e742b3fdd95fcd77beb4c9f8888ea Mon Sep 17 00:00:00 2001 From: David Schachter Date: Mon, 7 Sep 2026 07:40:01 -0700 Subject: [PATCH 6/6] ADFA-5526: stop paying for a fresh file on every report This processor is registered for every Sentry event, not only crashes, and the IDE captures non-fatals on purpose -- in bursts, on whatever thread noticed, the main one included. Each one formatted and gzipped a full 3600-row buffer synchronously on that thread: 10-15ms measured on a desktop JVM, more on a phone. A crash pays that once and it does not matter; a burst pays it per event, and on the main thread that is a stutter per event. A file written within the last five seconds is handed out again instead. Every event still gets an attachment, and a crash gives up at most five seconds of its own tail -- short on purpose, because that tail is the part worth having. The existence check is not belt and braces: MetricsCsvFile prunes to the few most recent, so a file handed out here can be deleted by a later write. Not split by event type, which is the obvious alternative: the IDE reports its own crashes through a plain Sentry.captureException, so SentryEvent.isCrashed is false for them and there is nothing at this level that tells a crash from a deliberate capture. Three tests; the burst one fails without the reuse. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_017CCQUU7tBzZL61EmQhJP8j --- .../handlers/MetricsCrashAttachment.kt | 86 ++++++++++++++++--- .../handlers/MetricsCrashAttachmentTest.kt | 82 ++++++++++++++++++ 2 files changed, 157 insertions(+), 11 deletions(-) diff --git a/app/src/main/java/com/itsaky/androidide/handlers/MetricsCrashAttachment.kt b/app/src/main/java/com/itsaky/androidide/handlers/MetricsCrashAttachment.kt index 153b953947..e5c1796db9 100644 --- a/app/src/main/java/com/itsaky/androidide/handlers/MetricsCrashAttachment.kt +++ b/app/src/main/java/com/itsaky/androidide/handlers/MetricsCrashAttachment.kt @@ -18,6 +18,8 @@ package com.itsaky.androidide.handlers import android.content.Context +import android.os.SystemClock +import com.itsaky.androidide.utils.MetricsCsv import com.itsaky.androidide.utils.MetricsCsvFile import com.itsaky.androidide.utils.MetricsSnapshotAssembler import com.itsaky.androidide.utils.MetricsSource @@ -47,7 +49,26 @@ import java.io.File */ class MetricsCrashAttachment( private val context: Context, + private val nowMillis: () -> Long = SystemClock::elapsedRealtime, + private val writeFile: (MetricsCsv.Snapshot) -> File? = { snapshot -> + MetricsCsvFile.writeForReport(context, snapshot) + }, ) : EventProcessor { + /** + * The last file written, and when. Reused rather than rewritten for a moment afterwards. + * + * Not synchronised: two events racing here write two files and one of them wins the field, + * which costs a write and loses nothing. A lock would be the more expensive mistake, since this + * runs on the thread of whatever is being reported. + */ + @Volatile + private var recent: Recent? = null + + private class Recent( + val atMillis: Long, + val file: File, + ) + override fun process( event: SentryEvent, hint: Hint, @@ -69,22 +90,65 @@ class MetricsCrashAttachment( hint.addAttachment(Attachment(file.absolutePath, file.name, MetricsCsvFile.COMPRESSED_MIME_TYPE)) } - private fun writeSnapshot(metrics: MetricsSource.Metrics): File? = - MetricsSnapshotAssembler.withSnapshot( - context = context, - memory = metrics.memoryUsageWatcher, - network = metrics.networkUsageWatcher, - power = metrics.powerUsageWatcher, - annotations = metrics.annotations, - ) { snapshot -> - // Nothing sampled yet is nothing to say. A header-only attachment on every early crash - // would be noise in the reports rather than context. - if (!snapshot.hasRows) null else MetricsCsvFile.writeForReport(context, snapshot) + /** + * The file to attach, writing one if the last is too old to stand in. + * + * This runs on the thread of whatever is being reported, and it is not cheap: a full buffer is + * 3600 rows, which format and gzip in 10-15ms on a desktop JVM and a good deal more on a phone. + * A crash pays that once and it does not matter. But this processor is deliberately registered + * for *every* event, including the non-fatal `Sentry.captureException` calls the IDE makes on + * purpose -- and those arrive in bursts, on whatever thread noticed, the main one included. Paid + * per event that is a visible stutter per event. + * + * So a file written moments ago is handed out again instead. The window is short because + * freshness matters most at exactly the moment this is for: a crash gets at most + * [REUSE_WINDOW_MS] less of its own tail, while a burst of non-fatals collapses to one write. + * Every event still gets an attachment, which distinguishing crashes from non-fatals would not + * manage here -- the IDE reports its own crashes through a plain `captureException`, so + * `SentryEvent.isCrashed` is false for them and there is nothing at this level to tell the two + * apart. + * + * The existence check is not belt and braces: [MetricsCsvFile] prunes its directory to the few + * most recent, so a file handed out here can be deleted by a later write. + */ + private fun writeSnapshot(metrics: MetricsSource.Metrics): File? { + val now = nowMillis() + recent?.let { last -> + if (now - last.atMillis < REUSE_WINDOW_MS && last.file.exists()) { + return last.file + } + } + + val file = + MetricsSnapshotAssembler.withSnapshot( + context = context, + memory = metrics.memoryUsageWatcher, + network = metrics.networkUsageWatcher, + power = metrics.powerUsageWatcher, + annotations = metrics.annotations, + ) { snapshot -> + // Nothing sampled yet is nothing to say. A header-only attachment on every early + // crash would be noise in the reports rather than context. + if (!snapshot.hasRows) null else writeFile(snapshot) + } + if (file != null) { + recent = Recent(now, file) } + return file + } companion object { private val log = LoggerFactory.getLogger(MetricsCrashAttachment::class.java) + /** + * How long a written file stands in for the next one. + * + * Short deliberately: the cost this bounds is a burst of non-fatals, which arrive far + * faster than this, and the thing it risks is the tail of a crash, which is the part worth + * having. + */ + const val REUSE_WINDOW_MS = 5_000L + /** Registers this processor. Call once, from within `SentryAndroid.init`. */ fun install( options: SentryOptions, diff --git a/app/src/test/java/com/itsaky/androidide/handlers/MetricsCrashAttachmentTest.kt b/app/src/test/java/com/itsaky/androidide/handlers/MetricsCrashAttachmentTest.kt index 41f5be08e6..f80fa709d4 100644 --- a/app/src/test/java/com/itsaky/androidide/handlers/MetricsCrashAttachmentTest.kt +++ b/app/src/test/java/com/itsaky/androidide/handlers/MetricsCrashAttachmentTest.kt @@ -119,6 +119,88 @@ class MetricsCrashAttachmentTest { assertThat(process().attachments).isEmpty() } + @Test + fun `a burst of reports pays for one file, not one each`() { + MetricsSource.register(FakeMetrics(sampledWatcher())) + var writes = 0 + var clock = 1_000L + val processor = + MetricsCrashAttachment( + context = context, + nowMillis = { clock }, + writeFile = { snapshot -> + writes++ + MetricsCsvFile.writeForReport(context, snapshot) + }, + ) + + // This is registered for every event, not only crashes, and the IDE captures non-fatals + // deliberately -- in bursts, on whatever thread noticed, the main one included. A full + // buffer formats and gzips in 10-15ms on a desktop JVM and more on a phone, so paid per + // event that is a visible stutter per event. + val filenames = + (1..5).map { + clock += 100L + val hint = Hint() + processor.process(SentryEvent(), hint) + hint.attachments.single().filename + } + + assertThat(writes).isEqualTo(1) + assertThat(filenames.toSet()).hasSize(1) + } + + @Test + fun `a report after the window gets a file of its own`() { + MetricsSource.register(FakeMetrics(sampledWatcher())) + var writes = 0 + var clock = 1_000L + val processor = + MetricsCrashAttachment( + context = context, + nowMillis = { clock }, + writeFile = { snapshot -> + writes++ + MetricsCsvFile.writeForReport(context, snapshot) + }, + ) + + processor.process(SentryEvent(), Hint()) + // Freshness is what matters at a crash, so the reuse has to expire rather than latch. + clock += MetricsCrashAttachment.REUSE_WINDOW_MS + processor.process(SentryEvent(), Hint()) + + assertThat(writes).isEqualTo(2) + } + + @Test + fun `a reused file that has been pruned away is written again`() { + MetricsSource.register(FakeMetrics(sampledWatcher())) + var writes = 0 + val clock = 1_000L + val processor = + MetricsCrashAttachment( + context = context, + nowMillis = { clock }, + writeFile = { snapshot -> + writes++ + MetricsCsvFile.writeForReport(context, snapshot) + }, + ) + + val first = Hint() + processor.process(SentryEvent(), first) + // MetricsCsvFile keeps only the few most recent, so a file handed out here can be deleted + // by a later write. An attachment naming a file that is gone is worse than none. + File(first.attachments.single().pathname!!).delete() + + val second = Hint() + processor.process(SentryEvent(), second) + + assertThat(writes).isEqualTo(2) + assertThat(File(second.attachments.single().pathname!!).exists()).isTrue() + } + @Test fun `the event is returned unchanged even when the attachment fails`() { // The whole point of the guard: a report with no metrics beats no report. A watcher whose