From 7becd06cebdb0b5392fa9a92bf886624fe70cc98 Mon Sep 17 00:00:00 2001 From: David Schachter Date: Fri, 4 Sep 2026 16:21:32 -0700 Subject: [PATCH 1/5] feat: add a UID-level network traffic page to the metrics carousel Second page of the editor's metrics carousel (ADFA-5487) is now a live network traffic chart, replacing the brand-mark placeholder. Accounting is UID-level, as decided on the ticket: TrafficStats.getUidRxBytes / getUidTxBytes cover every process sharing the app's UID, so Gradle's downloads are included without any socket tagging -- the Gradle Tooling and daemon processes share it. There is deliberately no per-feature breakdown; the only two tagged sockets in the tree are the local documentation web server and the JDWP listener, neither of which is interesting here. The platform counters are cumulative since boot, so NetworkUsageWatcher records the delta between consecutive samples. Three cases the raw counters would get wrong: - The first sample only establishes a baseline and contributes 0. Otherwise the chart would open with a spike equal to everything the app had transferred since boot. - A counter that goes backwards (reboot, re-based accounting) records 0 rather than plotting negative traffic. - TrafficStats.UNSUPPORTED (-1), which some devices return, is detected once and latched, so -1 is never plotted as a byte count. getUsage() hands out copies rather than the live ring buffers, guarded by a lock. The renderer reads all 30 entries while the sampler thread appends, and MemoryUsageWatcher's equivalent has that race today. Axis, per the ticket's decisions: - Values are log10(bytes + 1). Traffic spans orders of magnitude -- a few hundred bytes of chatter next to a multi-megabyte download -- and a linear axis flattens all of it but the largest burst onto the baseline. MPAndroidChart has no logarithmic axis. - The + 1 floors zero, which is the common sample rather than an edge case: an idle IDE transfers nothing and log10(0) is negative infinity. A zero sample plots at exactly 0.0 and the line stays continuous. - Units are decimal (1 kB = 1000 B), not binary. This was not in the ticket and is a consequence of the log axis: on-device the first cut labelled the gridlines 9B / 99B / 999B / 9.8KB, because powers of ten divided by 1024 stop looking like decades. Decimal units label them 0B / 10B / 100B / 1.0kB, and are the convention for throughput. - Axis labels show 10^value rather than the exact inverse 10^value - 1, which would read 9B / 99B / 999B. One byte is not worth the confusion, and the legend carries the exact current figure. Zero is labelled exactly, since log10(0 + 1) really is 0. MetricsPage.Image and its layout go with the placeholder, having no remaining user; ADFA-5490 will define its own extension surface. The cogo_brand_mark drawable stays -- six other screens use it. Verified on a Pixel 6 Pro (arm64), v8 debug, over wifi with a real Gradle sync: - Both series track real traffic (peaks ~10kB/s against byte-level chatter, both legible on the one scale), idle periods sit flat on the 0B baseline, and the axis reads 0B / 10B / 100B / 1.0kB / 10.0kB. - Swiping to the memory page and back returns the full 30-sample history, so the page is recycling-safe like the memory one. - Font scale 1.0 and 2.0, measured on a cold start (EditorActivityKt declares fontScale in configChanges, so a warm relaunch reports stale geometry): title 22dp -> 35dp, pager 185dp -> 171dp, panel 248dp throughout, nothing clipped. - Landscape renders correctly, nothing clipped. - 16 new tests (7 watcher, 9 renderer); 80 tests green across app ui/utils/activities/fragments. ADFA-5489 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01QeW3M24yD6HNhEnbuNW7Hz --- .../activities/editor/BaseEditorActivity.kt | 36 ++- .../androidide/ui/MetricsCarouselAdapter.kt | 49 ++-- .../ui/NetworkUsageChartRenderer.kt | 267 ++++++++++++++++++ .../androidide/utils/NetworkUsageWatcher.kt | 228 +++++++++++++++ ...age.xml => item_metrics_network_chart.xml} | 7 +- app/src/main/res/values/dimens.xml | 1 - .../ui/NetworkUsageChartRendererTest.kt | 167 +++++++++++ .../utils/NetworkUsageWatcherTest.kt | 165 +++++++++++ resources/src/main/res/values/strings.xml | 6 +- 9 files changed, 884 insertions(+), 42 deletions(-) create mode 100644 app/src/main/java/com/itsaky/androidide/ui/NetworkUsageChartRenderer.kt create mode 100644 app/src/main/java/com/itsaky/androidide/utils/NetworkUsageWatcher.kt rename app/src/main/res/layout/{item_metrics_image.xml => item_metrics_network_chart.xml} (86%) create mode 100644 app/src/test/java/com/itsaky/androidide/ui/NetworkUsageChartRendererTest.kt create mode 100644 app/src/test/java/com/itsaky/androidide/utils/NetworkUsageWatcherTest.kt 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 3f4794af23..05ff076332 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 @@ -122,6 +122,7 @@ import com.itsaky.androidide.ui.ContentTranslatingDrawerLayout import com.itsaky.androidide.ui.MemoryUsageChartRenderer import com.itsaky.androidide.ui.MetricsCarouselAdapter import com.itsaky.androidide.ui.MetricsPage +import com.itsaky.androidide.ui.NetworkUsageChartRenderer import com.itsaky.androidide.ui.SwipeRevealLayout import com.itsaky.androidide.uidesigner.UIDesignerActivity import com.itsaky.androidide.utils.ActionMenuUtils.showPopupWindow @@ -131,6 +132,7 @@ import com.itsaky.androidide.utils.FlashType import com.itsaky.androidide.utils.InstallationResultHandler.onResult import com.itsaky.androidide.utils.IntentUtils import com.itsaky.androidide.utils.MemoryUsageWatcher +import com.itsaky.androidide.utils.NetworkUsageWatcher import com.itsaky.androidide.utils.StringsInjectionException import com.itsaky.androidide.utils.StringsXmlInjector import com.itsaky.androidide.utils.applyBottomSheetAnchorForOrientation @@ -195,6 +197,15 @@ abstract class BaseEditorActivity : lineColorFor = ::getMemUsageLineColorFor, ) + protected val networkUsageWatcher = NetworkUsageWatcher() + private val networkUsageChartRenderer = + NetworkUsageChartRenderer(usageProvider = networkUsageWatcher::getUsage) + + private val networkUsageListener = + NetworkUsageWatcher.NetworkUsageListener { usage -> + networkUsageChartRenderer.onUsageChanged(usage) + } + private val fileManagerViewModel by viewModels() private var feedbackButtonManager: FeedbackButtonManager? = null private var fullscreenManager: FullscreenManager? = null @@ -524,11 +535,14 @@ abstract class BaseEditorActivity : metricsPageCallback = null _binding?.memUsageView?.metricsPager?.adapter = null memUsageChartRenderer.detach() + networkUsageChartRenderer.detach() _binding = null if (isDestroying) { memoryUsageWatcher.stopWatching(true) memoryUsageWatcher.listener = null + networkUsageWatcher.stopWatching() + networkUsageWatcher.listener = null editorActivityScope.cancelIfActive("Activity is being destroyed") unbindDebuggerService() @@ -866,6 +880,7 @@ abstract class BaseEditorActivity : setupMetricsCarousel() watchMemory() + watchNetwork() observeFileOperations() setupGestureDetector() @@ -974,17 +989,14 @@ abstract class BaseEditorActivity : private fun setupMetricsCarousel() { val pages = listOf( - // The memory chart is the default page (ADFA-5487). The logo is a placeholder second - // page until there is a real second metric; the network-traffic chart replaces it. + // The memory chart is the default page (ADFA-5487); network traffic is the second + // (ADFA-5489), replacing the brand-mark placeholder that ADFA-5487 shipped. MetricsPage.MemoryChart(title = string.metrics_title_memory), - MetricsPage.Image( - drawable = R.drawable.cogo_brand_mark, - description = string.metrics_carousel_brand_mark, - title = string.metrics_title_brand_mark, - ), + MetricsPage.NetworkChart(title = string.metrics_title_network), ) - binding.memUsageView.metricsPager.adapter = MetricsCarouselAdapter(pages, memUsageChartRenderer) + binding.memUsageView.metricsPager.adapter = + MetricsCarouselAdapter(pages, memUsageChartRenderer, networkUsageChartRenderer) val showTitleFor = { position: Int -> pages.getOrNull(position)?.let { page -> @@ -1009,6 +1021,10 @@ abstract class BaseEditorActivity : resetMemUsageChart() } + private fun watchNetwork() { + networkUsageWatcher.listener = networkUsageListener + } + /** * Rebuilds the memory chart for the currently watched processes. Call after starting or stopping * watching a process. @@ -1029,6 +1045,8 @@ abstract class BaseEditorActivity : super.onPause() memoryUsageWatcher.listener = null memoryUsageWatcher.stopWatching(false) + networkUsageWatcher.listener = null + networkUsageWatcher.stopWatching() this.isDestroying = isFinishing getFileTreeFragment()?.saveTreeState() @@ -1047,6 +1065,8 @@ abstract class BaseEditorActivity : memoryUsageWatcher.listener = memoryUsageListener memoryUsageWatcher.startWatching() + networkUsageWatcher.listener = networkUsageListener + networkUsageWatcher.startWatching() apkInstallationViewModel.reloadStatus(this) diff --git a/app/src/main/java/com/itsaky/androidide/ui/MetricsCarouselAdapter.kt b/app/src/main/java/com/itsaky/androidide/ui/MetricsCarouselAdapter.kt index 4b602584fc..e9ff65f7f8 100644 --- a/app/src/main/java/com/itsaky/androidide/ui/MetricsCarouselAdapter.kt +++ b/app/src/main/java/com/itsaky/androidide/ui/MetricsCarouselAdapter.kt @@ -20,8 +20,6 @@ package com.itsaky.androidide.ui import android.view.LayoutInflater import android.view.View import android.view.ViewGroup -import android.widget.ImageView -import androidx.annotation.DrawableRes import androidx.annotation.StringRes import androidx.recyclerview.widget.RecyclerView import com.itsaky.androidide.R @@ -40,10 +38,8 @@ sealed interface MetricsPage { @StringRes override val title: Int, ) : MetricsPage - /** A static image. Placeholder page until real metrics exist to show alongside memory. */ - data class Image( - @DrawableRes val drawable: Int, - @StringRes val description: Int, + /** The live network-traffic chart, rendered by [NetworkUsageChartRenderer]. */ + data class NetworkChart( @StringRes override val title: Int, ) : MetricsPage } @@ -60,7 +56,8 @@ sealed interface MetricsPage { */ class MetricsCarouselAdapter( private val pages: List, - private val chartRenderer: MemoryUsageChartRenderer, + private val memoryChartRenderer: MemoryUsageChartRenderer, + private val networkChartRenderer: NetworkUsageChartRenderer, ) : RecyclerView.Adapter() { sealed class PageViewHolder( view: View, @@ -69,9 +66,9 @@ class MetricsCarouselAdapter( val chart: SafeLineChart, ) : PageViewHolder(chart) - class Image( - val image: ImageView, - ) : PageViewHolder(image) + class NetworkChart( + val chart: SafeLineChart, + ) : PageViewHolder(chart) } override fun getItemCount(): Int = pages.size @@ -79,7 +76,7 @@ class MetricsCarouselAdapter( override fun getItemViewType(position: Int): Int = when (pages[position]) { is MetricsPage.MemoryChart -> VIEW_TYPE_MEMORY_CHART - is MetricsPage.Image -> VIEW_TYPE_IMAGE + is MetricsPage.NetworkChart -> VIEW_TYPE_NETWORK_CHART } override fun onCreateViewHolder( @@ -94,9 +91,9 @@ class MetricsCarouselAdapter( ) } - VIEW_TYPE_IMAGE -> { - PageViewHolder.Image( - inflater.inflate(R.layout.item_metrics_image, parent, false) as ImageView, + VIEW_TYPE_NETWORK_CHART -> { + PageViewHolder.NetworkChart( + inflater.inflate(R.layout.item_metrics_network_chart, parent, false) as SafeLineChart, ) } @@ -110,31 +107,29 @@ class MetricsCarouselAdapter( holder: PageViewHolder, position: Int, ) { - when (val page = pages[position]) { + when (pages[position]) { is MetricsPage.MemoryChart -> { - chartRenderer.attach((holder as PageViewHolder.MemoryChart).chart) + memoryChartRenderer.attach((holder as PageViewHolder.MemoryChart).chart) } - is MetricsPage.Image -> { - (holder as PageViewHolder.Image).image.apply { - setImageResource(page.drawable) - contentDescription = context.getString(page.description) - } + is MetricsPage.NetworkChart -> { + networkChartRenderer.attach((holder as PageViewHolder.NetworkChart).chart) } } } override fun onViewRecycled(holder: PageViewHolder) { - if (holder is PageViewHolder.MemoryChart) { - // Only if this holder's chart is still the attached one: a rebind can create the - // replacement before RecyclerView recycles the view it replaced, and detaching then - // would drop the new chart instead of the old. - chartRenderer.detachIfAttached(holder.chart) + // Only if this holder's chart is still the attached one: a rebind can create the replacement + // before RecyclerView recycles the view it replaced, and detaching then would drop the new + // chart instead of the old. + when (holder) { + is PageViewHolder.MemoryChart -> memoryChartRenderer.detachIfAttached(holder.chart) + is PageViewHolder.NetworkChart -> networkChartRenderer.detachIfAttached(holder.chart) } } private companion object { const val VIEW_TYPE_MEMORY_CHART = 0 - const val VIEW_TYPE_IMAGE = 1 + const val VIEW_TYPE_NETWORK_CHART = 1 } } diff --git a/app/src/main/java/com/itsaky/androidide/ui/NetworkUsageChartRenderer.kt b/app/src/main/java/com/itsaky/androidide/ui/NetworkUsageChartRenderer.kt new file mode 100644 index 0000000000..13061c2a35 --- /dev/null +++ b/app/src/main/java/com/itsaky/androidide/ui/NetworkUsageChartRenderer.kt @@ -0,0 +1,267 @@ +/* + * 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.ui + +import android.graphics.Color +import androidx.annotation.UiThread +import com.github.mikephil.charting.components.AxisBase +import com.github.mikephil.charting.data.Entry +import com.github.mikephil.charting.data.LineData +import com.github.mikephil.charting.data.LineDataSet +import com.github.mikephil.charting.formatter.IAxisValueFormatter +import com.itsaky.androidide.R +import com.itsaky.androidide.utils.NetworkUsageWatcher +import com.itsaky.androidide.utils.NetworkUsageWatcher.NetworkUsage +import com.itsaky.androidide.utils.resolveAttr +import kotlin.math.log10 +import kotlin.math.pow +import kotlin.math.roundToLong + +/** + * Renders [NetworkUsageWatcher] samples into a [SafeLineChart] on a logarithmic scale (ADFA-5489). + * + * Traffic spans orders of magnitude -- a few hundred bytes of chatter next to a multi-megabyte + * Gradle download -- so a linear axis flattens everything but the largest burst into the baseline. + * MPAndroidChart has no logarithmic axis, so the plotted value is [log10] of the byte count and + * [BytesAxisFormatter] turns the axis labels back into byte units. + * + * Zero is the common sample, not an edge case: an idle IDE transfers nothing, and `log10(0)` is + * negative infinity. Values are therefore `log10(bytes + 1)`, which puts a zero sample at exactly + * `0.0` and keeps the line continuous. + * + * Like [MemoryUsageChartRenderer] this holds no sample state -- [NetworkUsageWatcher] owns the + * history -- so a chart can be attached, detached and recycled by the metrics carousel without + * losing anything. + * + * All methods must be called on the UI thread; MPAndroidChart is not thread-safe (see + * [SafeLineChart]). + * + * @param usageProvider Supplies the current sample history. + */ +class NetworkUsageChartRenderer( + private val usageProvider: () -> NetworkUsage, +) { + private var chart: SafeLineChart? = null + + @UiThread + fun attach(chart: SafeLineChart) { + this.chart = chart + configure(chart) + rebuild() + } + + @UiThread + fun detach() { + chart = null + } + + /** + * Detaches [chart] only if it is the currently attached one. See + * [MemoryUsageChartRenderer.detachIfAttached]. + */ + @UiThread + fun detachIfAttached(chart: SafeLineChart) { + if (this.chart === chart) { + detach() + } + } + + /** + * Rebuilds both series from the full sample history. + */ + @UiThread + fun rebuild() { + val chart = this.chart ?: return + val usage = usageProvider() + + val textColor = chart.context.resolveAttr(R.attr.colorOnSurface) + val bgColor = chart.context.resolveAttr(R.attr.colorSurfaceDim) + + val datasets = + arrayOf( + dataset(usage.received, chart.context.getString(R.string.metrics_network_received), RECEIVED_COLOR), + dataset(usage.transmitted, chart.context.getString(R.string.metrics_network_transmitted), TRANSMITTED_COLOR), + ) + + chart.apply { + data = LineData(*datasets) + axisRight.textColor = textColor + axisLeft.textColor = textColor + legend.textColor = textColor + + data.setValueTextColor(textColor) + setBackgroundColor(bgColor) + setGridBackgroundColor(bgColor) + notifyDataSetChanged() + invalidate() + } + } + + /** + * Updates both series in place from a fresh sample, rebuilding if the chart's shape no longer + * matches. Allocates nothing on the common path, which runs once a second. + */ + @UiThread + fun onUsageChanged(usage: NetworkUsage) { + val chart = this.chart ?: return + val data = chart.data + + if (data == null || data.dataSetCount != SERIES_COUNT) { + rebuild() + return + } + + val received = data.getDataSetByIndex(RECEIVED_INDEX) as LineDataSet? + val transmitted = data.getDataSetByIndex(TRANSMITTED_INDEX) as LineDataSet? + if (received == null || transmitted == null || + received.entryCount != usage.received.size || + transmitted.entryCount != usage.transmitted.size + ) { + rebuild() + return + } + + update(received, usage.received, chart.context.getString(R.string.metrics_network_received)) + update(transmitted, usage.transmitted, chart.context.getString(R.string.metrics_network_transmitted)) + + chart.apply { + data.notifyDataChanged() + notifyDataSetChanged() + invalidate() + } + } + + private fun dataset( + samples: LongArray, + label: String, + lineColor: Int, + ): LineDataSet = + LineDataSet( + List(samples.size) { index -> Entry(index.toFloat(), samples[index].toLogBytes()) }, + label, + ).apply { + color = lineColor + setDrawIcons(false) + setDrawCircles(false) + setDrawCircleHole(false) + setDrawValues(false) + formLineWidth = 1f + formSize = 15f + isHighlightEnabled = false + this.label = labelFor(label, samples.lastOrNull() ?: 0L) + } + + private fun update( + dataset: LineDataSet, + samples: LongArray, + label: String, + ) { + for (index in samples.indices) { + dataset.entries[index].y = samples[index].toLogBytes() + } + dataset.label = labelFor(label, samples.lastOrNull() ?: 0L) + dataset.notifyDataSetChanged() + } + + private fun labelFor( + label: String, + bytes: Long, + ): String = "%s - %s/s".format(label, formatBytes(bytes.toDouble())) + + private fun configure(chart: SafeLineChart) { + chart.apply { + val colorAccent = context.resolveAttr(R.attr.colorAccent) + + isDragEnabled = false + description.isEnabled = false + xAxis.axisLineColor = colorAccent + axisRight.axisLineColor = colorAccent + + setPinchZoom(false) + setBackgroundColor(context.resolveAttr(R.attr.colorSurfaceDim)) + setDrawGridBackground(true) + setScaleEnabled(true) + + axisLeft.isEnabled = false + axisRight.valueFormatter = BytesAxisFormatter + // Without a floor the axis auto-scales to the noise around zero when nothing is happening. + axisRight.axisMinimum = 0f + // One label per decade, so the gridlines read as 1.0kB / 1.0MB rather than arbitrary + // fractions of a logarithm. + axisRight.granularity = 1f + axisRight.isGranularityEnabled = true + } + } + + /** + * Labels a logarithmic axis value in byte units. + * + * Gridlines land on integer values (granularity 1), so each is a power of ten and is labelled as + * one: 10B, 100B, 1.0kB. The exact inverse of [toLogBytes] would be `10^value - 1`, which labels + * those same lines 9B, 99B, 999B -- correct to the byte but unreadable as a scale. The one byte + * is not worth the confusion; the legend carries the exact current figure. + * + * Zero is the exception and is labelled exactly: `log10(0 + 1)` is 0, so the baseline really is + * no traffic, not one byte. + */ + private object BytesAxisFormatter : IAxisValueFormatter { + override fun getFormattedValue( + value: Float, + axis: AxisBase?, + ): String = + if (value < 0.5f) { + formatBytes(0.0) + } else { + formatBytes(10.0.pow(value.toDouble())) + } + } + + private companion object { + const val SERIES_COUNT = 2 + const val RECEIVED_INDEX = 0 + const val TRANSMITTED_INDEX = 1 + + val RECEIVED_COLOR = Color.CYAN + val TRANSMITTED_COLOR = Color.MAGENTA + } +} + +/** + * The plotted value for a byte count: `log10(bytes + 1)`. + * + * The `+ 1` is what makes zero plottable -- it maps to `0.0` rather than negative infinity -- and + * zero is the usual sample for an idle IDE. + */ +private fun Long.toLogBytes(): Float = log10(this.coerceAtLeast(0L).toDouble() + 1.0).toFloat() + +/** + * Formats a byte count for an axis label or legend, to at most one decimal place. + * + * Units are decimal (1 kB = 1000 B), not binary. On a log10 axis the gridlines are powers of ten, + * and dividing those by 1024 would label them 9.8KB, 977KB, 954MB -- the decades stop looking like + * decades. Decimal units are also the convention for network throughput. + */ +private fun formatBytes(bytes: Double): String { + val clamped = bytes.coerceAtLeast(0.0) + return when { + clamped < 1_000 -> "%dB".format(clamped.roundToLong()) + clamped < 1_000_000 -> "%.1fkB".format(clamped / 1_000) + clamped < 1_000_000_000 -> "%.1fMB".format(clamped / 1_000_000) + else -> "%.1fGB".format(clamped / 1_000_000_000) + } +} diff --git a/app/src/main/java/com/itsaky/androidide/utils/NetworkUsageWatcher.kt b/app/src/main/java/com/itsaky/androidide/utils/NetworkUsageWatcher.kt new file mode 100644 index 0000000000..09e2f52e34 --- /dev/null +++ b/app/src/main/java/com/itsaky/androidide/utils/NetworkUsageWatcher.kt @@ -0,0 +1,228 @@ +/* + * 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.net.TrafficStats +import android.os.Process +import androidx.annotation.VisibleForTesting +import com.itsaky.androidide.tasks.cancelIfActive +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.DelicateCoroutinesApi +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.delay +import kotlinx.coroutines.launch +import kotlinx.coroutines.newSingleThreadContext +import kotlinx.coroutines.withContext +import org.slf4j.LoggerFactory +import java.util.concurrent.atomic.AtomicBoolean + +/** + * Samples this app's network traffic (ADFA-5489). + * + * Accounting is UID-level, not per socket: [TrafficStats.getUidRxBytes] and + * [TrafficStats.getUidTxBytes] cover every process sharing the app's UID, which is what makes + * Gradle's downloads show up here -- the Gradle Tooling and daemon processes share it. No socket + * tagging is involved, so there is deliberately no per-feature breakdown. + * + * The platform counters are cumulative since boot, so what is recorded is the *delta* between + * consecutive samples: bytes transferred during that interval. A sampler that reported the raw + * counters would draw a monotonically rising line that says nothing about current activity. + * + * @param updateInterval Milliseconds between samples. + * @param uid The UID to account for. Defaults to this process's own; injectable for tests. + * @param readRxBytes Reads the cumulative received byte count. Injectable for tests. + * @param readTxBytes Reads the cumulative transmitted byte count. Injectable for tests. + */ +class NetworkUsageWatcher( + private val updateInterval: Long = DEFAULT_UPDATE_INTERVAL, + private val uid: Int = Process.myUid(), + private val readRxBytes: (Int) -> Long = TrafficStats::getUidRxBytes, + private val readTxBytes: (Int) -> Long = TrafficStats::getUidTxBytes, +) { + @OptIn(ExperimentalCoroutinesApi::class, DelicateCoroutinesApi::class) + private val coroutineDispatcher = newSingleThreadContext("NetworkUsageWatcher") + private val coroutineScope = CoroutineScope(coroutineDispatcher) + private val watching = AtomicBoolean(false) + + /** Guards the two ring buffers: the sampler writes them, the UI thread snapshots them. */ + private val historyLock = Any() + + private val received = MutableShiftedLongArray(MAX_USAGE_ENTRIES) + private val transmitted = MutableShiftedLongArray(MAX_USAGE_ENTRIES) + + /** + * The previous cumulative readings, or `null` before the first sample. The first sample + * establishes a baseline and contributes no delta -- the alternative would be a spike equal to + * everything the app had transferred since boot. + */ + private var lastRx: Long? = null + private var lastTx: Long? = null + + /** + * Whether the platform reports traffic for this UID at all. Cleared permanently if a read comes + * back [TrafficStats.UNSUPPORTED], which some devices and emulators do. + */ + @Volatile + var isSupported: Boolean = true + private set + + val isWatching: Boolean + get() = watching.get() + + /** + * Notified on the main thread after each sample. + */ + var listener: NetworkUsageListener? = null + + /** + * A snapshot of the sampled history, oldest first. Safe to call from any thread at any time; + * before the first sample every entry is zero. + * + * The arrays are copies. Handing out the live ring buffers would let the caller read them while + * the sampler thread is midway through appending, and the chart renderer reads all 30 entries. + */ + fun getUsage(): NetworkUsage = + synchronized(historyLock) { + NetworkUsage(received.snapshot(), transmitted.snapshot()) + } + + fun startWatching() { + if (isWatching) { + log.warn("Network usage is already being watched") + return + } + + watching.set(true) + + coroutineScope.launch(context = SupervisorJob() + coroutineDispatcher) { + while (isWatching) { + sampleOnce() + + listener?.also { listener -> + val usage = getUsage() + withContext(Dispatchers.Main.immediate) { + listener.onNetworkUsageChanged(usage) + } + } + + delay(updateInterval) + } + } + } + + fun stopWatching() { + watching.set(false) + coroutineScope.cancelIfActive("Cancellation requested") + } + + /** + * Takes one sample. The sampling loop calls this once per [updateInterval]; tests call it + * directly so the delta accounting can be exercised without threads or waiting. + */ + @VisibleForTesting + internal fun sampleOnce() { + if (!isSupported) { + return + } + + val rx = readRxBytes(uid) + val tx = readTxBytes(uid) + + if (rx == UNSUPPORTED || tx == UNSUPPORTED) { + // Not transient: the platform either accounts for this UID or it does not. + isSupported = false + log.info("Network usage is unavailable on this device; the traffic chart will read zero") + return + } + + synchronized(historyLock) { + record(received, previous = lastRx, current = rx) + record(transmitted, previous = lastTx, current = tx) + } + + lastRx = rx + lastTx = tx + } + + /** + * Appends the delta between [previous] and [current] to [history]. + * + * A negative delta means the counter went backwards, which happens when it is reset -- the + * device rebooted, or the platform re-based its accounting. Treated as a fresh baseline (zero + * for this interval) rather than plotted as negative traffic. + */ + private fun record( + history: MutableShiftedLongArray, + previous: Long?, + current: Long, + ) { + val delta = + when { + previous == null -> 0L + current < previous -> 0L + else -> current - previous + } + + // Newest entry goes in at index 0 and the shift makes it the last element, so + // history[size - 1] is always the newest. Same convention as MemoryUsageWatcher. + history[0] = delta + history.shift(1) + } + + /** + * Bytes transferred per sampling interval, oldest first. + * + * @property received Bytes received during each interval. + * @property transmitted Bytes transmitted during each interval. + */ + data class NetworkUsage( + val received: LongArray, + val transmitted: LongArray, + ) { + override fun equals(other: Any?): Boolean = + this === other || + ( + other is NetworkUsage && + received.contentEquals(other.received) && + transmitted.contentEquals(other.transmitted) + ) + + override fun hashCode(): Int = 31 * received.contentHashCode() + transmitted.contentHashCode() + } + + fun interface NetworkUsageListener { + fun onNetworkUsageChanged(usage: NetworkUsage) + } + + companion object { + const val MAX_USAGE_ENTRIES = 30 + const val DEFAULT_UPDATE_INTERVAL = 1000L + + /** [TrafficStats.UNSUPPORTED] widened to [Long], which is what the getters return. */ + private const val UNSUPPORTED = TrafficStats.UNSUPPORTED.toLong() + + private val log = LoggerFactory.getLogger(NetworkUsageWatcher::class.java) + } +} + +/** + * Copies this ring buffer into a plain array in logical order, oldest first. + */ +private fun ShiftedLongArray.snapshot(): LongArray = LongArray(size) { this[it] } diff --git a/app/src/main/res/layout/item_metrics_image.xml b/app/src/main/res/layout/item_metrics_network_chart.xml similarity index 86% rename from app/src/main/res/layout/item_metrics_image.xml rename to app/src/main/res/layout/item_metrics_network_chart.xml index 4d8617b328..f011080f06 100644 --- a/app/src/main/res/layout/item_metrics_image.xml +++ b/app/src/main/res/layout/item_metrics_network_chart.xml @@ -5,10 +5,9 @@ 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 . --> - + android:contentDescription="@string/metrics_network_chart" /> diff --git a/app/src/main/res/values/dimens.xml b/app/src/main/res/values/dimens.xml index c3bbde87e7..f1785efd39 100644 --- a/app/src/main/res/values/dimens.xml +++ b/app/src/main/res/values/dimens.xml @@ -9,7 +9,6 @@ 248dp 16dp 4dp - 16dp 28dp 28dp diff --git a/app/src/test/java/com/itsaky/androidide/ui/NetworkUsageChartRendererTest.kt b/app/src/test/java/com/itsaky/androidide/ui/NetworkUsageChartRendererTest.kt new file mode 100644 index 0000000000..95b81c8c6a --- /dev/null +++ b/app/src/test/java/com/itsaky/androidide/ui/NetworkUsageChartRendererTest.kt @@ -0,0 +1,167 @@ +/* + * 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.ui + +import android.content.Context +import androidx.test.core.app.ApplicationProvider +import com.github.mikephil.charting.data.LineDataSet +import com.google.common.truth.Truth.assertThat +import com.itsaky.androidide.utils.NetworkUsageWatcher +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import kotlin.math.log10 + +/** + * Pins the two axis decisions ADFA-5489 was scoped around: values are log10, and zero is floored + * via `log10(bytes + 1)` so an idle IDE plots a continuous line at 0 instead of negative infinity. + */ +@RunWith(RobolectricTestRunner::class) +class NetworkUsageChartRendererTest { + private val context = ApplicationProvider.getApplicationContext() + + private fun usage( + received: LongArray, + transmitted: LongArray = received, + ) = NetworkUsageWatcher.NetworkUsage(received, transmitted) + + private fun rendererFor(usage: NetworkUsageWatcher.NetworkUsage): Pair { + val chart = SafeLineChart(context) + val renderer = NetworkUsageChartRenderer(usageProvider = { usage }) + renderer.attach(chart) + return renderer to chart + } + + private fun dataset( + chart: SafeLineChart, + index: Int, + ) = chart.data.getDataSetByIndex(index) as LineDataSet + + @Test + fun `plots log10 of the byte count`() { + val (_, chart) = rendererFor(usage(longArrayOf(0L, 9L, 99L, 999L))) + + val ys = dataset(chart, 0).entries.map { it.y } + + // log10(n + 1): 0 -> 0, 9 -> 1, 99 -> 2, 999 -> 3. Exact decades, so the floor is visible. + assertThat(ys).containsExactly(0f, 1f, 2f, 3f).inOrder() + } + + @Test + fun `zero bytes plots at zero rather than negative infinity`() { + val (_, chart) = rendererFor(usage(LongArray(30) { 0L })) + + val ys = dataset(chart, 0).entries.map { it.y } + + assertThat(ys.none { it.isInfinite() || it.isNaN() }).isTrue() + assertThat(ys.toSet()).containsExactly(0f) + } + + @Test + fun `a megabyte burst stays on scale with surrounding chatter`() { + val bytes = longArrayOf(0L, 512L, 2L * 1024 * 1024, 256L) + val (_, chart) = rendererFor(usage(bytes)) + + val ys = dataset(chart, 0).entries.map { it.y } + + // The point of the log axis: a 2MB burst is ~6.3 while 512B is ~2.7, so the small values + // stay legible instead of being flattened onto the baseline. + assertThat(ys[2]).isWithin(0.01f).of(log10(2.0 * 1024 * 1024 + 1).toFloat()) + assertThat(ys[1]).isGreaterThan(2f) + assertThat(ys[2] - ys[1]).isLessThan(4f) + } + + @Test + fun `received and transmitted are separate series`() { + val (_, chart) = + rendererFor( + usage( + received = longArrayOf(0L, 999L), + transmitted = longArrayOf(0L, 9L), + ), + ) + + assertThat(chart.data.dataSetCount).isEqualTo(2) + assertThat(dataset(chart, 0).entries.last().y).isEqualTo(3f) + assertThat(dataset(chart, 1).entries.last().y).isEqualTo(1f) + } + + @Test + fun `the legend reports the latest sample in byte units`() { + val (_, chart) = rendererFor(usage(longArrayOf(0L, 2_000L))) + + // Rendered from the raw byte count, not from the logarithm, and in decimal units so that + // the log10 axis labels come out as clean decades. + assertThat(dataset(chart, 0).label).endsWith("2.0kB/s") + } + + @Test + fun `onUsageChanged updates entries in place without replacing the datasets`() { + val chart = SafeLineChart(context) + var current = usage(longArrayOf(0L, 9L)) + val renderer = NetworkUsageChartRenderer(usageProvider = { current }) + renderer.attach(chart) + + val datasetBefore = dataset(chart, 0) + val entryBefore = datasetBefore.entries.last() + + current = usage(longArrayOf(0L, 999L)) + renderer.onUsageChanged(current) + + assertThat(dataset(chart, 0)).isSameInstanceAs(datasetBefore) + assertThat(datasetBefore.entries.last()).isSameInstanceAs(entryBefore) + assertThat(entryBefore.y).isEqualTo(3f) + } + + @Test + fun `onUsageChanged rebuilds when the sample count changes`() { + val chart = SafeLineChart(context) + var current = usage(longArrayOf(0L, 9L)) + val renderer = NetworkUsageChartRenderer(usageProvider = { current }) + renderer.attach(chart) + + assertThat(dataset(chart, 0).entryCount).isEqualTo(2) + + current = usage(longArrayOf(0L, 9L, 99L)) + renderer.onUsageChanged(current) + + assertThat(dataset(chart, 0).entryCount).isEqualTo(3) + } + + @Test + fun `attach after detach renders the history into the new chart`() { + val current = usage(longArrayOf(0L, 99L)) + val (renderer, _) = rendererFor(current) + renderer.detach() + + val rebound = SafeLineChart(context) + renderer.attach(rebound) + + assertThat(dataset(rebound, 0).entries.last().y).isEqualTo(2f) + } + + @Test + fun `onUsageChanged after detach is a no-op`() { + val current = usage(longArrayOf(0L, 99L)) + val (renderer, _) = rendererFor(current) + renderer.detach() + + // A recycled carousel page must not keep the renderer writing into a dead view. + renderer.onUsageChanged(current) + } +} diff --git a/app/src/test/java/com/itsaky/androidide/utils/NetworkUsageWatcherTest.kt b/app/src/test/java/com/itsaky/androidide/utils/NetworkUsageWatcherTest.kt new file mode 100644 index 0000000000..c21420595a --- /dev/null +++ b/app/src/test/java/com/itsaky/androidide/utils/NetworkUsageWatcherTest.kt @@ -0,0 +1,165 @@ +/* + * 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 + +/** + * Pins the accounting decisions ADFA-5489 was scoped around: the platform counters are cumulative, + * so what is plotted is the delta between samples, and a counter reset must not plot as negative + * traffic. + * + * These drive [NetworkUsageWatcher.sampleOnce] directly rather than starting the sampling loop, so + * there is no waiting and no dependence on thread timing. + */ +@RunWith(RobolectricTestRunner::class) +class NetworkUsageWatcherTest { + /** + * A watcher fed a scripted sequence of cumulative readings, advancing one step per sample. + */ + private class Fixture( + rx: List, + tx: List = rx, + ) { + private var index = -1 + private val rxReadings = rx + private val txReadings = tx + + val watcher = + NetworkUsageWatcher( + uid = TEST_UID, + readRxBytes = { rxReadings[index.coerceIn(0, rxReadings.lastIndex)] }, + readTxBytes = { txReadings[index.coerceIn(0, txReadings.lastIndex)] }, + ) + + /** Takes [count] samples, walking the scripted readings. */ + fun sample(count: Int) { + repeat(count) { + index++ + watcher.sampleOnce() + } + } + } + + /** The last [count] recorded samples, ignoring the leading zeros of an unfilled buffer. */ + private fun LongArray.recent(count: Int): List = takeLast(count) + + @Test + fun `history is all zeros before the first sample`() { + val fixture = Fixture(listOf(5_000L)) + + val usage = fixture.watcher.getUsage() + + assertThat(usage.received).hasLength(NetworkUsageWatcher.MAX_USAGE_ENTRIES) + assertThat(usage.received.sum()).isEqualTo(0L) + assertThat(usage.transmitted.sum()).isEqualTo(0L) + } + + @Test + fun `plots deltas between samples, not the cumulative counters`() { + // Cumulative since boot: 1000, then +500, then +2500. + val fixture = Fixture(listOf(1_000L, 1_500L, 4_000L)) + + fixture.sample(3) + val usage = fixture.watcher.getUsage() + + // The first sample only establishes a baseline, so it contributes 0 rather than a + // 1000-byte spike for traffic that happened before the chart existed. + assertThat(usage.received.recent(3)).containsExactly(0L, 500L, 2_500L).inOrder() + assertThat(usage.transmitted.recent(3)).containsExactly(0L, 500L, 2_500L).inOrder() + } + + @Test + fun `a counter reset records zero rather than negative traffic`() { + // A reboot or re-based accounting makes the counter go backwards. + val fixture = Fixture(listOf(10_000L, 10_400L, 200L, 700L)) + + fixture.sample(4) + val usage = fixture.watcher.getUsage() + + assertThat(usage.received.recent(4)).containsExactly(0L, 400L, 0L, 500L).inOrder() + assertThat(usage.received.none { it < 0L }).isTrue() + } + + @Test + fun `received and transmitted are accounted separately`() { + val fixture = + Fixture( + rx = listOf(0L, 1_000L), + tx = listOf(0L, 7L), + ) + + fixture.sample(2) + val usage = fixture.watcher.getUsage() + + assertThat(usage.received.recent(2)).containsExactly(0L, 1_000L).inOrder() + assertThat(usage.transmitted.recent(2)).containsExactly(0L, 7L).inOrder() + } + + @Test + fun `the ring buffer keeps only the most recent samples`() { + val capacity = NetworkUsageWatcher.MAX_USAGE_ENTRIES + // Cumulative readings rising by 10 bytes each sample, for one more sample than fits. + val readings = List(capacity + 2) { it * 10L } + val fixture = Fixture(readings) + + fixture.sample(readings.size) + val usage = fixture.watcher.getUsage() + + assertThat(usage.received).hasLength(capacity) + // The baseline zero has been pushed out; every retained sample is a full 10-byte delta. + assertThat(usage.received.toList()).containsNoneIn(listOf(-10L)) + assertThat(usage.received.last()).isEqualTo(10L) + assertThat(usage.received.sum()).isEqualTo(10L * capacity) + } + + @Test + fun `an unsupported counter is detected and nothing is recorded`() { + // TrafficStats.UNSUPPORTED is -1. + val fixture = Fixture(listOf(-1L)) + + fixture.sample(2) + val usage = fixture.watcher.getUsage() + + assertThat(fixture.watcher.isSupported).isFalse() + // In particular, -1 is not plotted as traffic. + assertThat(usage.received.sum()).isEqualTo(0L) + assertThat(usage.transmitted.sum()).isEqualTo(0L) + } + + @Test + fun `getUsage returns a copy, not the live buffer`() { + val fixture = Fixture(listOf(0L, 100L, 300L)) + + fixture.sample(2) + val first = fixture.watcher.getUsage() + val asHandedOut = first.received.copyOf() + fixture.sample(1) + + // The array handed out earlier must not have been mutated by the later sample. + assertThat(first.received).isEqualTo(asHandedOut) + assertThat(fixture.watcher.getUsage().received).isNotEqualTo(asHandedOut) + } + + private companion object { + const val TEST_UID = 10_123 + } +} diff --git a/resources/src/main/res/values/strings.xml b/resources/src/main/res/values/strings.xml index 808f226785..1b9946fa0c 100644 --- a/resources/src/main/res/values/strings.xml +++ b/resources/src/main/res/values/strings.xml @@ -1675,8 +1675,10 @@ Memory usage chart Memory usage - Code On The Go - Code On The Go logo + Network traffic chart + Network traffic + Received + Sent From d668f782584ffd932ab860f286848fb674b330b3 Mon Sep 17 00:00:00 2001 From: David Schachter Date: Fri, 4 Sep 2026 16:34:55 -0700 Subject: [PATCH 2/5] fix: label the network axis in whole units and rest zero on the baseline Two axis problems, one cosmetic and one a real rendering bug. Labels now read "10 kB" rather than "10.0kB". Gridlines sit on whole decades (granularity 1), so the mantissa is always exact and the decimal place carried no information. formatBytes takes the precision as an argument: none for axis labels, one place for the legend, where the figure is an arbitrary sample and the decimal does carry information. A space separates value from unit throughout. Zero now rests on the baseline. Two causes, both fixed: - The series were scaled against the wrong axis. LineDataSet defaults to axisDependency LEFT, and the labelled axis here is the right one, so the line was positioned by the disabled, auto-ranged left axis while the labels came from the right. The two only agree while both auto-range over the same data; pinning one made them disagree visibly -- an idle chart drew its zero line halfway up a plot whose baseline was labelled 0 B. - The range was not pinned. With every sample zero the data range is degenerate and the chart pads around it. applyAxisRange now fixes the minimum at 0 and the maximum at whole decades above the peak, with a floor of three decades so an idle chart keeps a sensible scale instead of collapsing onto a single value. Worth noting for review: the unit tests asserting axisMinimum and axisMaximum passed throughout, because the axis really was configured correctly -- the data simply was not drawn against it. Only the device showed it. There is now a test asserting the axis dependency of both series, which is the part that was untested. MemoryUsageChartRenderer has the same LEFT-dependency-with-RIGHT-labels shape and renders correctly, because it pins neither axis and both auto-range over the same data. Left alone. Verified on a Pixel 6 Pro (arm64), v8 debug: - Idle: both series rest exactly on the 0 B baseline, axis reads 0 B / 10 B / 100 B / 1 kB. - Under a Gradle sync: axis grows to 10 kB, peaks and zero-traffic troughs both legible, legend reads "212 B/s". - 49 tests green across app ui/utils, including four new ones covering the axis range, its growth across both series, whole-unit labels, and the axis dependency. ADFA-5489 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01QeW3M24yD6HNhEnbuNW7Hz --- .../ui/NetworkUsageChartRenderer.kt | 61 ++++++++++++++---- .../ui/NetworkUsageChartRendererTest.kt | 63 ++++++++++++++++++- 2 files changed, 111 insertions(+), 13 deletions(-) diff --git a/app/src/main/java/com/itsaky/androidide/ui/NetworkUsageChartRenderer.kt b/app/src/main/java/com/itsaky/androidide/ui/NetworkUsageChartRenderer.kt index 13061c2a35..2dae31b2f2 100644 --- a/app/src/main/java/com/itsaky/androidide/ui/NetworkUsageChartRenderer.kt +++ b/app/src/main/java/com/itsaky/androidide/ui/NetworkUsageChartRenderer.kt @@ -20,6 +20,7 @@ package com.itsaky.androidide.ui import android.graphics.Color import androidx.annotation.UiThread import com.github.mikephil.charting.components.AxisBase +import com.github.mikephil.charting.components.YAxis import com.github.mikephil.charting.data.Entry import com.github.mikephil.charting.data.LineData import com.github.mikephil.charting.data.LineDataSet @@ -28,7 +29,9 @@ import com.itsaky.androidide.R import com.itsaky.androidide.utils.NetworkUsageWatcher import com.itsaky.androidide.utils.NetworkUsageWatcher.NetworkUsage import com.itsaky.androidide.utils.resolveAttr +import kotlin.math.ceil import kotlin.math.log10 +import kotlin.math.max import kotlin.math.pow import kotlin.math.roundToLong @@ -98,6 +101,8 @@ class NetworkUsageChartRenderer( dataset(usage.transmitted, chart.context.getString(R.string.metrics_network_transmitted), TRANSMITTED_COLOR), ) + applyAxisRange(chart, usage) + chart.apply { data = LineData(*datasets) axisRight.textColor = textColor @@ -139,6 +144,8 @@ class NetworkUsageChartRenderer( update(received, usage.received, chart.context.getString(R.string.metrics_network_received)) update(transmitted, usage.transmitted, chart.context.getString(R.string.metrics_network_transmitted)) + applyAxisRange(chart, usage) + chart.apply { data.notifyDataChanged() notifyDataSetChanged() @@ -155,6 +162,11 @@ class NetworkUsageChartRenderer( List(samples.size) { index -> Entry(index.toFloat(), samples[index].toLogBytes()) }, label, ).apply { + // The labelled axis is the right one, and applyAxisRange pins its range. Without this the + // series is scaled against the (disabled, auto-ranged) left axis instead, so the line is + // drawn at a position the labels do not describe -- an idle chart plots its zero line + // halfway up a plot whose baseline is labelled 0 B. + axisDependency = YAxis.AxisDependency.RIGHT color = lineColor setDrawIcons(false) setDrawCircles(false) @@ -181,7 +193,24 @@ class NetworkUsageChartRenderer( private fun labelFor( label: String, bytes: Long, - ): String = "%s - %s/s".format(label, formatBytes(bytes.toDouble())) + ): String = "%s - %s/s".format(label, formatBytes(bytes.toDouble(), decimals = 1)) + + /** + * Pins the axis to whole decades, from zero up to at least [MIN_AXIS_DECADES]. + * + * Two things depend on this. Zero has to sit on the baseline: when every sample is zero -- an + * idle IDE -- the data range is degenerate, and left to itself the chart pads around it and + * floats the flat line up the middle of the plot. And the maximum has to be a whole number, so + * the gridlines (granularity 1) land on exact powers of ten and can be labelled as whole units. + */ + private fun applyAxisRange( + chart: SafeLineChart, + usage: NetworkUsage, + ) { + val peak = max(usage.received.maxOrNull() ?: 0L, usage.transmitted.maxOrNull() ?: 0L) + chart.axisRight.axisMinimum = 0f + chart.axisRight.axisMaximum = ceil(peak.toLogBytes()).coerceAtLeast(MIN_AXIS_DECADES) + } private fun configure(chart: SafeLineChart) { chart.apply { @@ -199,10 +228,8 @@ class NetworkUsageChartRenderer( axisLeft.isEnabled = false axisRight.valueFormatter = BytesAxisFormatter - // Without a floor the axis auto-scales to the noise around zero when nothing is happening. - axisRight.axisMinimum = 0f - // One label per decade, so the gridlines read as 1.0kB / 1.0MB rather than arbitrary - // fractions of a logarithm. + // One label per decade, so the gridlines read as 1 kB / 1 MB rather than arbitrary + // fractions of a logarithm. The range itself is set per sample by applyAxisRange. axisRight.granularity = 1f axisRight.isGranularityEnabled = true } @@ -225,13 +252,20 @@ class NetworkUsageChartRenderer( axis: AxisBase?, ): String = if (value < 0.5f) { - formatBytes(0.0) + formatBytes(0.0, decimals = 0) } else { - formatBytes(10.0.pow(value.toDouble())) + // Gridlines are whole decades, so the mantissa is exact and needs no decimal place. + formatBytes(10.0.pow(value.toDouble()), decimals = 0) } } private companion object { + /** + * The axis always spans at least this many decades (0 B to 1 kB), so an idle chart keeps a + * sensible scale instead of collapsing onto a single value. + */ + const val MIN_AXIS_DECADES = 3f + const val SERIES_COUNT = 2 const val RECEIVED_INDEX = 0 const val TRANSMITTED_INDEX = 1 @@ -256,12 +290,15 @@ private fun Long.toLogBytes(): Float = log10(this.coerceAtLeast(0L).toDouble() + * and dividing those by 1024 would label them 9.8KB, 977KB, 954MB -- the decades stop looking like * decades. Decimal units are also the convention for network throughput. */ -private fun formatBytes(bytes: Double): String { +private fun formatBytes( + bytes: Double, + decimals: Int, +): String { val clamped = bytes.coerceAtLeast(0.0) return when { - clamped < 1_000 -> "%dB".format(clamped.roundToLong()) - clamped < 1_000_000 -> "%.1fkB".format(clamped / 1_000) - clamped < 1_000_000_000 -> "%.1fMB".format(clamped / 1_000_000) - else -> "%.1fGB".format(clamped / 1_000_000_000) + clamped < 1_000 -> "%d B".format(clamped.roundToLong()) + clamped < 1_000_000 -> "%.${decimals}f kB".format(clamped / 1_000) + clamped < 1_000_000_000 -> "%.${decimals}f MB".format(clamped / 1_000_000) + else -> "%.${decimals}f GB".format(clamped / 1_000_000_000) } } diff --git a/app/src/test/java/com/itsaky/androidide/ui/NetworkUsageChartRendererTest.kt b/app/src/test/java/com/itsaky/androidide/ui/NetworkUsageChartRendererTest.kt index 95b81c8c6a..d24b027cd8 100644 --- a/app/src/test/java/com/itsaky/androidide/ui/NetworkUsageChartRendererTest.kt +++ b/app/src/test/java/com/itsaky/androidide/ui/NetworkUsageChartRendererTest.kt @@ -19,6 +19,7 @@ package com.itsaky.androidide.ui import android.content.Context import androidx.test.core.app.ApplicationProvider +import com.github.mikephil.charting.components.YAxis import com.github.mikephil.charting.data.LineDataSet import com.google.common.truth.Truth.assertThat import com.itsaky.androidide.utils.NetworkUsageWatcher @@ -107,7 +108,7 @@ class NetworkUsageChartRendererTest { // Rendered from the raw byte count, not from the logarithm, and in decimal units so that // the log10 axis labels come out as clean decades. - assertThat(dataset(chart, 0).label).endsWith("2.0kB/s") + assertThat(dataset(chart, 0).label).endsWith("2.0 kB/s") } @Test @@ -155,6 +156,66 @@ class NetworkUsageChartRendererTest { assertThat(dataset(rebound, 0).entries.last().y).isEqualTo(2f) } + @Test + fun `axis labels are whole units with no decimal place`() { + val (_, chart) = rendererFor(usage(longArrayOf(0L, 10_000_000L))) + val formatter = chart.axisRight.valueFormatter + + // Gridlines sit on whole decades, so the mantissa is exact. + assertThat(formatter.getFormattedValue(0f, chart.axisRight)).isEqualTo("0 B") + assertThat(formatter.getFormattedValue(1f, chart.axisRight)).isEqualTo("10 B") + assertThat(formatter.getFormattedValue(3f, chart.axisRight)).isEqualTo("1 kB") + assertThat(formatter.getFormattedValue(4f, chart.axisRight)).isEqualTo("10 kB") + assertThat(formatter.getFormattedValue(6f, chart.axisRight)).isEqualTo("1 MB") + } + + @Test + fun `the series are scaled against the labelled axis`() { + val (_, chart) = rendererFor(usage(longArrayOf(0L, 100L))) + + // The right axis is the one carrying the labels and the pinned range. A dataset left on the + // default LEFT dependency is drawn against the auto-ranged left axis, so the line lands + // somewhere the labels do not describe -- which is invisible to an assertion on the axis + // alone, and was only caught on a device. + assertThat(dataset(chart, 0).axisDependency).isEqualTo(YAxis.AxisDependency.RIGHT) + assertThat(dataset(chart, 1).axisDependency).isEqualTo(YAxis.AxisDependency.RIGHT) + } + + @Test + fun `an idle chart keeps zero on the baseline`() { + // Every sample zero. Left to itself the chart pads around a degenerate range and floats the + // flat line up the middle of the plot instead of resting it on the axis minimum. + val (_, chart) = rendererFor(usage(LongArray(30) { 0L })) + + assertThat(chart.axisRight.axisMinimum).isEqualTo(0f) + assertThat(chart.axisRight.axisMaximum).isEqualTo(3f) + } + + @Test + fun `the axis grows to whole decades around the peak`() { + // 2 MB peak -> log10 is ~6.3, so the axis tops out at the 10 MB decade. + val (_, chart) = rendererFor(usage(longArrayOf(0L, 2_000_000L))) + + assertThat(chart.axisRight.axisMinimum).isEqualTo(0f) + assertThat(chart.axisRight.axisMaximum).isEqualTo(7f) + } + + @Test + fun `the axis follows the peak across both series`() { + val chart = SafeLineChart(context) + var current = usage(longArrayOf(0L, 100L)) + val renderer = NetworkUsageChartRenderer(usageProvider = { current }) + renderer.attach(chart) + + assertThat(chart.axisRight.axisMaximum).isEqualTo(3f) + + // A burst on the transmitted series alone must still lift the axis. + current = usage(received = longArrayOf(0L, 100L), transmitted = longArrayOf(0L, 500_000L)) + renderer.onUsageChanged(current) + + assertThat(chart.axisRight.axisMaximum).isEqualTo(6f) + } + @Test fun `onUsageChanged after detach is a no-op`() { val current = usage(longArrayOf(0L, 99L)) From 245d95de5133d4cdfa434301e8646d48963f643d Mon Sep 17 00:00:00 2001 From: David Schachter Date: Sat, 5 Sep 2026 22:20:58 -0700 Subject: [PATCH 3/5] fix(metrics): make the network sampling loop stoppable and crash-proof (ADFA-5489) CodeRabbit raised three Major findings against this watcher. They were fixed, but on #1785 -- a later PR in the stack than the one that ships the bug. This PR is already approved and ahead of that one, so on its own it still carried all three. Moving the fix to where the defect lives. The scope had no parent Job and startWatching() supplied its own SupervisorJob per launch, so nothing the scope did could cancel the sampler. stopWatching() only lowered a flag the loop checks once per interval, and the loop spends nearly all its time in delay() -- up to 60s once ADFA-5486 makes the rate configurable. A stop and start inside that window left two loops appending to one buffer, splitting each delta between them. The scope now has a parent job, the launch is stored, and stopWatching() cancels it. Nothing caught exceptions inside the loop. An exception -- a misbehaving listener is enough -- ended the coroutine while `watching` stayed true, so every later startWatching() was refused as "already watching" and sampling was dead for the rest of the session. The body is wrapped, and CancellationException is rethrown so structured cancellation still works. The dedicated sampling thread was never released. close() is separate from stopWatching() on purpose: the editor stops and restarts the watcher across its lifecycle, and only the terminal teardown should give up the thread that newSingleThreadContext keeps alive. The activity's destroy path calls it. startWatching() now guards with compareAndSet rather than a read followed by a write, so two callers racing cannot each start a sampler. The watcher takes its dispatchers as parameters, matching MemoryUsageWatcher, so NetworkWatcherLifecycleTest can drive the loop on a virtual clock. Waiting on the wall clock is what hung the test executor the first time this was attempted. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01QeW3M24yD6HNhEnbuNW7Hz --- .../activities/editor/BaseEditorActivity.kt | 5 +- .../androidide/utils/NetworkUsageWatcher.kt | 340 ++++++++++-------- .../utils/NetworkWatcherLifecycleTest.kt | 132 +++++++ 3 files changed, 328 insertions(+), 149 deletions(-) create mode 100644 app/src/test/java/com/itsaky/androidide/utils/NetworkWatcherLifecycleTest.kt 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 05ff076332..39aef60a74 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 @@ -541,8 +541,9 @@ abstract class BaseEditorActivity : if (isDestroying) { memoryUsageWatcher.stopWatching(true) memoryUsageWatcher.listener = null - networkUsageWatcher.stopWatching() - networkUsageWatcher.listener = null + // close(), not stopWatching(): this is the terminal teardown, and the watcher holds a + // dedicated sampling thread that newSingleThreadContext keeps alive until it is closed. + networkUsageWatcher.close() editorActivityScope.cancelIfActive("Activity is being destroyed") unbindDebuggerService() 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 09e2f52e34..ea167568c7 100644 --- a/app/src/main/java/com/itsaky/androidide/utils/NetworkUsageWatcher.kt +++ b/app/src/main/java/com/itsaky/androidide/utils/NetworkUsageWatcher.kt @@ -21,10 +21,13 @@ import android.net.TrafficStats import android.os.Process import androidx.annotation.VisibleForTesting import com.itsaky.androidide.tasks.cancelIfActive +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.DelicateCoroutinesApi import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.ExecutorCoroutineDispatcher import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.Job import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.delay import kotlinx.coroutines.launch @@ -32,6 +35,7 @@ import kotlinx.coroutines.newSingleThreadContext import kotlinx.coroutines.withContext import org.slf4j.LoggerFactory import java.util.concurrent.atomic.AtomicBoolean +import kotlin.coroutines.CoroutineContext /** * Samples this app's network traffic (ADFA-5489). @@ -50,177 +54,219 @@ import java.util.concurrent.atomic.AtomicBoolean * @param readRxBytes Reads the cumulative received byte count. Injectable for tests. * @param readTxBytes Reads the cumulative transmitted byte count. Injectable for tests. */ -class NetworkUsageWatcher( - private val updateInterval: Long = DEFAULT_UPDATE_INTERVAL, - private val uid: Int = Process.myUid(), - private val readRxBytes: (Int) -> Long = TrafficStats::getUidRxBytes, - private val readTxBytes: (Int) -> Long = TrafficStats::getUidTxBytes, -) { +class NetworkUsageWatcher @OptIn(ExperimentalCoroutinesApi::class, DelicateCoroutinesApi::class) - private val coroutineDispatcher = newSingleThreadContext("NetworkUsageWatcher") - private val coroutineScope = CoroutineScope(coroutineDispatcher) - private val watching = AtomicBoolean(false) - - /** Guards the two ring buffers: the sampler writes them, the UI thread snapshots them. */ - private val historyLock = Any() - - private val received = MutableShiftedLongArray(MAX_USAGE_ENTRIES) - private val transmitted = MutableShiftedLongArray(MAX_USAGE_ENTRIES) - - /** - * The previous cumulative readings, or `null` before the first sample. The first sample - * establishes a baseline and contributes no delta -- the alternative would be a spike equal to - * everything the app had transferred since boot. - */ - private var lastRx: Long? = null - private var lastTx: Long? = null - - /** - * Whether the platform reports traffic for this UID at all. Cleared permanently if a read comes - * back [TrafficStats.UNSUPPORTED], which some devices and emulators do. - */ - @Volatile - var isSupported: Boolean = true - private set - - val isWatching: Boolean - get() = watching.get() - - /** - * Notified on the main thread after each sample. - */ - var listener: NetworkUsageListener? = null - - /** - * A snapshot of the sampled history, oldest first. Safe to call from any thread at any time; - * before the first sample every entry is zero. - * - * The arrays are copies. Handing out the live ring buffers would let the caller read them while - * the sampler thread is midway through appending, and the chart renderer reads all 30 entries. - */ - fun getUsage(): NetworkUsage = - synchronized(historyLock) { - NetworkUsage(received.snapshot(), transmitted.snapshot()) - } + constructor( + private val updateInterval: Long = DEFAULT_UPDATE_INTERVAL, + private val uid: Int = Process.myUid(), + private val readRxBytes: (Int) -> Long = TrafficStats::getUidRxBytes, + private val readTxBytes: (Int) -> Long = TrafficStats::getUidTxBytes, + // Injectable so a test can drive the sampling loop on a virtual clock. Waiting on the wall + // clock instead is what hung the test executor the first time this was attempted. + private val coroutineDispatcher: CoroutineContext = newSingleThreadContext("NetworkUsageWatcher"), + private val mainDispatcher: CoroutineContext = Dispatchers.Main.immediate, + ) { + // A parent job, so cancelling the scope in close() actually reaches the sampler. Without one + // the launch below had to supply its own, and nothing the scope did could stop it. + private val coroutineScope = CoroutineScope(SupervisorJob() + coroutineDispatcher) + private val watching = AtomicBoolean(false) - fun startWatching() { - if (isWatching) { - log.warn("Network usage is already being watched") - return - } + /** The running sampling loop, so [stopWatching] can actually stop it. */ + private var samplingJob: Job? = null - watching.set(true) + /** Guards the two ring buffers: the sampler writes them, the UI thread snapshots them. */ + private val historyLock = Any() - coroutineScope.launch(context = SupervisorJob() + coroutineDispatcher) { - while (isWatching) { - sampleOnce() + private val received = MutableShiftedLongArray(MAX_USAGE_ENTRIES) + private val transmitted = MutableShiftedLongArray(MAX_USAGE_ENTRIES) - listener?.also { listener -> - val usage = getUsage() - withContext(Dispatchers.Main.immediate) { - listener.onNetworkUsageChanged(usage) - } - } + /** + * The previous cumulative readings, or `null` before the first sample. The first sample + * establishes a baseline and contributes no delta -- the alternative would be a spike equal to + * everything the app had transferred since boot. + */ + private var lastRx: Long? = null + private var lastTx: Long? = null - delay(updateInterval) + /** + * Whether the platform reports traffic for this UID at all. Cleared permanently if a read comes + * back [TrafficStats.UNSUPPORTED], which some devices and emulators do. + */ + @Volatile + var isSupported: Boolean = true + private set + + val isWatching: Boolean + get() = watching.get() + + /** + * Notified on the main thread after each sample. + */ + var listener: NetworkUsageListener? = null + + /** + * A snapshot of the sampled history, oldest first. Safe to call from any thread at any time; + * before the first sample every entry is zero. + * + * The arrays are copies. Handing out the live ring buffers would let the caller read them while + * the sampler thread is midway through appending, and the chart renderer reads all 30 entries. + */ + fun getUsage(): NetworkUsage = + synchronized(historyLock) { + NetworkUsage(received.snapshot(), transmitted.snapshot()) } - } - } - fun stopWatching() { - watching.set(false) - coroutineScope.cancelIfActive("Cancellation requested") - } + fun startWatching() { + // compareAndSet, not a read then a write: two callers racing here would each start a + // sampler, and both would append to the same buffers. + if (!watching.compareAndSet(false, true)) { + log.warn("Network usage is already being watched") + return + } - /** - * Takes one sample. The sampling loop calls this once per [updateInterval]; tests call it - * directly so the delta accounting can be exercised without threads or waiting. - */ - @VisibleForTesting - internal fun sampleOnce() { - if (!isSupported) { - return - } + samplingJob = + coroutineScope.launch(context = SupervisorJob() + coroutineDispatcher) { + while (isWatching) { + // The loop must outlive a bad sample. Without this an exception -- a + // misbehaving listener is enough -- ends the coroutine while `watching` stays + // true, so every later startWatching() is refused as "already watching" and + // sampling is dead for the rest of the session. + runCatching { + sampleOnce() - val rx = readRxBytes(uid) - val tx = readTxBytes(uid) + listener?.also { listener -> + val usage = getUsage() + withContext(mainDispatcher) { + listener.onNetworkUsageChanged(usage) + } + } + }.onFailure { failure -> + if (failure is CancellationException) { + throw failure + } + log.error("Network usage sampling failed; continuing", failure) + } - if (rx == UNSUPPORTED || tx == UNSUPPORTED) { - // Not transient: the platform either accounts for this UID or it does not. - isSupported = false - log.info("Network usage is unavailable on this device; the traffic chart will read zero") - return + delay(updateInterval) + } + } } - synchronized(historyLock) { - record(received, previous = lastRx, current = rx) - record(transmitted, previous = lastTx, current = tx) + /** + * Stops sampling. The watcher can be started again; the history is kept. + */ + fun stopWatching() { + watching.set(false) + // Cancel the job, not the scope. The loop spends nearly all its time in delay(), so waiting + // for it to notice the flag leaves it sampling for up to a full interval after the editor + // asked it to stop -- long enough for a stop/start to run two samplers at once. Cancelling + // the scope instead would end the watcher for good, and this is a pause, not a teardown. + coroutineScope.cancelIfActive("Cancellation requested") } - lastRx = rx - lastTx = tx - } + /** + * Stops sampling and releases the sampling thread. Terminal: the watcher cannot be restarted. + * + * Separate from [stopWatching] because the editor stops and restarts the watcher across its + * lifecycle, and only the final teardown should give up the thread that + * [newSingleThreadContext] keeps alive. + */ + fun close() { + stopWatching() + listener = null + coroutineScope.cancelIfActive("Watcher closed") + (coroutineDispatcher as? ExecutorCoroutineDispatcher)?.close() + } - /** - * Appends the delta between [previous] and [current] to [history]. - * - * A negative delta means the counter went backwards, which happens when it is reset -- the - * device rebooted, or the platform re-based its accounting. Treated as a fresh baseline (zero - * for this interval) rather than plotted as negative traffic. - */ - private fun record( - history: MutableShiftedLongArray, - previous: Long?, - current: Long, - ) { - val delta = - when { - previous == null -> 0L - current < previous -> 0L - else -> current - previous + /** + * Takes one sample. The sampling loop calls this once per [updateInterval]; tests call it + * directly so the delta accounting can be exercised without threads or waiting. + */ + @VisibleForTesting + internal fun sampleOnce() { + if (!isSupported) { + return } - // Newest entry goes in at index 0 and the shift makes it the last element, so - // history[size - 1] is always the newest. Same convention as MemoryUsageWatcher. - history[0] = delta - history.shift(1) - } + val rx = readRxBytes(uid) + val tx = readTxBytes(uid) - /** - * Bytes transferred per sampling interval, oldest first. - * - * @property received Bytes received during each interval. - * @property transmitted Bytes transmitted during each interval. - */ - data class NetworkUsage( - val received: LongArray, - val transmitted: LongArray, - ) { - override fun equals(other: Any?): Boolean = - this === other || - ( - other is NetworkUsage && - received.contentEquals(other.received) && - transmitted.contentEquals(other.transmitted) - ) - - override fun hashCode(): Int = 31 * received.contentHashCode() + transmitted.contentHashCode() - } + if (rx == UNSUPPORTED || tx == UNSUPPORTED) { + // Not transient: the platform either accounts for this UID or it does not. + isSupported = false + log.info("Network usage is unavailable on this device; the traffic chart will read zero") + return + } - fun interface NetworkUsageListener { - fun onNetworkUsageChanged(usage: NetworkUsage) - } + synchronized(historyLock) { + record(received, previous = lastRx, current = rx) + record(transmitted, previous = lastTx, current = tx) + } + + lastRx = rx + lastTx = tx + } + + /** + * Appends the delta between [previous] and [current] to [history]. + * + * A negative delta means the counter went backwards, which happens when it is reset -- the + * device rebooted, or the platform re-based its accounting. Treated as a fresh baseline (zero + * for this interval) rather than plotted as negative traffic. + */ + private fun record( + history: MutableShiftedLongArray, + previous: Long?, + current: Long, + ) { + val delta = + when { + previous == null -> 0L + current < previous -> 0L + else -> current - previous + } - companion object { - const val MAX_USAGE_ENTRIES = 30 - const val DEFAULT_UPDATE_INTERVAL = 1000L + // Newest entry goes in at index 0 and the shift makes it the last element, so + // history[size - 1] is always the newest. Same convention as MemoryUsageWatcher. + history[0] = delta + history.shift(1) + } - /** [TrafficStats.UNSUPPORTED] widened to [Long], which is what the getters return. */ - private const val UNSUPPORTED = TrafficStats.UNSUPPORTED.toLong() + /** + * Bytes transferred per sampling interval, oldest first. + * + * @property received Bytes received during each interval. + * @property transmitted Bytes transmitted during each interval. + */ + data class NetworkUsage( + val received: LongArray, + val transmitted: LongArray, + ) { + override fun equals(other: Any?): Boolean = + this === other || + ( + other is NetworkUsage && + received.contentEquals(other.received) && + transmitted.contentEquals(other.transmitted) + ) - private val log = LoggerFactory.getLogger(NetworkUsageWatcher::class.java) + override fun hashCode(): Int = 31 * received.contentHashCode() + transmitted.contentHashCode() + } + + fun interface NetworkUsageListener { + fun onNetworkUsageChanged(usage: NetworkUsage) + } + + companion object { + const val MAX_USAGE_ENTRIES = 30 + const val DEFAULT_UPDATE_INTERVAL = 1000L + + /** [TrafficStats.UNSUPPORTED] widened to [Long], which is what the getters return. */ + private const val UNSUPPORTED = TrafficStats.UNSUPPORTED.toLong() + + private val log = LoggerFactory.getLogger(NetworkUsageWatcher::class.java) + } } -} /** * Copies this ring buffer into a plain array in logical order, oldest first. diff --git a/app/src/test/java/com/itsaky/androidide/utils/NetworkWatcherLifecycleTest.kt b/app/src/test/java/com/itsaky/androidide/utils/NetworkWatcherLifecycleTest.kt new file mode 100644 index 0000000000..3b69d60fc4 --- /dev/null +++ b/app/src/test/java/com/itsaky/androidide/utils/NetworkWatcherLifecycleTest.kt @@ -0,0 +1,132 @@ +/* + * 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 kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.advanceTimeBy +import kotlinx.coroutines.test.runTest +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +/** + * Pins the sampling loop's lifecycle (ADFA-5489). + * + * The loop spends nearly all of its time in `delay()`, so "stopped" cannot mean "will notice a + * flag eventually": between the request and the next tick the watcher is still sampling, and a + * stop followed by a start inside that window used to leave two loops appending to one buffer. + * + * Driven on a virtual clock. Waiting on the wall clock instead is what hung the test executor the + * first time this was attempted. + */ +@OptIn(ExperimentalCoroutinesApi::class) +@RunWith(RobolectricTestRunner::class) +class NetworkWatcherLifecycleTest { + private fun watcher( + dispatcher: kotlin.coroutines.CoroutineContext, + onSample: () -> Unit = {}, + ): NetworkUsageWatcher { + var counter = 0L + return NetworkUsageWatcher( + updateInterval = INTERVAL_MS, + uid = TEST_UID, + readRxBytes = { + onSample() + counter += 100L + counter + }, + readTxBytes = { counter }, + coroutineDispatcher = dispatcher, + mainDispatcher = dispatcher, + ) + } + + @Test + fun `stopping inside the sampling interval actually stops sampling`() = + runTest { + val dispatcher = StandardTestDispatcher(testScheduler) + var samples = 0 + val watcher = watcher(dispatcher) { samples++ } + + watcher.startWatching() + advanceTimeBy(INTERVAL_MS * 3) + val whileRunning = samples + + watcher.stopWatching() + advanceTimeBy(INTERVAL_MS * 5) + + // Cancelling the job rather than waiting for the loop to observe a flag is what makes + // this exact: nothing is sampled after the stop. + assertThat(whileRunning).isGreaterThan(0) + assertThat(samples).isEqualTo(whileRunning) + assertThat(watcher.isWatching).isFalse() + watcher.close() + } + + @Test + fun `restarting inside the sampling interval does not leave two loops running`() = + runTest { + val dispatcher = StandardTestDispatcher(testScheduler) + var samples = 0 + val watcher = watcher(dispatcher) { samples++ } + + watcher.startWatching() + advanceTimeBy(INTERVAL_MS * 2) + watcher.stopWatching() + watcher.startWatching() + + val before = samples + advanceTimeBy(INTERVAL_MS * 4) + val perInterval = (samples - before) / 4 + + // Two loops would double the rate against the same buffer. + assertThat(perInterval).isEqualTo(1) + watcher.close() + } + + @Test + fun `a listener that throws does not kill sampling for the rest of the session`() = + runTest { + val dispatcher = StandardTestDispatcher(testScheduler) + var samples = 0 + val watcher = watcher(dispatcher) { samples++ } + var thrown = 0 + watcher.listener = + NetworkUsageWatcher.NetworkUsageListener { + if (thrown++ == 0) { + throw IllegalStateException("listener blew up") + } + } + + watcher.startWatching() + advanceTimeBy(INTERVAL_MS * 4) + + // Uncaught, the exception ends the coroutine while isWatching stays true, so every + // later startWatching() is refused and the charts freeze for good. + assertThat(samples).isGreaterThan(1) + assertThat(watcher.isWatching).isTrue() + watcher.close() + } + + private companion object { + const val INTERVAL_MS = 1_000L + const val TEST_UID = 10_123 + } +} From afdd6aec0401e11f65d6546af28a0b141bea1633 Mon Sep 17 00:00:00 2001 From: David Schachter Date: Sat, 5 Sep 2026 22:58:51 -0700 Subject: [PATCH 4/5] fix(metrics): actually apply the sampler fix, and stop a failed test spinning The previous commit shipped the commit message for this fix without the fix. An interrupted command had reverted the watcher to its pre-fix shape for a negative check and was killed before it restored it, so what got committed was `launch(SupervisorJob() + dispatcher)` and a scope cancel that cannot reach the sampler -- the very defect being fixed. stopWatching() now cancels the stored job, as its own comment already claimed. That mistake did prove the tests: against the unfixed watcher NetworkWatcherLifecycleTest reported two samples per interval where one was expected, which is exactly the two-loop overlap the fix exists to prevent. The tests also gained the cleanup they should have had. Each body now closes its watcher in a finally. Without it a failed assertion skipped close(), left the sampling loop live, and runTest's trailing advanceUntilIdle advanced virtual time forever -- a synchronous spin no test timeout can interrupt, which pinned a core and took the Gradle task to its ten-minute limit with no output. CodeRabbit raised exactly this about the tests on #1785; the lesson had not been carried over here. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01QeW3M24yD6HNhEnbuNW7Hz --- .../androidide/utils/NetworkUsageWatcher.kt | 5 +- .../utils/NetworkWatcherLifecycleTest.kt | 91 +++++++++++-------- 2 files changed, 58 insertions(+), 38 deletions(-) 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 ea167568c7..7911086b2e 100644 --- a/app/src/main/java/com/itsaky/androidide/utils/NetworkUsageWatcher.kt +++ b/app/src/main/java/com/itsaky/androidide/utils/NetworkUsageWatcher.kt @@ -125,7 +125,7 @@ class NetworkUsageWatcher } samplingJob = - coroutineScope.launch(context = SupervisorJob() + coroutineDispatcher) { + coroutineScope.launch { while (isWatching) { // The loop must outlive a bad sample. Without this an exception -- a // misbehaving listener is enough -- ends the coroutine while `watching` stays @@ -161,7 +161,8 @@ class NetworkUsageWatcher // for it to notice the flag leaves it sampling for up to a full interval after the editor // asked it to stop -- long enough for a stop/start to run two samplers at once. Cancelling // the scope instead would end the watcher for good, and this is a pause, not a teardown. - coroutineScope.cancelIfActive("Cancellation requested") + samplingJob?.cancel() + samplingJob = null } /** diff --git a/app/src/test/java/com/itsaky/androidide/utils/NetworkWatcherLifecycleTest.kt b/app/src/test/java/com/itsaky/androidide/utils/NetworkWatcherLifecycleTest.kt index 3b69d60fc4..a63ce316e5 100644 --- a/app/src/test/java/com/itsaky/androidide/utils/NetworkWatcherLifecycleTest.kt +++ b/app/src/test/java/com/itsaky/androidide/utils/NetworkWatcherLifecycleTest.kt @@ -65,19 +65,25 @@ class NetworkWatcherLifecycleTest { var samples = 0 val watcher = watcher(dispatcher) { samples++ } - watcher.startWatching() - advanceTimeBy(INTERVAL_MS * 3) - val whileRunning = samples - - watcher.stopWatching() - advanceTimeBy(INTERVAL_MS * 5) - - // Cancelling the job rather than waiting for the loop to observe a flag is what makes - // this exact: nothing is sampled after the stop. - assertThat(whileRunning).isGreaterThan(0) - assertThat(samples).isEqualTo(whileRunning) - assertThat(watcher.isWatching).isFalse() - watcher.close() + try { + watcher.startWatching() + advanceTimeBy(INTERVAL_MS * 3) + val whileRunning = samples + + watcher.stopWatching() + advanceTimeBy(INTERVAL_MS * 5) + + // Cancelling the job rather than waiting for the loop to observe a flag is what makes + // this exact: nothing is sampled after the stop. + assertThat(whileRunning).isGreaterThan(0) + assertThat(samples).isEqualTo(whileRunning) + assertThat(watcher.isWatching).isFalse() + } finally { + // In a finally: a failed assertion would otherwise leave the sampling loop alive, + // and runTest's trailing advanceUntilIdle then advances virtual time forever. That + // spin is synchronous, so no test timeout can interrupt it -- it just pins a core. + watcher.close() + } } @Test @@ -87,18 +93,24 @@ class NetworkWatcherLifecycleTest { var samples = 0 val watcher = watcher(dispatcher) { samples++ } - watcher.startWatching() - advanceTimeBy(INTERVAL_MS * 2) - watcher.stopWatching() - watcher.startWatching() + try { + watcher.startWatching() + advanceTimeBy(INTERVAL_MS * 2) + watcher.stopWatching() + watcher.startWatching() - val before = samples - advanceTimeBy(INTERVAL_MS * 4) - val perInterval = (samples - before) / 4 + val before = samples + advanceTimeBy(INTERVAL_MS * 4) + val perInterval = (samples - before) / 4 - // Two loops would double the rate against the same buffer. - assertThat(perInterval).isEqualTo(1) - watcher.close() + // Two loops would double the rate against the same buffer. + assertThat(perInterval).isEqualTo(1) + } finally { + // In a finally: a failed assertion would otherwise leave the sampling loop alive, + // and runTest's trailing advanceUntilIdle then advances virtual time forever. That + // spin is synchronous, so no test timeout can interrupt it -- it just pins a core. + watcher.close() + } } @Test @@ -107,22 +119,29 @@ class NetworkWatcherLifecycleTest { val dispatcher = StandardTestDispatcher(testScheduler) var samples = 0 val watcher = watcher(dispatcher) { samples++ } - var thrown = 0 - watcher.listener = - NetworkUsageWatcher.NetworkUsageListener { - if (thrown++ == 0) { - throw IllegalStateException("listener blew up") + + try { + var thrown = 0 + watcher.listener = + NetworkUsageWatcher.NetworkUsageListener { + if (thrown++ == 0) { + throw IllegalStateException("listener blew up") + } } - } - watcher.startWatching() - advanceTimeBy(INTERVAL_MS * 4) + watcher.startWatching() + advanceTimeBy(INTERVAL_MS * 4) - // Uncaught, the exception ends the coroutine while isWatching stays true, so every - // later startWatching() is refused and the charts freeze for good. - assertThat(samples).isGreaterThan(1) - assertThat(watcher.isWatching).isTrue() - watcher.close() + // Uncaught, the exception ends the coroutine while isWatching stays true, so every + // later startWatching() is refused and the charts freeze for good. + assertThat(samples).isGreaterThan(1) + assertThat(watcher.isWatching).isTrue() + } finally { + // In a finally: a failed assertion would otherwise leave the sampling loop alive, + // and runTest's trailing advanceUntilIdle then advances virtual time forever. That + // spin is synchronous, so no test timeout can interrupt it -- it just pins a core. + watcher.close() + } } private companion object { From fb5bc7ba3ea0f01cfbe14960c5a799fa03c777aa Mon Sep 17 00:00:00 2001 From: David Schachter Date: Sun, 6 Sep 2026 07:48:01 -0700 Subject: [PATCH 5/5] fix(metrics): re-baseline on resume, and stop sampling a device that cannot (ADFA-5489) Four review findings on the network watcher. A resume reported the whole gap as one interval. stopWatching() left lastRx and lastTx set, so the first sample afterwards took the delta against a counter read minutes earlier: background a Gradle download for three minutes and the legend read hundreds of MB/s while the axis stretched to match. The baseline is now dropped on stop, which is exactly what the null baseline already means elsewhere -- the next sample re-establishes it and contributes nothing. The baseline was also written outside the lock that clears it. sampleOnce wrote lastRx/lastTx on the sampler thread while clearHistory nulled them on the UI thread, so an interleaving could restore a pre-clear baseline and produce the same spike at the moment the user changed the sampling rate -- the failure the "cumulative baseline is dropped too" test exists to prevent, which it cannot see because it drives sampleOnce synchronously. listener was a plain var written by the UI thread and read by the sampler every tick, with no happens-before edge, so a null written in onPause could go unobserved and the sampler keep dispatching into a paused activity. Now @Volatile, as isSupported on the same class already was for the same reason. A device whose counters are unsupported kept the loop running anyway. isSupported latched false and sampleOnce returned immediately, but every interval still snapshotted the buffers, hopped to the main thread and repainted the chart with data known to be permanently zero. The loop now ends, and clears the watching flag as it goes so isWatching does not claim a sampler that has stopped. Not fixed here, deliberately: the legend's "/s" suffix. It is accurate on this branch, where the interval is a constructor value fixed at one second. It only becomes wrong once ADFA-5486 makes the rate user-settable, and only that branch has the interval available to the renderer, so the fix belongs there. Co-Authored-By: Claude Opus 5 --- .../androidide/utils/NetworkUsageWatcher.kt | 24 ++++++++++++++-- .../utils/NetworkUsageWatcherTest.kt | 28 +++++++++++++++++++ 2 files changed, 50 insertions(+), 2 deletions(-) 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 7911086b2e..9499f527d9 100644 --- a/app/src/main/java/com/itsaky/androidide/utils/NetworkUsageWatcher.kt +++ b/app/src/main/java/com/itsaky/androidide/utils/NetworkUsageWatcher.kt @@ -102,6 +102,7 @@ class NetworkUsageWatcher /** * Notified on the main thread after each sample. */ + @Volatile var listener: NetworkUsageListener? = null /** @@ -147,6 +148,15 @@ class NetworkUsageWatcher log.error("Network usage sampling failed; continuing", failure) } + // A device whose counters are unsupported has nothing further to give, and + // the loop was otherwise repainting three charts a second with data known + // to be permanently zero. Clearing the flag too, so isWatching does not + // claim a sampler that has stopped. + if (!isSupported) { + watching.set(false) + break + } + delay(updateInterval) } } @@ -157,6 +167,14 @@ class NetworkUsageWatcher */ fun stopWatching() { watching.set(false) + // Drop the cumulative baseline as well. Left set, the first sample after a resume + // reports everything transferred while the watcher was stopped as a single interval -- + // background a Gradle download for three minutes and the chart reads hundreds of MB/s. + // The next sample re-establishes it, which is what the null baseline means. + synchronized(historyLock) { + lastRx = null + lastTx = null + } // Cancel the job, not the scope. The loop spends nearly all its time in delay(), so waiting // for it to notice the flag leaves it sampling for up to a full interval after the editor // asked it to stop -- long enough for a stop/start to run two samplers at once. Cancelling @@ -204,8 +222,10 @@ class NetworkUsageWatcher record(transmitted, previous = lastTx, current = tx) } - lastRx = rx - lastTx = tx + synchronized(historyLock) { + lastRx = rx + lastTx = tx + } } /** diff --git a/app/src/test/java/com/itsaky/androidide/utils/NetworkUsageWatcherTest.kt b/app/src/test/java/com/itsaky/androidide/utils/NetworkUsageWatcherTest.kt index c21420595a..2ec49fbf69 100644 --- a/app/src/test/java/com/itsaky/androidide/utils/NetworkUsageWatcherTest.kt +++ b/app/src/test/java/com/itsaky/androidide/utils/NetworkUsageWatcherTest.kt @@ -159,6 +159,34 @@ class NetworkUsageWatcherTest { assertThat(fixture.watcher.getUsage().received).isNotEqualTo(asHandedOut) } + @Test + fun `stopping drops the cumulative baseline so a resume does not spike`() { + // 1 MB transferred, then the watcher is stopped while a download keeps running. + val fixture = Fixture(listOf(1_000_000L, 1_000_000L, 250_000_000L, 250_500_000L)) + fixture.sample(2) + + fixture.watcher.stopWatching() + + // Resume: the counter has moved by 249 MB while nothing was watching. + fixture.sample(2) + val usage = fixture.watcher.getUsage() + + // Kept, the baseline turns the whole gap into one interval's traffic -- the legend reads + // hundreds of MB/s and the axis is stretched for the next minute. + assertThat(usage.received.recent(2)).containsExactly(0L, 500_000L).inOrder() + } + + @Test + fun `an unsupported counter stops the watcher rather than sampling zeroes forever`() { + val fixture = Fixture(listOf(-1L)) + + fixture.sample(1) + + // Nothing more to read, so nothing more to do: the loop was repainting the charts once a + // second with data known to be permanently unavailable. + assertThat(fixture.watcher.isSupported).isFalse() + } + private companion object { const val TEST_UID = 10_123 }