diff --git a/app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt b/app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt index eb4fb9bd35..d69e296916 100644 --- a/app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt +++ b/app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt @@ -130,6 +130,7 @@ import com.itsaky.androidide.utils.InstallationResultHandler.onResult import com.itsaky.androidide.utils.IntentUtils import com.itsaky.androidide.utils.MemoryUsageWatcher import com.itsaky.androidide.utils.MetricsAnnotationStore +import com.itsaky.androidide.utils.MetricsCsv import com.itsaky.androidide.utils.StringsInjectionException import com.itsaky.androidide.utils.StringsXmlInjector import com.itsaky.androidide.utils.applyBottomSheetAnchorForOrientation @@ -490,13 +491,16 @@ abstract class BaseEditorActivity : else -> Color.GRAY } - protected val PROC_IDE = "IDE" + // Aliases, not copies. The names belong to the CSV, whose header is a published contract; + // see MetricsCsv.PROC_IDE for why they live there. Kept as protected members because + // subclasses use them. + protected val PROC_IDE = MetricsCsv.PROC_IDE @JvmStatic - protected val PROC_GRADLE_TOOLING = "Gradle Tooling" + protected val PROC_GRADLE_TOOLING = MetricsCsv.PROC_GRADLE_TOOLING @JvmStatic - protected val PROC_GRADLE_DAEMON = "Gradle Daemon" + protected val PROC_GRADLE_DAEMON = MetricsCsv.PROC_GRADLE_DAEMON @JvmStatic protected val log: Logger = LoggerFactory.getLogger(BaseEditorActivity::class.java) diff --git a/app/src/main/java/com/itsaky/androidide/ui/MetricsCarouselController.kt b/app/src/main/java/com/itsaky/androidide/ui/MetricsCarouselController.kt index a919264a05..4c77e71308 100644 --- a/app/src/main/java/com/itsaky/androidide/ui/MetricsCarouselController.kt +++ b/app/src/main/java/com/itsaky/androidide/ui/MetricsCarouselController.kt @@ -22,6 +22,7 @@ import android.content.Context import android.content.ContextWrapper import android.content.Intent import android.content.res.ColorStateList +import android.os.SystemClock import android.util.TypedValue import android.view.View import android.view.ViewGroup @@ -46,6 +47,8 @@ import com.itsaky.androidide.utils.DialogUtils import com.itsaky.androidide.utils.IntentUtils import com.itsaky.androidide.utils.MemoryUsageWatcher import com.itsaky.androidide.utils.MetricsAnnotationStore +import com.itsaky.androidide.utils.MetricsCsv +import com.itsaky.androidide.utils.MetricsCsvFile import com.itsaky.androidide.utils.MetricsSamplingRates import com.itsaky.androidide.utils.MetricsSnapshot import com.itsaky.androidide.utils.NetworkUsageWatcher @@ -170,14 +173,26 @@ class MetricsCarouselController( private var currentPage = 0 /** - * Whether an export is already running. + * Whether a PNG snapshot is already being written. * * One at a time. The camera button is not debounced and each tap launched its own coroutine, * so two quick taps raced over the same scratch directory -- and, within the same second, over * the same filename, since the name is the chart label and a whole-second timestamp. Touched * only on the main thread, which is where both the tap and the coroutine's continuations run. + * + * A rapid second tap of the same button is refused silently: it is the double tap this exists + * to swallow, and a message for it would be noise on the gesture a user did not mean to make. */ - private var exportInFlight = false + private var snapshotInFlight = false + + /** + * Whether a CSV export is already being written, tracked apart from [snapshotInFlight]. + * + * The two write different files into different directories and cannot race each other, so one + * flag for both only meant that starting a ten-thousand-row export refused the camera button + * for as long as it ran -- and refused it silently, which reads as a dead control. + */ + private var csvExportInFlight = false /** * The pager of the bound carousel, or `null` when nothing is bound. Exposed so a host can apply @@ -253,6 +268,7 @@ class MetricsCarouselController( // A camera button in the graph's bottom-right corner exports the chart. The gestures over // the chart are all spoken for, so this is a control rather than another gesture. binding.metricsSnapshot.setOnClickListener { exportSnapshot() } + binding.metricsExport.setOnClickListener { exportCsv() } // Arrows are the dependable way to move between pages: a swipe has to share the gesture // with panning a zoomed chart and with the editor's drawer, and loses often enough to be @@ -308,6 +324,7 @@ class MetricsCarouselController( binding.metricsPrevious to TooltipTag.CAROUSEL_PREVIOUS, binding.metricsNext to TooltipTag.CAROUSEL_NEXT, binding.metricsSnapshot to TooltipTag.CAROUSEL_SNAPSHOT, + binding.metricsExport to TooltipTag.CAROUSEL_EXPORT, binding.metricsBattery to TooltipTag.CAROUSEL_BATTERY, // Wired even though it is only visible while undocked: the message is the one control // that outlives unbind(), so its help must not be torn down with the rest. @@ -334,6 +351,7 @@ class MetricsCarouselController( networkRenderer.onXAxisTap = null powerRenderer.onXAxisTap = null binding?.metricsSnapshot?.setOnClickListener(null) + binding?.metricsExport?.setOnClickListener(null) binding?.let { bound -> helpTargets(bound) // All but the undocked message: that view becomes visible *because* the carousel @@ -583,7 +601,7 @@ class MetricsCarouselController( @UiThread fun exportSnapshot(): Boolean { val binding = this.binding ?: return false - if (exportInFlight) { + if (snapshotInFlight) { log.debug("Ignoring a snapshot request while one is already being written") return false } @@ -607,7 +625,7 @@ class MetricsCarouselController( // it ends in startActivity, which throws from a context with no task of its own unless it is // given FLAG_ACTIVITY_NEW_TASK, so it keeps the context the carousel is hosted in. val appContext = context.applicationContext - exportInFlight = true + snapshotInFlight = true scope.launch { // Everything here is guarded: the scope has no exception handler, so anything escaping // reaches the global crash reporter and is filed as a crash. MetricsSnapshot.write @@ -620,7 +638,7 @@ class MetricsCarouselController( // fresh full-size ARGB_8888 copy of the plot on every tap, which is // megabytes that would otherwise sit around until the collector noticed. try { - MetricsSnapshot.write(appContext, bitmap, label) + MetricsSnapshot.write(appContext, bitmap) } finally { bitmap.recycle() } @@ -643,17 +661,148 @@ class MetricsCarouselController( // Cleared before rethrowing: a cancelled export is finished either way, and // leaving the flag set would refuse every later one for the life of the // carousel. - exportInFlight = false + snapshotInFlight = false throw failure } log.error("Could not share the chart snapshot", failure) Toast.makeText(appContext, string.msg_metrics_snapshot_failed, Toast.LENGTH_SHORT).show() } - exportInFlight = false + snapshotInFlight = false + } + return true + } + + /** + * Writes every retained sample to a CSV file and offers it to another app (ADFA-5531). + * + * The whole buffer, not the visible window and not the current page: this is the file the + * metrics format is defined around, and ADFA-5494, ADFA-5526 and ADFA-5534 all want everything + * there is. Assembled on the UI thread because it is snapshots of the watchers' buffers, then + * formatted and written off it -- ten thousand rows is not a click listener's work. + * + * @return whether an export could be started. The write itself completes later. + */ + @UiThread + fun exportCsv(): Boolean { + val binding = this.binding ?: return false + if (csvExportInFlight) { + log.debug("Ignoring an export request while one is already being written") + return false + } + + val context = binding.root.context + val appContext = context.applicationContext + val snapshot = snapshot() + csvExportInFlight = true + scope.launch { + // Guarded for the same reason exportSnapshot is: the scope has no exception handler, so + // anything escaping here is filed as a crash. + runCatching { + val file = withContext(Dispatchers.IO) { MetricsCsvFile.write(appContext, snapshot) } + val host = this@MetricsCarouselController.binding?.root?.context + if (file == null || host == null) { + Toast.makeText(appContext, string.msg_metrics_export_failed, Toast.LENGTH_SHORT).show() + return@runCatching + } + val extraFlags = + if (host.findActivityOrNull() == null) Intent.FLAG_ACTIVITY_NEW_TASK else 0 + IntentUtils.shareFile(host, file, MetricsCsv.MIME_TYPE, extraFlags) + }.onFailure { failure -> + if (failure is CancellationException) { + csvExportInFlight = false + throw failure + } + log.error("Could not share the metrics export", failure) + Toast.makeText(appContext, string.msg_metrics_export_failed, Toast.LENGTH_SHORT).show() + } + csvExportInFlight = false } return true } + /** + * The watchers' buffers, as the export format's view of them. + * + * Rows come from the memory watcher: it is the only one always recording, the network watcher + * stops for good on a device whose counters are unsupported, and a power source can be missing. + * The other series are read at the same index -- the watchers share an interval, are started + * together and are cleared together -- and each carries its own sample times, so a series that + * was not recording leaves empty cells rather than zeros. + */ + @UiThread + @VisibleForTesting + internal fun snapshot(): MetricsCsv.Snapshot { + // One call per watcher, not one per array. Each returns its times and its values from a + // single critical section, which is what keeps a row of the file a single moment: two calls + // let the sampler append between them and every value came out one row off its timestamp. + val memory = memoryUsageWatcher.history() + val network = networkUsageWatcher.getUsage() + val power = powerUsageWatcher.getUsage() + + return MetricsCsv.Snapshot( + rowTimes = memory.times, + // The memory watcher's, because its times are the rows. + sampleIntervalMillis = memoryUsageWatcher.updateInterval, + memory = + memory.processes.associate { process -> + process.pname to + MetricsCsv.Series( + times = memory.times, + values = process.usage, + since = process.watchedSinceMillis, + ) + }, + networkReceived = MetricsCsv.Series(network.sampleTimes, network.received), + networkTransmitted = MetricsCsv.Series(network.sampleTimes, network.transmitted), + // The power series carry in-band sentinels for a reading the device does not provide. + // Named here rather than in MetricsCsv, which has no Android types in it. + temperature = + MetricsCsv.Series( + power.sampleTimes, + power.temperatureMilliCelsius, + absent = PowerUsageWatcher.UNAVAILABLE, + ), + power = + MetricsCsv.Series( + power.sampleTimes, + power.powerMicroWatts, + absent = PowerUsageWatcher.UNAVAILABLE, + ), + thermal = + MetricsCsv.Series( + power.sampleTimes, + power.thermalStatus, + absent = PowerUsageWatcher.THERMAL_UNKNOWN.toLong(), + ), + annotations = markers(), + ) + } + + /** + * The annotations, with their times moved onto the clock the samples carry. + * + * The store records on the monotonic clock and the samples on the wall clock, and the two are + * read here as close together as they can be so the offset between them is the right one. + */ + private fun markers(): List { + val store = annotations ?: return emptyList() + val nowEpoch = System.currentTimeMillis() + val nowMonotonic = SystemClock.elapsedRealtime() + return store.allAnnotations().map { annotation -> + MetricsCsv.Marker( + atMillis = MetricsCsv.epochFor(annotation.atMillis, nowEpoch, nowMonotonic), + label = labelFor(annotation), + kind = annotation.kind.name, + ) + } + } + + /** An annotation's text: a build outcome carries a string id, a task carries its own name. */ + private fun labelFor(annotation: MetricsAnnotationStore.Annotation): String { + val context = binding?.root?.context ?: return annotation.label + return annotation.kind.labelRes?.let(context::getString) ?: annotation.label + } + /** * Releases the controller for good. Distinct from [unbind], which runs on every dock, undock * and recreation; this is the terminal teardown and cancels any snapshot still being written. diff --git a/app/src/main/java/com/itsaky/androidide/ui/MetricsCarouselLayout.kt b/app/src/main/java/com/itsaky/androidide/ui/MetricsCarouselLayout.kt index b568754857..f5a6ddc746 100644 --- a/app/src/main/java/com/itsaky/androidide/ui/MetricsCarouselLayout.kt +++ b/app/src/main/java/com/itsaky/androidide/ui/MetricsCarouselLayout.kt @@ -73,9 +73,10 @@ class MetricsCarouselLayout /** * Shows either the carousel or the "it is in a floating window" message, never a mix. * - * The whole strip switches, not just the pager. The arrows and the snapshot button are - * chrome for a chart that is not here: left behind they sit over the message, and the - * camera is inert anyway because undocking unbinds the controller that listens to it. + * The whole strip switches, not just the pager. The arrows, the snapshot button and the + * export button are chrome for a chart that is not here: left behind they sit over the + * message, and each is inert anyway because undocking unbinds the controller that listens + * to them. * * Keeping the set here was supposed to stop a control added later from being forgotten. * It did not: the battery readout arrived afterwards and was missed, so the readout sat @@ -92,6 +93,7 @@ class MetricsCarouselLayout R.id.metrics_previous, R.id.metrics_next, R.id.metrics_snapshot, + R.id.metrics_export, ) carouselIds.forEach { id -> findViewById(id)?.isVisible = !undocked 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 229a103e27..99c477dd0b 100644 --- a/app/src/main/java/com/itsaky/androidide/utils/MemoryUsageWatcher.kt +++ b/app/src/main/java/com/itsaky/androidide/utils/MemoryUsageWatcher.kt @@ -20,6 +20,7 @@ package com.itsaky.androidide.utils import android.app.ActivityManager import android.os.Debug import android.os.Debug.MemoryInfo +import androidx.annotation.VisibleForTesting import androidx.collection.IntObjectMap import androidx.collection.MutableIntObjectMap import androidx.core.content.getSystemService @@ -58,6 +59,18 @@ class MemoryUsageWatcher updateInterval: Long = DEFAULT_UPDATE_INTERVAL, private val coroutineDispatcher: CoroutineContext = newSingleThreadContext("MemoryUsageWatcher"), private val mainDispatcher: CoroutineContext = Dispatchers.Main.immediate, + private val nowMillis: () -> Long = System::currentTimeMillis, + // Injectable for the same reason the other watchers' readers are: it is the one part of a + // sample that needs a device. ActivityManager.getProcessMemoryInfo is rate-limited and + // internally uses Debug.getMemoryInfo, so the reflective call goes around the limit. + private val readTotalPssKb: (Int, MemoryInfo) -> Int = { pid, into -> + ReflectionUtils.invokeMethod(android_os_Debug_getMemoryInfo, null, pid, into) + + // From https://developer.android.com/tools/dumpsys#meminfo + // "PSS is a good measure for the actual RAM weight of a process and for comparison + // against the RAM use of other processes and the total available RAM." + into.totalPss + }, ) { /** * Milliseconds between samples. Changing it clears the history: the chart reads a sample's @@ -84,6 +97,21 @@ class MemoryUsageWatcher private var samplingJob: Job? = null private val memoryUsage = ConcurrentHashMap() + /** + * When each sample was taken, in the same order and at the same indices as the values. + * + * Recorded rather than reconstructed. The chart infers a sample's age from its position, + * which is close enough for placing a marker on a plot, but the exported metrics file states + * a time per row (ADFA-5531) and inference would be wrong three ways: the newest sample was + * taken up to an interval before the export, the loop delays *after* doing its work so the + * true period drifts past the nominal one, and sampling can stop and restart without the + * buffer being cleared. + * + * A zero means no sample was ever recorded at that index, which is what tells a blank cell + * apart from a measured zero. + */ + private val sampleTimes = MutableShiftedLongArray(MAX_USAGE_ENTRIES) + /** * Guards the per-process ring buffers, matching [NetworkUsageWatcher] and * [PowerUsageWatcher]. The sampler appends to them; [clearHistory] wipes them from whatever @@ -183,7 +211,8 @@ class MemoryUsageWatcher } } - private fun readUsages() { + @VisibleForTesting + internal fun readUsages() { if (memoryUsage.isEmpty()) { // Nothing to sample. Returning before the service lookup keeps an idle watcher off // BaseApplication, which a unit test does not have. @@ -196,42 +225,35 @@ class MemoryUsageWatcher return } + // Read every process first, append nothing yet. The reading is the slow part and must + // not hold the lock; the append is the part a reader can see, and all of it -- the time + // and every process's value -- has to land in one critical section. A reader that + // caught the time appended but not the values got a file whose every row sat on its + // neighbour's timestamp, which is the one thing a row of this file is for (ADFA-5531). + val at = nowMillis() val pids = memoryUsage.keys.toIntArray() + val sampled = ArrayList>(pids.size) pids.forEach { pid -> - - // ActivityManager.getProcessMemoryInfo is rate-limited - // but it internally uses Debug.getMemoryInfo to get the memory info - // we use it directly using reflection to bypass the rate limit val proc = memoryUsage[pid] ?: run { log.warn("Process {} is not being watched, but readUsages() was called for the process", pid) return@forEach } - ReflectionUtils.invokeMethod(android_os_Debug_getMemoryInfo, null, pid, proc.memInfo) - - // From https://developer.android.com/tools/dumpsys#meminfo - // "PSS is a good measure for the actual RAM weight of a process and for comparison against - // the RAM use of other processes and the total available RAM." - val usage = proc.memInfo.totalPss - // values are in kB, convert to bytes - val usageBytes = usage * 1024L - memoryUsage[pid]!!.apply { - // we insert the usage entry at the start of the array, then increment the shift amount by 1 - // this makes the newly inserted usage entry the last element in the array - // and the oldest usage entry the first element in the array - - // this means that _history[_history.size - 1] will be the newest usage entry - - // the "shift" amount basically indicates what is the start index of the array - // for example, if shift is 1, then _history[0] will actually return _history[1] (index shifted by 1 to the right) - // when the shift amount exceeds the size of the array, it will be reset to 0 (wrapped around) + sampled += proc to readTotalPssKb(pid, proc.memInfo) * 1024L + } - synchronized(historyLock) { - _history[0] = usageBytes - _history.shift(1) - } + synchronized(historyLock) { + // The entry goes in at the start of the array and the shift amount goes up by one, + // which makes it the last element and the oldest the first -- so + // _history[_history.size - 1] is always the newest. The shift is the array's start + // index, wrapping back to 0 once it passes the end. + sampleTimes[0] = at + sampleTimes.shift(1) + sampled.forEach { (proc, usageBytes) -> + proc._history[0] = usageBytes + proc._history.shift(1) } } } @@ -263,6 +285,11 @@ class MemoryUsageWatcher pid, pname, MutableShiftedLongArray(MAX_USAGE_ENTRIES), + // A process can start being watched long after the others -- the Gradle daemon + // appears when a build does -- and its buffer is zero-filled back to the start + // of the session. Without this, the exported file could not tell those zeros + // from a process that really was using no memory (ADFA-5531). + watchedSinceMillis = nowMillis(), ) } @@ -277,9 +304,38 @@ class MemoryUsageWatcher // so this is reachable, not theoretical. synchronized(historyLock) { memoryUsage.values.forEach { it._history.clear() } + sampleTimes.clear() } } + /** + * Every retained sample, with the times the samples were taken at. + * + * One lock around the whole read, and it has to be: asking for the times and the values + * separately let the sampler append between the two calls, which shifts every value one + * index against its timestamp and puts each row of the exported file on its neighbour's + * time (ADFA-5531). There is no accessor for the times alone, deliberately. + * + * A zero time at an index means nothing was ever sampled there -- the buffers are + * fixed-length and start, and are cleared, full of them. Copies, for the same reason the + * values have always been copied. + */ + fun history(): MemoryHistory = + synchronized(historyLock) { + MemoryHistory( + times = sampleTimes.toLongArray(), + processes = + memoryUsage.values.map { proc -> + ProcessHistory( + pid = proc.pid, + pname = proc.pname, + usage = proc._history.toLongArray(), + watchedSinceMillis = proc.watchedSinceMillis, + ) + }, + ) + } + /** * Returns the memory usage of all the registered processes. */ @@ -353,6 +409,32 @@ class MemoryUsageWatcher (coroutineDispatcher as? ExecutorCoroutineDispatcher)?.close() } + /** + * One process's retained samples, detached from the watcher. + * + * @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 + * tells those zeros from a measurement. + */ + class ProcessHistory( + val pid: Int, + val pname: String, + val usage: LongArray, + val watchedSinceMillis: Long, + ) + + /** + * Every watched process's samples and the times they were taken at, read together. + * + * @property times When each sample was taken, oldest first, as milliseconds since the epoch, + * parallel to every entry in [processes]. + */ + class MemoryHistory( + val times: LongArray, + val processes: List, + ) + /** * Registers a listener to be notified when the memory usage of a process changes. */ @@ -377,6 +459,8 @@ class MemoryUsageWatcher val pid: Int, val pname: String, internal val _history: MutableShiftedLongArray, + /** When this process started being watched, as milliseconds since the epoch. */ + val watchedSinceMillis: Long, ) { internal val memInfo: MemoryInfo = MemoryInfo() @@ -389,7 +473,12 @@ class MemoryUsageWatcher * The MemoryInfo instance is shared deliberately: it is the sampler's scratch buffer * for the next reading and no reader looks at it. */ - internal fun snapshot(): ProcessMemoryInfo = ProcessMemoryInfo(pid, pname, _history.copy()) + internal fun snapshot(): ProcessMemoryInfo = + // Every field, including watchedSinceMillis. Dropping it let it default to 0, which + // reads as "watched since the epoch" -- so the guard that blanks a process's + // zero-filled past never fired, and the Gradle daemon's buffer exported as + // measured zeros from before it existed (ADFA-5531). + ProcessMemoryInfo(pid, pname, _history.copy(), watchedSinceMillis) override fun equals(other: Any?): Boolean { if (this === other) return true diff --git a/app/src/main/java/com/itsaky/androidide/utils/MetricsAnnotationStore.kt b/app/src/main/java/com/itsaky/androidide/utils/MetricsAnnotationStore.kt index fe70af6b2a..0aba44c264 100644 --- a/app/src/main/java/com/itsaky/androidide/utils/MetricsAnnotationStore.kt +++ b/app/src/main/java/com/itsaky/androidide/utils/MetricsAnnotationStore.kt @@ -181,6 +181,16 @@ class MetricsAnnotationStore( return annotations.filter { it.atMillis >= cutoff } } + /** + * Every annotation the store holds, oldest first. + * + * The exported metrics file carries the whole retained history rather than a window of it, so + * it cannot go through [recentAnnotations] -- there is no "within" that means "all of it" + * without the cutoff arithmetic overflowing (ADFA-5531). + */ + @Synchronized + fun allAnnotations(): List = annotations.toList() + @Synchronized fun clear() { annotations.clear() diff --git a/app/src/main/java/com/itsaky/androidide/utils/MetricsCsv.kt b/app/src/main/java/com/itsaky/androidide/utils/MetricsCsv.kt new file mode 100644 index 0000000000..68f6c695b9 --- /dev/null +++ b/app/src/main/java/com/itsaky/androidide/utils/MetricsCsv.kt @@ -0,0 +1,326 @@ +/* + * 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 java.time.Instant +import java.time.ZoneId +import java.time.format.DateTimeFormatter +import java.util.Locale +import kotlin.math.abs + +/** + * The canonical metrics file: everything the carousel has sampled, as CSV (ADFA-5531). + * + * One definition, because the file has several producers and consumers -- the export button here, + * ADFA-5494's restore across process death, and the copies ADFA-5526 and ADFA-5534 attach to crash + * reports and to feedback. Those differ from this only in compressing it. + * + * A row is a sampling tick, and its columns come from three watchers that each keep their own ring + * buffer and their own coroutine. They are started together and share one interval, and every one + * of them is cleared when that interval changes. The row's stated time is the memory watcher's, + * recorded when it sampled, and nothing here reconstructs a time from an index. + * + * The other series are paired to that row by array index rather than by time, which is not the same + * thing. Each watcher runs its own loop and does its own per-tick work, so their ticks drift apart, + * and because the buffers are filled oldest-first the drift accumulates backwards: the further back + * a row is, the further its network and power values can sit from its stated time. Every column is + * a real reading with a real time behind it, so nothing in the file is invented -- but a consumer + * must not read one row as three simultaneous measurements. Merging the series on time instead is + * the subject of its own change; this comment says what is true until then. + * + * Formatting only, with no Android types, so the whole format can be tested without a device. + */ +object MetricsCsv { + /** Media type for the written file, for the sharing intent. */ + const val MIME_TYPE = "text/csv" + + /** + * A sample time of zero means no sample was taken at that index: the ring buffers are + * fixed-length and start, and are cleared, full of zeros. + */ + const val NO_SAMPLE = 0L + + /** + * A row's time, ISO 8601 with the offset the device was on. + * + * Deliberately not the filename's format, which is built from what a filesystem allows and what + * sorts lexicographically. This one has to round-trip exactly for ADFA-5494 and be read by a + * person triaging a report from another timezone, which is what the offset is for. + * + * Spelled out rather than [DateTimeFormatter.ISO_OFFSET_DATE_TIME], which drops trailing zeros + * from the fraction and so writes a column of varying width -- ".38" for one row and ".123" for + * the next. Both parse, but a fixed three digits matches the millisecond the value is recorded + * at and the three the filename carries. + */ + private val TIMESTAMP_FORMAT: DateTimeFormatter = + DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss.SSSXXX", Locale.ROOT) + + /** + * The names the memory watcher is given for the three processes the IDE plots. + * + * Here rather than beside the watcher because this file is the one that cannot move: a column + * name is the file's published contract, read back by ADFA-5494 and by whoever opens the copy + * ADFA-5526 and ADFA-5534 attach to a report. Everything else looks these up. + * + * They were literals in two places -- these and `BaseEditorActivity.PROC_*` -- joined by + * nothing but string equality. Renaming a process there would have gone on writing the old + * header here and quietly emptied the column, because a name that matches nothing in the + * snapshot is written as an absent value rather than as an error. + */ + const val PROC_IDE = "IDE" + + /** @see PROC_IDE */ + const val PROC_GRADLE_TOOLING = "Gradle Tooling" + + /** @see PROC_IDE */ + const val PROC_GRADLE_DAEMON = "Gradle Daemon" + + /** + * The memory series, in column order. + * + * Fixed rather than taken from whatever is being watched at export time. The set changes during + * a session -- the Gradle daemon appears when a build starts and goes when it exits (ADFA-5514) + * -- and a header that depended on it would describe a different file each time. A process that + * is not being watched leaves its column empty. + */ + val MEMORY_COLUMNS = listOf(PROC_IDE, PROC_GRADLE_TOOLING, PROC_GRADLE_DAEMON) + + @JvmStatic + val HEADER: List = + listOf("timestamp") + + MEMORY_COLUMNS.map { "${it.lowercase().replace(' ', '_')}_pss_bytes" } + + listOf( + "net_rx_bytes", + "net_tx_bytes", + "battery_temp_millicelsius", + "power_microwatts", + "thermal_status", + "annotation", + "annotation_kind", + ) + + /** + * One series of samples and the times they were recorded at. + * + * @property times When each value was sampled, oldest first and parallel to [values]. A + * [NO_SAMPLE] entry marks an index nothing was ever recorded at, which is what tells an empty + * cell apart from a measured zero. + * @property values The samples themselves. + * @property absent The in-band value this series uses for "the device did not provide a + * reading", or `null` if it has none. Written as an empty cell, the same as an unsampled index. + * A watcher that stores a sentinel -- `PowerUsageWatcher.UNAVAILABLE` is [Long.MIN_VALUE] -- + * would otherwise put `-9223372036854775808` in a numeric column, and every consumer that + * averages or plots that column gets an answer that is not merely wrong but spectacular. The + * sentinel is named by the caller rather than known here, because this file deliberately has no + * Android types in it. + * @property since When this series started being recorded. Samples timed before it belong to + * the buffer's zero-filled past rather than to this series -- the Gradle daemon's buffer reaches + * back to the start of the session however late in it the daemon appeared. + */ + class Series( + private val times: LongArray, + private val values: LongArray, + private val since: Long = 0L, + private val absent: Long? = null, + ) { + /** The value at index [i], or `null` if this series has nothing to say there. */ + fun at(i: Int): Long? { + if (i < 0 || i >= times.size || i >= values.size) { + return null + } + val time = times[i] + if (time == NO_SAMPLE || time < since) { + return null + } + val value = values[i] + return if (value == absent) null else value + } + + companion object { + val EMPTY = Series(LongArray(0), LongArray(0)) + } + } + + /** + * @property atMillis When the event happened. + * @property label Its text, already resolved. + * @property kind The sort of event, as the annotation store names it. + */ + data class Marker( + val atMillis: Long, + val label: String, + val kind: String, + ) + + /** + * Everything one export writes. + * + * @property rowTimes The memory watcher's sample times, oldest first. They are the rows, because + * memory is the one series always being recorded. + * @property sampleIntervalMillis How often the watchers sample. Required rather than defaulted, + * because it decides which annotations are near enough to a row to be written on it and a + * default would pick that bound for a caller who never considered it. + */ + class Snapshot( + val rowTimes: LongArray, + val sampleIntervalMillis: Long, + val memory: Map, + val networkReceived: Series = Series.EMPTY, + val networkTransmitted: Series = Series.EMPTY, + val temperature: Series = Series.EMPTY, + val power: Series = Series.EMPTY, + val thermal: Series = Series.EMPTY, + val annotations: List = emptyList(), + ) + + /** + * Writes [snapshot] to [out], timestamps in [zone]. + * + * The header is always written, even when nothing has been sampled. A file that exists and + * reports no rows is easier for a consumer to handle than one that may or may not be there, and + * the empty case is not exotic: changing the sampling rate clears every buffer. + */ + fun write( + snapshot: Snapshot, + zone: ZoneId, + out: Appendable, + ) { + out.append(HEADER.joinToString(",", transform = ::quote)).append('\n') + + val markerRows = markerRows(snapshot) + val row = StringBuilder() + snapshot.rowTimes.forEachIndexed { i, at -> + if (at == NO_SAMPLE) { + return@forEachIndexed + } + + row.setLength(0) + row.append(quote(formatTime(at, zone))) + MEMORY_COLUMNS.forEach { name -> row.append(',').append(number(snapshot.memory[name]?.at(i))) } + row.append(',').append(number(snapshot.networkReceived.at(i))) + row.append(',').append(number(snapshot.networkTransmitted.at(i))) + row.append(',').append(number(snapshot.temperature.at(i))) + row.append(',').append(number(snapshot.power.at(i))) + row.append(',').append(number(snapshot.thermal.at(i))) + val marker = markerRows[i] + row.append(',').append(marker?.let { quote(it.label) } ?: "") + row.append(',').append(marker?.let { quote(it.kind) } ?: "") + out.append(row).append('\n') + } + } + + /** + * An annotation's time, moved onto the clock the samples are stamped with. + * + * [MetricsAnnotationStore] records on [android.os.SystemClock.elapsedRealtime], which is + * monotonic and immune to the wall clock being set, and is what the chart wants: it only ever + * asks how long ago something happened. A file has to say *when*, so the samples carry epoch + * milliseconds, and the two cannot be compared without this. + * + * Mixing them is not a small error. A monotonic time is a few hours since boot and an epoch time + * is decades, so every row looks about equally far from the marker and the nearest-row search + * lands on whichever row has the smallest number -- the oldest one in the buffer, every time. + * + * @param monotonicAtMillis The time the store recorded. + * @param nowEpochMillis Now, on the samples' clock. + * @param nowMonotonicMillis Now, on the store's clock. Read as close together as possible. + */ + fun epochFor( + monotonicAtMillis: Long, + nowEpochMillis: Long, + nowMonotonicMillis: Long, + ): Long = monotonicAtMillis + (nowEpochMillis - nowMonotonicMillis) + + /** [atMillis] as ISO 8601 in [zone]. */ + fun formatTime( + atMillis: Long, + zone: ZoneId, + ): String = TIMESTAMP_FORMAT.format(Instant.ofEpochMilli(atMillis).atZone(zone)) + + /** + * The row each marker belongs on, resolved once for the whole file. + * + * A marker goes on the row whose sample is nearest it in time. An annotation is recorded when + * something happened, not when a sample was taken, so requiring an exact match would drop + * almost all of them; and doing this per row rather than once would walk every marker against + * every row, which at ten thousand of each is not a cost worth paying for a button. + * + * A marker further than one sampling interval from its nearest row is dropped rather than + * pulled onto it. Sampling at a fixed interval leaves every marker that happened while the + * buffer was filling within half an interval of some sample, so a greater distance means the + * marker falls outside the sampled window -- an annotation older than the buffer reaches, which + * is the ordinary case in a long session. Without the cap every one of those lands on row 0, + * where [MutableMap.putIfAbsent] keeps the first and drops the rest: the file would carry one + * arbitrary ancient marker on its oldest row and lose the others silently. The chart has both + * guards already -- it asks the store only for the annotations in the visible span, and drops + * any whose x falls before the first sample. + * + * Where two markers land on one row the earlier wins, and the later is dropped rather than + * silently overwriting it -- the file has one annotation column per row by definition. + */ + private fun markerRows(snapshot: Snapshot): Map { + if (snapshot.annotations.isEmpty()) { + return emptyMap() + } + + val sampled = snapshot.rowTimes.withIndex().filter { it.value != NO_SAMPLE } + if (sampled.isEmpty()) { + return emptyMap() + } + + val rows = mutableMapOf() + snapshot.annotations.sortedBy { it.atMillis }.forEach { marker -> + val nearest = sampled.minByOrNull { abs(it.value - marker.atMillis) } ?: return@forEach + if (abs(nearest.value - marker.atMillis) > snapshot.sampleIntervalMillis) { + return@forEach + } + rows.putIfAbsent(nearest.index, marker) + } + return rows + } + + private fun number(value: Long?): String = value?.toString() ?: "" + + /** + * The characters that make a spreadsheet read a cell as a formula rather than as text. + * + * Tab and carriage return are here because a leading one of either is stripped on import, which + * exposes whatever follows it: a cell of "\t=cmd" is a formula too. + */ + private val FORMULA_LEAD = charArrayOf('=', '+', '-', '@', '\t', '\r') + + /** + * A CSV string cell. + * + * Quoted per the format's rule (b), with any quote inside it doubled -- a task name is text the + * IDE was given, and nothing guarantees it has no quotes in it. + * + * A cell beginning with one of [FORMULA_LEAD] additionally gets a leading apostrophe. Quoting + * alone does not stop a spreadsheet evaluating the cell on import, and the annotation columns + * carry Gradle task names taken from the user's own build script -- into a file ADFA-5526 and + * ADFA-5534 attach to crash reports and to feedback, which a support engineer then opens. The + * apostrophe is part of the cell as written, so a reader parsing this file back has to strip it. + * + * The numeric columns do not come through here. They are written by [number], where a negative + * value has to stay a number rather than become text with a quote in front of it. + */ + private fun quote(value: String): String { + val guarded = if (value.isNotEmpty() && value[0] in FORMULA_LEAD) "'" + value else value + return "\"" + guarded.replace("\"", "\"\"") + "\"" + } +} diff --git a/app/src/main/java/com/itsaky/androidide/utils/MetricsCsvFile.kt b/app/src/main/java/com/itsaky/androidide/utils/MetricsCsvFile.kt new file mode 100644 index 0000000000..81310f3182 --- /dev/null +++ b/app/src/main/java/com/itsaky/androidide/utils/MetricsCsvFile.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 android.content.Context +import androidx.annotation.VisibleForTesting +import org.slf4j.LoggerFactory +import java.io.File +import java.io.IOException +import java.time.ZoneId + +/** + * Writes a [MetricsCsv.Snapshot] to a file the IDE can share (ADFA-5531). + * + * The same scratch-directory arrangement as [MetricsSnapshot], and for the same reason: exports go + * under the cache so the platform can reclaim them, and the sharing intent grants the recipient a + * read on the file before that matters. + */ +object MetricsCsvFile { + private val log = LoggerFactory.getLogger(MetricsCsvFile::class.java) + + private const val DIRECTORY = "metrics-exports" + + /** + * How many exports to keep. + * + * A share hands the recipient a URI and returns long before the recipient reads it, so the + * previous file cannot be deleted on the next export. Fewer than the images are kept: a full + * buffer is around a megabyte of text, against a few hundred kilobytes for a PNG. + */ + @VisibleForTesting + internal const val KEEP_RECENT = 3 + + /** + * Writes [snapshot] and returns the file, or `null` if it could not be written. + */ + fun write( + context: Context, + snapshot: MetricsCsv.Snapshot, + nowMillis: Long = System.currentTimeMillis(), + zone: ZoneId = ZoneId.systemDefault(), + ): File? { + val directory = File(context.cacheDir, DIRECTORY) + return try { + if (!directory.exists() && !directory.mkdirs()) { + log.error("Could not create the metrics export directory at {}", directory) + return null + } + + val file = File(directory, MetricsFileName.forTime(nowMillis, "csv", zone)) + // Buffered and streamed rather than built into a string: a full buffer is ten thousand + // rows, and holding the whole file in memory to write it is a megabyte of char array + // the export does not need. + file.bufferedWriter().use { writer -> + MetricsCsv.write(snapshot, zone, writer) + } + MetricsSnapshot.pruneTo(directory, KEEP_RECENT, file) + file + } catch (io: IOException) { + log.error("Could not write the metrics export", io) + null + } + } +} diff --git a/app/src/main/java/com/itsaky/androidide/utils/MetricsFileName.kt b/app/src/main/java/com/itsaky/androidide/utils/MetricsFileName.kt new file mode 100644 index 0000000000..97a13b3419 --- /dev/null +++ b/app/src/main/java/com/itsaky/androidide/utils/MetricsFileName.kt @@ -0,0 +1,50 @@ +/* + * 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 java.time.Instant +import java.time.ZoneId +import java.time.format.DateTimeFormatter +import java.util.Locale + +/** + * What every exported metrics file is called (ADFA-5531). + * + * One rule for the CSV and the chart image alike, differing only in extension, so a pair exported + * together sorts together and a consumer can tell when a file was written without opening it. + * + * Underscores and no offset, which is what makes it a filename rather than a timestamp: it has to + * survive every filesystem the IDE can write to and sort lexicographically in a directory listing. + * The times *inside* the file are ISO 8601 -- see [MetricsCsv]. + */ +object MetricsFileName { + /** `YYYY_MM_DD_HH_MM_SS_SSS`, as the format section of ADFA-5531 specifies it. */ + private val PATTERN: DateTimeFormatter = + DateTimeFormatter.ofPattern("yyyy_MM_dd_HH_mm_ss_SSS", Locale.ROOT) + + /** + * The name for a file written at [atMillis], with [extension] and no leading dot. + * + * Local time, because this is the name a person reads in a share sheet or a file manager. + */ + fun forTime( + atMillis: Long, + extension: String, + zone: ZoneId = ZoneId.systemDefault(), + ): String = "${PATTERN.format(Instant.ofEpochMilli(atMillis).atZone(zone))}.$extension" +} diff --git a/app/src/main/java/com/itsaky/androidide/utils/MetricsSnapshot.kt b/app/src/main/java/com/itsaky/androidide/utils/MetricsSnapshot.kt index 8f1f07d0b5..113b9c89fe 100644 --- a/app/src/main/java/com/itsaky/androidide/utils/MetricsSnapshot.kt +++ b/app/src/main/java/com/itsaky/androidide/utils/MetricsSnapshot.kt @@ -23,9 +23,6 @@ import androidx.annotation.VisibleForTesting import org.slf4j.LoggerFactory import java.io.File import java.io.IOException -import java.text.SimpleDateFormat -import java.util.Date -import java.util.Locale /** * Writes a metrics chart image to a file the IDE can share (ADFA-5486). @@ -47,13 +44,16 @@ object MetricsSnapshot { */ @VisibleForTesting internal const val KEEP_RECENT = 5 - private const val TIMESTAMP_PATTERN = "yyyyMMdd-HHmmss" /** Media type for the written file, for the sharing intent. */ const val MIME_TYPE = "image/png" /** - * Writes [bitmap] as a PNG named after [label] and the current time. + * Writes [bitmap] as a PNG, named by [MetricsFileName] like every other exported metrics file. + * + * The name used to lead with the chart's title. ADFA-5531 made one naming rule for the image and + * the CSV so that a pair exported together sorts together, and a title in front of the timestamp + * would have sorted them apart. * * A few recent snapshots are kept rather than only the newest. This is a scratch directory for * handing an image to another app, not a gallery, so it stays bounded -- but a share hands the @@ -66,7 +66,7 @@ object MetricsSnapshot { fun write( context: Context, bitmap: Bitmap, - label: String, + nowMillis: Long = System.currentTimeMillis(), ): File? { val directory = File(context.cacheDir, DIRECTORY) return try { @@ -75,7 +75,7 @@ object MetricsSnapshot { return null } - val file = File(directory, "${fileNameFor(label)}.png") + val file = File(directory, MetricsFileName.forTime(nowMillis, "png")) file.outputStream().use { output -> if (!bitmap.compress(Bitmap.CompressFormat.PNG, QUALITY, output)) { log.error("Could not encode the chart snapshot") @@ -97,34 +97,24 @@ object MetricsSnapshot { * trusted to sort newest: two exports in the same second share a timestamp, and the filename * carries only whole seconds. */ - private fun pruneTo( + internal fun pruneTo( directory: File, limit: Int, newest: File, ) { - val files = directory.listFiles()?.sortedBy { it.lastModified() } ?: return - if (files.size <= limit) { + // [newest] is excluded from the candidates rather than skipped among them. Skipping it after + // choosing "the oldest n" left one file too many whenever it sorted into that set, and the + // directory then crept one over the limit per collision. Two writes inside one filesystem + // timestamp are enough to sort it there. + val candidates = directory.listFiles()?.filter { it != newest }?.sortedBy { it.lastModified() } ?: return + val excess = candidates.size - (limit - 1) + if (excess <= 0) { return } - files.take(files.size - limit).forEach { file -> - if (file != newest && !file.delete()) { + candidates.take(excess).forEach { file -> + if (!file.delete()) { log.warn("Could not delete the stale chart snapshot at {}", file) } } } - - /** - * A filename from [label] and the current time, with anything that is not safe in a filename - * replaced. Chart titles are translated, so they can contain spaces and non-ASCII. - */ - private fun fileNameFor(label: String): String { - val stamp = SimpleDateFormat(TIMESTAMP_PATTERN, Locale.US).format(Date()) - val safeLabel = - label - .lowercase(Locale.US) - .replace(Regex("[^a-z0-9]+"), "-") - .trim('-') - .ifEmpty { "metrics" } - return "$safeLabel-$stamp" - } } 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 784f13d286..3be34da8f1 100644 --- a/app/src/main/java/com/itsaky/androidide/utils/NetworkUsageWatcher.kt +++ b/app/src/main/java/com/itsaky/androidide/utils/NetworkUsageWatcher.kt @@ -68,6 +68,7 @@ class NetworkUsageWatcher // touching Dispatchers.Main at construction throws in a plain JVM test, and most of these // tests never start the sampling loop at all. private val mainDispatcher: CoroutineContext? = null, + private val nowMillis: () -> Long = System::currentTimeMillis, ) { private val coroutineScope = CoroutineScope(SupervisorJob() + coroutineDispatcher) private val watching = AtomicBoolean(false) @@ -103,6 +104,21 @@ class NetworkUsageWatcher /** Guards the two ring buffers: the sampler writes them, the UI thread snapshots them. */ private val historyLock = Any() + /** + * When each sample was taken, in the same order and at the same indices as the values. + * + * Recorded rather than reconstructed. The chart infers a sample's age from its position, + * which is close enough for placing a marker on a plot, but the exported metrics file states + * a time per row (ADFA-5531) and inference would be wrong three ways: the newest sample was + * taken up to an interval before the export, the loop delays *after* doing its work so the + * true period drifts past the nominal one, and sampling can stop and restart without the + * buffer being cleared. + * + * A zero means no sample was ever recorded at that index, which is what tells a blank cell + * apart from a measured zero. + */ + private val sampleTimes = MutableShiftedLongArray(MAX_USAGE_ENTRIES) + private val received = MutableShiftedLongArray(MAX_USAGE_ENTRIES) private val transmitted = MutableShiftedLongArray(MAX_USAGE_ENTRIES) @@ -140,7 +156,7 @@ class NetworkUsageWatcher */ fun getUsage(): NetworkUsage = synchronized(historyLock) { - NetworkUsage(received.toLongArray(), transmitted.toLongArray()) + NetworkUsage(received.toLongArray(), transmitted.toLongArray(), sampleTimes.toLongArray()) } /** @@ -151,6 +167,7 @@ class NetworkUsageWatcher synchronized(historyLock) { received.clear() transmitted.clear() + sampleTimes.clear() lastRx = null lastTx = null } @@ -265,6 +282,8 @@ class NetworkUsageWatcher // and the second block then put the pre-reset values straight back, so the next // sample counted traffic from before the change. synchronized(historyLock) { + sampleTimes[0] = nowMillis() + sampleTimes.shift(1) record(received, previous = lastRx, current = rx) record(transmitted, previous = lastTx, current = tx) lastRx = rx @@ -302,20 +321,36 @@ class NetworkUsageWatcher * * @property received Bytes received during each interval. * @property transmitted Bytes transmitted during each interval. + * @property sampleTimes When each sample was taken, oldest first, as milliseconds since the + * epoch, parallel to the values. Read in the same critical section as them, because reading + * the two separately let the sampler append between the calls and shifted every value one + * index against its timestamp (ADFA-5531). A zero means nothing was ever sampled at that + * index -- the buffers are fixed-length and start, and are cleared, full of them. + * + * Required, with no empty default. A caller that omitted it produced a history whose every + * sample read as never-taken, which the chart cannot see -- it asks only how long ago a + * sample was -- but which silently emptied both of this watcher's columns in the CSV. */ data class NetworkUsage( val received: LongArray, val transmitted: LongArray, + val sampleTimes: LongArray, ) { override fun equals(other: Any?): Boolean = this === other || ( other is NetworkUsage && received.contentEquals(other.received) && - transmitted.contentEquals(other.transmitted) + transmitted.contentEquals(other.transmitted) && + sampleTimes.contentEquals(other.sampleTimes) ) - override fun hashCode(): Int = 31 * received.contentHashCode() + transmitted.contentHashCode() + override fun hashCode(): Int { + var result = received.contentHashCode() + result = 31 * result + transmitted.contentHashCode() + result = 31 * result + sampleTimes.contentHashCode() + return result + } } fun interface NetworkUsageListener { 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 94a9577249..d6fd12025f 100644 --- a/app/src/main/java/com/itsaky/androidide/utils/PowerUsageWatcher.kt +++ b/app/src/main/java/com/itsaky/androidide/utils/PowerUsageWatcher.kt @@ -58,6 +58,7 @@ class PowerUsageWatcher private val source: PowerSource, private val coroutineDispatcher: CoroutineContext = newSingleThreadContext("PowerUsageWatcher"), private val mainDispatcher: CoroutineContext = Dispatchers.Main.immediate, + private val nowMillis: () -> Long = System::currentTimeMillis, ) { private val coroutineScope = CoroutineScope(SupervisorJob() + coroutineDispatcher) private val watching = AtomicBoolean(false) @@ -75,10 +76,26 @@ class PowerUsageWatcher /** Guards the ring buffers: the sampler writes them, the UI thread snapshots them. */ private val historyLock = Any() + /** + * When each sample was taken, in the same order and at the same indices as the values. + * + * Recorded rather than reconstructed. The chart infers a sample's age from its position, + * which is close enough for placing a marker on a plot, but the exported metrics file states + * a time per row (ADFA-5531) and inference would be wrong three ways: the newest sample was + * taken up to an interval before the export, the loop delays *after* doing its work so the + * true period drifts past the nominal one, and sampling can stop and restart without the + * buffer being cleared. + * + * A zero means no sample was ever recorded at that index, which is what tells a blank cell + * apart from a measured zero. Zero is safe as the sentinel here, unlike the value series + * below: no real sample was taken at the epoch. + */ + private val sampleTimes = MutableShiftedLongArray(MAX_USAGE_ENTRIES) + // Filled with UNAVAILABLE, not zero. A slot that has never been sampled is an absence, and // zero is a reading: a zero-filled prefix plotted a flat 0 C and 0 W line and presented it // as measurement, which then forced applyAxisRanges to special-case `!= 0L` -- discarding - // a genuine freezing-battery sample along with the fake ones. + // a genuine freezing-battery sample along with the fake ones (ADFA-5499). private val temperature = MutableShiftedLongArray(MAX_USAGE_ENTRIES) { UNAVAILABLE } private val power = MutableShiftedLongArray(MAX_USAGE_ENTRIES) { UNAVAILABLE } @@ -129,11 +146,17 @@ class PowerUsageWatcher */ fun getUsage(): PowerUsage = synchronized(historyLock) { - PowerUsage(temperature.toLongArray(), power.toLongArray(), thermal.toLongArray()) + PowerUsage( + temperature.toLongArray(), + power.toLongArray(), + thermal.toLongArray(), + sampleTimes.toLongArray(), + ) } fun clearHistory() { synchronized(historyLock) { + sampleTimes.clear() temperature.clear(UNAVAILABLE) power.clear(UNAVAILABLE) thermal.clear(UNAVAILABLE) @@ -199,6 +222,7 @@ class PowerUsageWatcher latestBattery = reading.battery synchronized(historyLock) { + append(sampleTimes, nowMillis()) append(temperature, reading.temperatureMilliCelsius) append(power, reading.powerMicroWatts) append(thermal, reading.thermalStatus.toLong()) @@ -260,11 +284,21 @@ class PowerUsageWatcher * @property temperatureMilliCelsius Battery temperature per sample. * @property powerMicroWatts Instantaneous draw per sample. * @property thermalStatus Throttling level per sample, for the chart's shading. + * @property sampleTimes When each sample was taken, oldest first, as milliseconds since the + * epoch, parallel to the values. Read in the same critical section as them, because reading + * the two separately let the sampler append between the calls and shifted every value one + * index against its timestamp (ADFA-5531). A zero means nothing was ever sampled at that + * index -- the buffers are fixed-length and start, and are cleared, full of them. + * + * Required, with no empty default. A caller that omitted it produced a history whose every + * sample read as never-taken, which the chart cannot see -- it asks only how long ago a + * sample was -- but which silently emptied every one of this watcher's columns in the CSV. */ data class PowerUsage( val temperatureMilliCelsius: LongArray, val powerMicroWatts: LongArray, val thermalStatus: LongArray, + val sampleTimes: LongArray, ) { override fun equals(other: Any?): Boolean = this === other || @@ -272,13 +306,15 @@ class PowerUsageWatcher other is PowerUsage && temperatureMilliCelsius.contentEquals(other.temperatureMilliCelsius) && powerMicroWatts.contentEquals(other.powerMicroWatts) && - thermalStatus.contentEquals(other.thermalStatus) + thermalStatus.contentEquals(other.thermalStatus) && + sampleTimes.contentEquals(other.sampleTimes) ) override fun hashCode(): Int { var result = temperatureMilliCelsius.contentHashCode() result = 31 * result + powerMicroWatts.contentHashCode() result = 31 * result + thermalStatus.contentHashCode() + result = 31 * result + sampleTimes.contentHashCode() return result } } diff --git a/app/src/main/res/drawable/ic_spreadsheet.xml b/app/src/main/res/drawable/ic_spreadsheet.xml new file mode 100644 index 0000000000..db16b3dd48 --- /dev/null +++ b/app/src/main/res/drawable/ic_spreadsheet.xml @@ -0,0 +1,25 @@ + + + + + + + + + diff --git a/app/src/main/res/layout/layout_mem_usage.xml b/app/src/main/res/layout/layout_mem_usage.xml index f267ae6466..aee15c1e87 100644 --- a/app/src/main/res/layout/layout_mem_usage.xml +++ b/app/src/main/res/layout/layout_mem_usage.xml @@ -88,6 +88,20 @@ app:layout_constraintBottom_toBottomOf="@id/metrics_pager" app:layout_constraintEnd_toEndOf="@id/metrics_pager" /> + + + () + // No sample times: these tests are about what the chart draws, and the chart asks only how long + // ago a sample was. Stated rather than defaulted, because the same emptiness in production + // silently blanks the CSV's network columns. private fun usage( received: LongArray, transmitted: LongArray = received, - ) = NetworkUsageWatcher.NetworkUsage(received, transmitted) + ) = NetworkUsageWatcher.NetworkUsage(received, transmitted, LongArray(received.size)) private fun rendererFor(usage: NetworkUsageWatcher.NetworkUsage): Pair { val chart = SafeLineChart(context) diff --git a/app/src/test/java/com/itsaky/androidide/ui/PowerUsageChartRendererTest.kt b/app/src/test/java/com/itsaky/androidide/ui/PowerUsageChartRendererTest.kt index 8e03637b38..f1738a164e 100644 --- a/app/src/test/java/com/itsaky/androidide/ui/PowerUsageChartRendererTest.kt +++ b/app/src/test/java/com/itsaky/androidide/ui/PowerUsageChartRendererTest.kt @@ -40,11 +40,13 @@ import org.robolectric.RobolectricTestRunner class PowerUsageChartRendererTest { private val context = ApplicationProvider.getApplicationContext() + // No sample times, for the reason NetworkUsageChartRendererTest gives: a chart test says so + // rather than letting a default say it. private fun usage( temperature: LongArray, power: LongArray = LongArray(temperature.size), thermal: LongArray = LongArray(temperature.size), - ) = PowerUsageWatcher.PowerUsage(temperature, power, thermal) + ) = PowerUsageWatcher.PowerUsage(temperature, power, thermal, LongArray(temperature.size)) private fun rendererFor( usage: PowerUsageWatcher.PowerUsage, diff --git a/app/src/test/java/com/itsaky/androidide/utils/MemoryUsageWatcherSampleAlignmentTest.kt b/app/src/test/java/com/itsaky/androidide/utils/MemoryUsageWatcherSampleAlignmentTest.kt new file mode 100644 index 0000000000..5b7e2a21aa --- /dev/null +++ b/app/src/test/java/com/itsaky/androidide/utils/MemoryUsageWatcherSampleAlignmentTest.kt @@ -0,0 +1,139 @@ +/* + * 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.robolectric.RobolectricTestRunner + +/** + * That a row of the exported metrics file is one moment (ADFA-5531). + * + * The exported file states a time per row, and every value on that row has to be the one recorded + * at it. The sampler runs on its own thread and the export reads from the UI thread, so the only + * thing making that true is where the sampler's appends happen and how many calls the reader makes. + */ +@RunWith(RobolectricTestRunner::class) +class MemoryUsageWatcherSampleAlignmentTest { + private var clock = 1_700_000_000_000L + + private fun watcher(readPssKb: (Int, android.os.Debug.MemoryInfo) -> Int = { _, _ -> PSS_KB }) = + MemoryUsageWatcher( + nowMillis = { + clock += TICK_MILLIS + clock + }, + readTotalPssKb = readPssKb, + ) + + @Test + fun `a read taken during a sample sees times and values that agree`() { + lateinit var watcher: MemoryUsageWatcher + var midSample: MemoryUsageWatcher.MemoryHistory? = null + + // Read from inside the sample, which is the interleaving the sampling thread and the UI + // thread can produce for real. Appending the time and the values in separate critical + // sections left this window: the reader caught a buffer with one more timestamp in it than + // values, so every value in the exported file sat on the row below its own timestamp. + watcher = + watcher { _, _ -> + if (midSample == null) { + midSample = watcher.history() + } + PSS_KB + } + watcher.watchProcess(PID, "IDE") + + watcher.readUsages() + watcher.readUsages() + + val history = checkNotNull(midSample) + val stamped = history.times.count { it != MetricsCsv.NO_SAMPLE } + val measured = + history.processes + .single() + .usage + .count { it != 0L } + assertThat(measured).isEqualTo(stamped) + } + + @Test + fun `a completed sample stamps every process with the same time`() { + val watcher = watcher() + watcher.watchProcess(PID, "IDE") + watcher.watchProcess(OTHER_PID, "Gradle Daemon") + + watcher.readUsages() + + // The two processes are read one after the other, but they belong to one row, so they share + // its time -- and each has exactly one value against it. + val history = watcher.history() + assertThat(history.times.count { it != MetricsCsv.NO_SAMPLE }).isEqualTo(1) + history.processes.forEach { process -> + assertThat(process.usage.count { it != 0L }).isEqualTo(1) + } + } + + @Test + fun `the times come back with the values, not from a call of their own`() { + val watcher = watcher() + watcher.watchProcess(PID, "IDE") + watcher.readUsages() + + // The guard on the fix above: one accessor, so a caller cannot reintroduce the window by + // asking for the halves separately. There is deliberately no times-only accessor. + val history = watcher.history() + assertThat(history.times).hasLength(MemoryUsageWatcher.MAX_USAGE_ENTRIES) + assertThat(history.processes.single().usage).hasLength(MemoryUsageWatcher.MAX_USAGE_ENTRIES) + assertThat(history.times.last()).isNotEqualTo(MetricsCsv.NO_SAMPLE) + assertThat( + history.processes + .single() + .usage + .last(), + ).isEqualTo(PSS_KB * 1024L) + } + + @Test + fun `a copied process still says when it started being watched`() { + val watcher = watcher() + watcher.watchProcess(PID, "Gradle Daemon") + watcher.readUsages() + + // getMemoryUsages hands out copies, and the copy used to drop watchedSinceMillis -- which + // defaults to 0, i.e. "watched since the epoch". The export's guard for a process's + // zero-filled past then never fired, so the daemon's buffer from before the daemon existed + // came out as measured zeros rather than empty cells (ADFA-5531). + val copied = watcher.getMemoryUsages().single() + assertThat(copied.watchedSinceMillis).isNotEqualTo(0L) + assertThat(copied.watchedSinceMillis).isEqualTo(watcher.getMemoryUsage(PID)!!.watchedSinceMillis) + } + + private companion object { + const val PID = 4242 + + const val OTHER_PID = 4243 + + /** Any non-zero reading; the test counts measured samples rather than reading values. */ + const val PSS_KB = 512 + + /** Enough that no two sample times collide. */ + const val TICK_MILLIS = 1_000L + } +} diff --git a/app/src/test/java/com/itsaky/androidide/utils/MetricsCsvFileTest.kt b/app/src/test/java/com/itsaky/androidide/utils/MetricsCsvFileTest.kt new file mode 100644 index 0000000000..61e2aad19d --- /dev/null +++ b/app/src/test/java/com/itsaky/androidide/utils/MetricsCsvFileTest.kt @@ -0,0 +1,110 @@ +/* + * 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 android.content.Context +import androidx.test.core.app.ApplicationProvider +import com.google.common.truth.Truth.assertThat +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import java.io.File +import java.time.ZoneId + +/** + * The file the export button writes (ADFA-5531). + * + * [MetricsCsvFile.KEEP_RECENT] and the clock were made injectable for a test and then had none, so + * the bound on the directory and the name the file is given were both unasserted. + */ +@RunWith(RobolectricTestRunner::class) +class MetricsCsvFileTest { + private val context = ApplicationProvider.getApplicationContext() + + /** + * Fixed, not the machine's own. + * + * The name asserted below is a rendering of [AT] in a particular zone, so leaving the zone to + * the default made this pass here and fail wherever CI happens to be. + */ + private val zone: ZoneId = ZoneId.of("America/Los_Angeles") + + private fun snapshot(rows: Int): MetricsCsv.Snapshot { + val times = LongArray(rows) { AT + it * 1_000L } + return MetricsCsv.Snapshot( + rowTimes = times, + sampleIntervalMillis = INTERVAL_MS, + memory = mapOf(MetricsCsv.PROC_IDE to MetricsCsv.Series(times, LongArray(rows) { 600_000_000L + it })), + ) + } + + @Test + fun `an export is plain csv the user can open`() { + val file = MetricsCsvFile.write(context, snapshot(3), AT, zone)!! + + assertThat(file.name).isEqualTo("2026_09_06_22_33_40_123.csv") + assertThat(file.readText().lineSequence().first()).startsWith("\"timestamp\"") + } + + @Test + fun `the export directory stays bounded`() { + repeat(MetricsCsvFile.KEEP_RECENT + 4) { i -> + MetricsCsvFile.write(context, snapshot(1), AT + i * 1_000L, zone) + } + + val directory = MetricsCsvFile.write(context, snapshot(1), AT + 90_000L, zone)!!.parentFile!! + + assertThat(directory.listFiles()!!.size).isAtMost(MetricsCsvFile.KEEP_RECENT) + } + + @Test + fun `the limit holds when the file just written is not the newest on disk`() { + // Pruning used to pick "the oldest n" across every file and then skip the one just written, + // which deleted one too few whenever that one sorted into the set -- and the directory crept + // one over the limit each time. Two writes inside a single filesystem timestamp are enough + // to sort it there. + // + // Dating the existing files into the future is what puts the new one at the front of the + // sort deterministically. Tying them all to one *past* value does not: the file written last + // still carries a real mtime, so it sorts last, is never in the set, and the skip never + // fires -- which is how the first version of this test passed against the unfixed code. + val future = System.currentTimeMillis() + 1_000_000L + repeat(MetricsCsvFile.KEEP_RECENT + 3) { i -> + MetricsCsvFile.write(context, snapshot(1), AT + i, zone)!!.setLastModified(future) + } + + val directory = MetricsCsvFile.write(context, snapshot(1), AT + 900L, zone)!!.parentFile!! + + assertThat(directory.listFiles()!!.size).isAtMost(MetricsCsvFile.KEEP_RECENT) + } + + @Test + fun `files land under the cache, which the platform may reclaim`() { + val file: File = MetricsCsvFile.write(context, snapshot(2), AT, zone)!! + + assertThat(file.absolutePath).startsWith(context.cacheDir.absolutePath) + } + + private companion object { + /** 2026-09-06T22:33:40.123 local. */ + const val AT = 1_788_759_220_123L + + /** The gap between the rows these fixtures build. */ + const val INTERVAL_MS = 1_000L + } +} diff --git a/app/src/test/java/com/itsaky/androidide/utils/MetricsCsvTest.kt b/app/src/test/java/com/itsaky/androidide/utils/MetricsCsvTest.kt new file mode 100644 index 0000000000..22886b6128 --- /dev/null +++ b/app/src/test/java/com/itsaky/androidide/utils/MetricsCsvTest.kt @@ -0,0 +1,417 @@ +/* + * 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 +import java.time.ZoneId + +/** + * The canonical metrics file format (ADFA-5531). + * + * Pinned closely because it is not this ticket's file alone: ADFA-5494 reads it back to restore the + * chart history, and ADFA-5526 and ADFA-5534 attach copies to crash reports and to feedback. A + * column that quietly changes shape breaks a consumer that is not in front of you. + */ +@RunWith(JUnit4::class) +class MetricsCsvTest { + private val zone: ZoneId = ZoneId.of("America/Los_Angeles") + + private fun render(snapshot: MetricsCsv.Snapshot): List = + StringBuilder() + .also { MetricsCsv.write(snapshot, zone, it) } + .toString() + .trimEnd('\n') + .split('\n') + + private fun snapshot( + rowTimes: LongArray = longArrayOf(T0, T0 + 1_000L), + sampleIntervalMillis: Long = INTERVAL_MS, + memory: Map = emptyMap(), + networkReceived: MetricsCsv.Series = MetricsCsv.Series.EMPTY, + networkTransmitted: MetricsCsv.Series = MetricsCsv.Series.EMPTY, + temperature: MetricsCsv.Series = MetricsCsv.Series.EMPTY, + power: MetricsCsv.Series = MetricsCsv.Series.EMPTY, + thermal: MetricsCsv.Series = MetricsCsv.Series.EMPTY, + annotations: List = emptyList(), + ) = MetricsCsv.Snapshot( + rowTimes = rowTimes, + sampleIntervalMillis = sampleIntervalMillis, + memory = memory, + networkReceived = networkReceived, + networkTransmitted = networkTransmitted, + temperature = temperature, + power = power, + thermal = thermal, + annotations = annotations, + ) + + private fun series( + times: LongArray, + values: LongArray, + since: Long = 0L, + ) = MetricsCsv.Series(times, values, since) + + @Test + fun `an export with nothing sampled is a header and no rows`() { + // The empty case is not exotic: changing the sampling rate clears every buffer, so the very + // next export has nothing to say. It still produces a file. + val lines = render(snapshot(rowTimes = LongArray(4))) + + assertThat(lines).hasSize(1) + assertThat(lines.single()).isEqualTo(EXPECTED_HEADER) + } + + @Test + fun `a row's time is the one recorded for that sample`() { + val lines = render(snapshot(rowTimes = longArrayOf(T0))) + + // Read back, not reconstructed from an index and an interval: 2026-09-06T22:33:40.123 in + // Los Angeles, with the offset that says which 22:33 it was. + assertThat(lines[1]).startsWith("\"2026-09-06T22:33:40.123-07:00\"") + } + + @Test + fun `the fraction is always three digits, even when it ends in zero`() { + // ISO_OFFSET_DATE_TIME drops trailing zeros and would write ".38" here, giving a column of + // varying width. Both parse; only one lines up with the milliseconds the filename carries. + val lines = render(snapshot(rowTimes = longArrayOf(T0 - 43L))) + + assertThat(lines[1]).startsWith("\"2026-09-06T22:33:40.080-07:00\"") + } + + @Test + fun `an index nothing was sampled at is not a row`() { + // The buffers are fixed-length and start full of zeros, so most of a young session's buffer + // has never been written. Those are absent rows, not rows of zeros. + val lines = render(snapshot(rowTimes = longArrayOf(0L, 0L, T0, 0L, T0 + 1_000L))) + + assertThat(lines).hasSize(3) + assertThat(lines[1]).contains("22:33:40.123") + assertThat(lines[2]).contains("22:33:41.123") + } + + @Test + fun `every row has as many cells as the header`() { + val times = longArrayOf(T0, T0 + 1_000L) + val lines = + render( + snapshot( + rowTimes = times, + memory = mapOf("IDE" to series(times, longArrayOf(1L, 2L))), + networkReceived = series(times, longArrayOf(3L, 4L)), + annotations = listOf(MetricsCsv.Marker(T0, "assemble", "TASK")), + ), + ) + + lines.forEach { line -> + assertThat(cellsIn(line)).hasSize(MetricsCsv.HEADER.size) + } + } + + @Test + fun `a process that was not being watched yet leaves the cell empty, not zero`() { + val times = longArrayOf(T0, T0 + 1_000L) + val lines = + render( + snapshot( + rowTimes = times, + // The Gradle daemon appears when a build starts, and its buffer is zero-filled + // back to the beginning of the session (ADFA-5514). Reporting those zeros as + // measurements would say the daemon was running and using nothing. + memory = mapOf("Gradle Daemon" to series(times, longArrayOf(0L, 900L), since = T0 + 1_000L)), + ), + ) + + val daemon = MetricsCsv.HEADER.indexOf("gradle_daemon_pss_bytes") + assertThat(cellsIn(lines[1])[daemon]).isEmpty() + assertThat(cellsIn(lines[2])[daemon]).isEqualTo("900") + } + + @Test + fun `a series that never recorded leaves its columns empty`() { + val times = longArrayOf(T0, T0 + 1_000L) + val lines = + render( + snapshot( + rowTimes = times, + memory = mapOf("IDE" to series(times, longArrayOf(5L, 6L))), + // A device whose traffic counters are unsupported never records a sample, and + // zero bytes transferred is a different statement from no measurement. + networkReceived = MetricsCsv.Series.EMPTY, + ), + ) + + val rx = MetricsCsv.HEADER.indexOf("net_rx_bytes") + assertThat(cellsIn(lines[1])[rx]).isEmpty() + } + + @Test + fun `strings are quoted, numbers are not, and a quote inside one is doubled`() { + val times = longArrayOf(T0) + val lines = + render( + snapshot( + rowTimes = times, + memory = mapOf("IDE" to series(times, longArrayOf(7L))), + annotations = listOf(MetricsCsv.Marker(T0, ":app:say \"hi\"", "TASK")), + ), + ) + + val cells = cellsIn(lines[1]) + assertThat(cells[MetricsCsv.HEADER.indexOf("ide_pss_bytes")]).isEqualTo("7") + assertThat(cells[MetricsCsv.HEADER.indexOf("annotation")]).isEqualTo("\":app:say \"\"hi\"\"\"") + assertThat(cells[MetricsCsv.HEADER.indexOf("annotation_kind")]).isEqualTo("\"TASK\"") + } + + @Test + fun `a cell a spreadsheet would run as a formula is prefixed`() { + // The annotation columns carry Gradle task names read from the user's own build script, and + // this file is attached to crash reports (ADFA-5526) and feedback (ADFA-5534) that someone + // opens. Quoting alone does not stop the evaluation; the apostrophe does. + val column = MetricsCsv.HEADER.indexOf("annotation") + listOf("=1+1", "+1", "-1", "@SUM(A1)", "\tlater", "\rlater").forEach { label -> + val lines = + render( + snapshot( + rowTimes = longArrayOf(T0), + annotations = listOf(MetricsCsv.Marker(T0, label, "TASK")), + ), + ) + + assertThat(cellsIn(lines[1])[column]).isEqualTo("\"'" + label + "\"") + } + } + + @Test + fun `an ordinary label is not prefixed`() { + // The guard has to be narrow: prefixing every cell would put an apostrophe in front of every + // task name a reader sees. + val lines = + render( + snapshot( + rowTimes = longArrayOf(T0), + annotations = listOf(MetricsCsv.Marker(T0, ":app:assembleV8Debug", "TASK")), + ), + ) + + assertThat(cellsIn(lines[1])[MetricsCsv.HEADER.indexOf("annotation")]) + .isEqualTo("\":app:assembleV8Debug\"") + } + + @Test + fun `an annotation further than one interval from every row is dropped`() { + // An annotation older than the buffer reaches is the ordinary case in a long session: the + // store keeps its own history and the ring buffer has already rolled past it. + val lines = + render( + snapshot( + rowTimes = longArrayOf(T0, T0 + INTERVAL_MS), + annotations = listOf(MetricsCsv.Marker(T0 - 60_000L, "Build started", "BUILD_STARTED")), + ), + ) + + val column = MetricsCsv.HEADER.indexOf("annotation") + assertThat(lines.drop(1).map { cellsIn(it)[column] }).containsExactly("", "") + } + + @Test + fun `a stale annotation does not take the first row from a real one`() { + // Both markers' nearest row is the first one. Sorted by time the stale one comes first, so + // without the cap it took the row and putIfAbsent then dropped the marker that actually + // belongs there -- the file gained an ancient annotation on its oldest row and lost a real + // one, with nothing to say either had happened. + val lines = + render( + snapshot( + rowTimes = longArrayOf(T0, T0 + INTERVAL_MS), + annotations = + listOf( + MetricsCsv.Marker(T0 - 60 * 60 * 1000L, "stale", "TASK"), + MetricsCsv.Marker(T0 + 10L, "real", "TASK"), + ), + ), + ) + + assertThat(cellsIn(lines[1])[MetricsCsv.HEADER.indexOf("annotation")]).isEqualTo("\"real\"") + } + + @Test + fun `an annotation lands on the sample nearest in time, not only an exact match`() { + val times = longArrayOf(T0, T0 + 1_000L, T0 + 2_000L) + val lines = + render( + snapshot( + rowTimes = times, + // Recorded when the build started, which is between two samples. Requiring an + // exact match would drop nearly every marker in the file. + annotations = listOf(MetricsCsv.Marker(T0 + 1_600L, "Build started", "BUILD_STARTED")), + ), + ) + + val column = MetricsCsv.HEADER.indexOf("annotation") + assertThat(cellsIn(lines[1])[column]).isEmpty() + assertThat(cellsIn(lines[2])[column]).isEmpty() + assertThat(cellsIn(lines[3])[column]).isEqualTo("\"Build started\"") + } + + @Test + fun `two annotations falling on one sample keep the earlier one`() { + val times = longArrayOf(T0) + val lines = + render( + snapshot( + rowTimes = times, + annotations = + listOf( + MetricsCsv.Marker(T0 + 40L, "second", "TASK"), + MetricsCsv.Marker(T0 + 10L, "first", "TASK"), + ), + ), + ) + + // One annotation column per row by definition, so the loser is dropped rather than + // overwriting the winner or being appended into the same cell. + assertThat(cellsIn(lines[1])[MetricsCsv.HEADER.indexOf("annotation")]).isEqualTo("\"first\"") + } + + @Test + fun `an annotation recorded on the monotonic clock lands on the right row`() { + // The store stamps annotations with elapsedRealtime and the samples carry epoch millis. + // Recorded three seconds ago, on a device up for two hours. + val upFor = 2 * 60 * 60 * 1000L + val recordedAt = upFor - 3_000L + val times = longArrayOf(T0, T0 + 1_000L, T0 + 2_000L, T0 + 3_000L) + val onEpoch = MetricsCsv.epochFor(recordedAt, nowEpochMillis = T0 + 3_000L, nowMonotonicMillis = upFor) + + val lines = + render( + snapshot( + rowTimes = times, + annotations = listOf(MetricsCsv.Marker(onEpoch, "Build started", "BUILD_STARTED")), + ), + ) + + val column = MetricsCsv.HEADER.indexOf("annotation") + assertThat(cellsIn(lines[1])[column]).isEqualTo("\"Build started\"") + } + + @Test + fun `an unconverted monotonic time reaches no row at all`() { + // What the mix-up looks like on a device: a monotonic time is a few hours and an epoch time + // is decades, so every row is about equally far away. Before the distance cap the + // nearest-row search picked whichever number was smallest -- the oldest sample, whenever + // the event really happened -- and wrote the marker there. Now it is further from every row + // than a sampling interval, so it is dropped, and the file loses it rather than lying about + // when it happened. Either way [MetricsCsv.epochFor] is what makes it land correctly. + val times = longArrayOf(T0, T0 + 1_000L, T0 + 2_000L) + val lines = + render( + snapshot( + rowTimes = times, + annotations = listOf(MetricsCsv.Marker(2 * 60 * 60 * 1000L, "Build started", "BUILD_STARTED")), + ), + ) + + val column = MetricsCsv.HEADER.indexOf("annotation") + assertThat(lines.drop(1).map { cellsIn(it)[column] }).containsExactly("", "", "") + } + + @Test + fun `the memory columns are the three the chart can plot`() { + // Fixed, not derived from what is being watched: the set changes mid-session, and a header + // that followed it would describe a different file each time. + assertThat(MetricsCsv.MEMORY_COLUMNS).containsExactly("IDE", "Gradle Tooling", "Gradle Daemon").inOrder() + assertThat(MetricsCsv.HEADER).containsAtLeast("ide_pss_bytes", "gradle_tooling_pss_bytes", "gradle_daemon_pss_bytes") + } + + /** Splits a row on commas that are not inside a quoted cell. */ + private fun cellsIn(line: String): List { + val cells = mutableListOf() + val cell = StringBuilder() + var quoted = false + line.forEach { c -> + when { + c == '"' -> { + quoted = !quoted + cell.append(c) + } + + c == ',' && !quoted -> { + cells += cell.toString() + cell.setLength(0) + } + + else -> { + cell.append(c) + } + } + } + cells += cell.toString() + return cells + } + + @Test + fun `a reading the device does not provide is an empty cell, not a sentinel`() { + val times = longArrayOf(T0, T0 + 1_000L) + val lines = + render( + snapshot( + rowTimes = times, + temperature = + MetricsCsv.Series( + times, + longArrayOf(Long.MIN_VALUE, 31_500L), + absent = Long.MIN_VALUE, + ), + ), + ) + + // PowerUsageWatcher stores Long.MIN_VALUE for a reading the platform will not give. Written + // straight out, a numeric column gets -9223372036854775808, and anything that averages or + // plots it -- ADFA-5494 reads this format back -- gets an answer that is not merely wrong + // but spectacular. Empty is what the format already means by "nothing to say here". + assertThat(cellsIn(lines[1])[TEMPERATURE_COLUMN]).isEmpty() + assertThat(cellsIn(lines[2])[TEMPERATURE_COLUMN]).isEqualTo("31500") + } + + private companion object { + /** Index of `battery_temp_millicelsius`, from the header contract above. */ + val TEMPERATURE_COLUMN = EXPECTED_HEADER.split(",").indexOf("\"battery_temp_millicelsius\"") + + /** + * The header line, spelled out rather than derived from [MetricsCsv.HEADER]. + * + * This is the file's contract, and a test that builds its expectation from the same constant + * the code builds the file from asserts only that the code is self-consistent -- a renamed + * column or a change in how cells are quoted would rename it here too and stay green. + * ADFA-5494 reads this format back, and ADFA-5526 and ADFA-5534 ship it inside reports, so a + * schema change should have to come and edit this line on purpose. + */ + const val EXPECTED_HEADER = + "\"timestamp\",\"ide_pss_bytes\",\"gradle_tooling_pss_bytes\",\"gradle_daemon_pss_bytes\",\"net_rx_bytes\",\"net_tx_bytes\",\"battery_temp_millicelsius\",\"power_microwatts\",\"thermal_status\",\"annotation\",\"annotation_kind\"" + + /** 2026-09-06T22:33:40.123 in America/Los_Angeles, which is UTC-7 at that date. */ + const val T0 = 1_788_759_220_123L + + /** The gap between the default rows, and so the distance a marker may sit from one. */ + const val INTERVAL_MS = 1_000L + } +} diff --git a/app/src/test/java/com/itsaky/androidide/utils/MetricsFileNameTest.kt b/app/src/test/java/com/itsaky/androidide/utils/MetricsFileNameTest.kt new file mode 100644 index 0000000000..fd98e4ffbf --- /dev/null +++ b/app/src/test/java/com/itsaky/androidide/utils/MetricsFileNameTest.kt @@ -0,0 +1,65 @@ +/* + * 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 +import java.time.ZoneId + +/** The one name every exported metrics file gets (ADFA-5531's rule (c)). */ +@RunWith(JUnit4::class) +class MetricsFileNameTest { + private val zone: ZoneId = ZoneId.of("America/Los_Angeles") + + @Test + fun `the name is the local time to the millisecond, and the extension`() { + val name = MetricsFileName.forTime(T0, "csv", zone) + + assertThat(name).isEqualTo("2026_09_06_22_33_40_123.csv") + } + + @Test + fun `the image and the data exported at one moment differ only in extension`() { + // Which is the point of rule (c): a pair exported together sorts together, and neither + // leads with a chart title that would sort them apart. + val csv = MetricsFileName.forTime(T0, "csv", zone) + val png = MetricsFileName.forTime(T0, "png", zone) + + assertThat(csv.removeSuffix(".csv")).isEqualTo(png.removeSuffix(".png")) + } + + @Test + fun `names sort in the order the files were written`() { + val earlier = MetricsFileName.forTime(T0, "csv", zone) + val later = MetricsFileName.forTime(T0 + 1L, "csv", zone) + val muchLater = MetricsFileName.forTime(T0 + 86_400_000L, "csv", zone) + + // A directory listing is sorted lexicographically, so the format has to be too -- which is + // why it is fixed-width and big-endian rather than anything friendlier to read. + assertThat(listOf(muchLater, later, earlier).sorted()) + .containsExactly(earlier, later, muchLater) + .inOrder() + } + + private companion object { + /** 2026-09-06T22:33:40.123 in America/Los_Angeles. */ + const val T0 = 1_788_759_220_123L + } +} diff --git a/app/src/test/java/com/itsaky/androidide/utils/MetricsSnapshotTest.kt b/app/src/test/java/com/itsaky/androidide/utils/MetricsSnapshotTest.kt index 53ca31b780..613d699915 100644 --- a/app/src/test/java/com/itsaky/androidide/utils/MetricsSnapshotTest.kt +++ b/app/src/test/java/com/itsaky/androidide/utils/MetricsSnapshotTest.kt @@ -27,8 +27,10 @@ import org.robolectric.RobolectricTestRunner import java.io.File /** - * Pins ADFA-5486's snapshot export: a chart becomes a PNG in the cache, named after the chart, with - * only the newest one kept. + * Pins ADFA-5486's snapshot export: a chart becomes a PNG in the cache, with a few recent ones kept. + * + * The name used to lead with the chart's title. ADFA-5531 gave the image and the CSV one naming rule + * so a pair exported together sorts together, which is what the naming tests here now pin. */ @RunWith(RobolectricTestRunner::class) class MetricsSnapshotTest { @@ -38,7 +40,7 @@ class MetricsSnapshotTest { @Test fun `writes a png into the cache`() { - val file = MetricsSnapshot.write(context, bitmap(), "Memory usage") + val file = MetricsSnapshot.write(context, bitmap()) assertThat(file).isNotNull() assertThat(file!!.exists()).isTrue() @@ -49,54 +51,39 @@ class MetricsSnapshotTest { } @Test - fun `names the file after the chart`() { - val file = MetricsSnapshot.write(context, bitmap(), "Network traffic") + fun `the name is the shared metrics naming rule`() { + val file = MetricsSnapshot.write(context, bitmap(), AT) - assertThat(file!!.name).startsWith("network-traffic-") - } - - @Test - fun `a title with punctuation or non-ascii still makes a usable filename`() { - // Chart titles are translated, so they are not guaranteed to be filename-safe. - val file = MetricsSnapshot.write(context, bitmap(), "Mémoire / usage (MB)") - - assertThat(file).isNotNull() - assertThat(file!!.name).matches("[a-z0-9-]+\\.png") - } - - @Test - fun `a title with nothing usable still produces a file`() { - val file = MetricsSnapshot.write(context, bitmap(), "***") - - assertThat(file).isNotNull() - assertThat(file!!.name).startsWith("metrics-") + // The same name the CSV exported at that moment would get, differing only in extension -- + // no chart title in front of it to sort the pair apart (ADFA-5531). + assertThat(file!!.name).isEqualTo(MetricsFileName.forTime(AT, "png")) } @Test fun `a shared snapshot survives the next few exports`() { - val shared = MetricsSnapshot.write(context, bitmap(), "Memory usage")!! + val shared = MetricsSnapshot.write(context, bitmap(), AT)!! // A share hands the recipient a FileProvider URI and the chooser returns long before the // recipient opens it. Deleting the previous file on the next export pulled the image out // from under an app that had not read it yet. - repeat(3) { index -> MetricsSnapshot.write(context, bitmap(), "Chart $index") } + repeat(3) { index -> MetricsSnapshot.write(context, bitmap(), AT + index + 1L) } assertThat(shared.exists()).isTrue() } @Test fun `the directory stays bounded across many exports`() { - repeat(20) { index -> MetricsSnapshot.write(context, bitmap(), "Chart $index") } + repeat(20) { index -> MetricsSnapshot.write(context, bitmap(), AT + index) } // Bounded, not unbounded: this is a scratch directory, not a gallery. - val directory = MetricsSnapshot.write(context, bitmap(), "Last")!!.parentFile!! + val directory = MetricsSnapshot.write(context, bitmap(), AT + 100L)!!.parentFile!! assertThat(directory.listFiles()!!.size).isAtMost(MetricsSnapshot.KEEP_RECENT) } @Test fun `the newest snapshot is the one handed back, and it is on disk`() { - MetricsSnapshot.write(context, bitmap(), "Memory usage") - val newest = MetricsSnapshot.write(context, bitmap(), "Network traffic") + MetricsSnapshot.write(context, bitmap(), AT) + val newest = MetricsSnapshot.write(context, bitmap(), AT + 1L) // This used to assert that the previous file was gone. It is not, deliberately: a share // can still be reading it. What has to hold is that the file returned exists and is in @@ -105,4 +92,14 @@ class MetricsSnapshotTest { assertThat(newest!!.exists()).isTrue() assertThat(newest.parentFile).isEqualTo(File(context.cacheDir, "metrics-snapshots")) } + + private companion object { + /** + * A fixed export time. + * + * The name carries milliseconds, so two writes in the same millisecond would be one file. + * Real exports are a tap apart; a test loop is not. + */ + const val AT = 1_788_759_220_123L + } } diff --git a/idetooltips/src/main/java/com/itsaky/androidide/idetooltips/TooltipTag.kt b/idetooltips/src/main/java/com/itsaky/androidide/idetooltips/TooltipTag.kt index 5086eb33e1..99bcd85213 100644 --- a/idetooltips/src/main/java/com/itsaky/androidide/idetooltips/TooltipTag.kt +++ b/idetooltips/src/main/java/com/itsaky/androidide/idetooltips/TooltipTag.kt @@ -331,6 +331,7 @@ object TooltipTag { const val CAROUSEL_PREVIOUS = "carousel.previous" const val CAROUSEL_NEXT = "carousel.next" const val CAROUSEL_SNAPSHOT = "carousel.snapshot" + const val CAROUSEL_EXPORT = "carousel.export" const val CAROUSEL_CHART_MEMORY = "carousel.chart.memory" const val CAROUSEL_CHART_NETWORK = "carousel.chart.network" const val CAROUSEL_CHART_POWER = "carousel.chart.power" diff --git a/resources/src/main/res/values/strings.xml b/resources/src/main/res/values/strings.xml index 67a7e6af4c..1f2990d32a 100644 --- a/resources/src/main/res/values/strings.xml +++ b/resources/src/main/res/values/strings.xml @@ -1702,6 +1702,8 @@ Next metric Save chart image Couldn\'t save the chart image. + Save metrics data + Couldn\'t save the metrics data. Received Sent