From 57debc0d280b7fd79846d32b304759e11f870fb3 Mon Sep 17 00:00:00 2001 From: David Schachter Date: Sun, 6 Sep 2026 23:18:04 -0700 Subject: [PATCH 1/6] fix: give the tooling server test suite a buildId so it compiles ToolingApiServerImplTest has not compiled since InitializeProjectParams gained a required buildId: every test in :subprojects:tooling-api-impl has been unrunnable, not failing. BuildId.Unknown already exists for callers that have no real build to name, which is exactly this case. Spotless reformats the file on the way past, since touching it enrols it in the ratchet. Separated from the change that needed it so the feature commit is only the feature. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_017CCQUU7tBzZL61EmQhJP8j --- .../tooling/impl/ToolingApiServerImplTest.kt | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/subprojects/tooling-api-impl/src/test/java/com/itsaky/androidide/tooling/impl/ToolingApiServerImplTest.kt b/subprojects/tooling-api-impl/src/test/java/com/itsaky/androidide/tooling/impl/ToolingApiServerImplTest.kt index 21a346df7e..d0b127de24 100644 --- a/subprojects/tooling-api-impl/src/test/java/com/itsaky/androidide/tooling/impl/ToolingApiServerImplTest.kt +++ b/subprojects/tooling-api-impl/src/test/java/com/itsaky/androidide/tooling/impl/ToolingApiServerImplTest.kt @@ -1,6 +1,7 @@ package com.itsaky.androidide.tooling.impl import com.google.common.truth.Truth.assertThat +import com.itsaky.androidide.tooling.api.messages.BuildId import com.itsaky.androidide.tooling.api.messages.InitializeProjectParams import com.itsaky.androidide.tooling.api.messages.result.InitializeResult import com.itsaky.androidide.tooling.api.messages.result.TaskExecutionResult @@ -25,18 +26,19 @@ import java.util.concurrent.TimeUnit */ @RunWith(JUnit4::class) class ToolingApiServerImplTest { - private fun testInitParams( directory: String = "/does/not/exist", forceSync: Boolean = false, ) = InitializeProjectParams( - directory = directory, needsGradleSync = forceSync + directory = directory, + needsGradleSync = forceSync, + buildId = BuildId.Unknown, ) private data class MockServer( val server: ToolingApiServerImpl, val connector: GradleConnector, - val connection: ProjectConnection + val connection: ProjectConnection, ) private fun mockkToolingServer(): MockServer { @@ -47,7 +49,10 @@ class ToolingApiServerImplTest { // ensure that we do not start actual Gradle build every { server.getOrConnectProject( - projectDir = any(), forceConnect = true, initParams = any(), gradleDist = any() + projectDir = any(), + forceConnect = true, + initParams = any(), + gradleDist = any(), ) } returns (connector to connection) @@ -56,12 +61,12 @@ class ToolingApiServerImplTest { @Test fun `GIVEN any initialization params WHEN project init fails THEN report as failure`() { - mockkObject(RootModelBuilder) every { // Simulate a Gradle sync failure RootModelBuilder.build( - any(), any() + any(), + any(), ) } throws RuntimeException("intentional failure") @@ -83,7 +88,6 @@ class ToolingApiServerImplTest { @Test fun `GIVEN force sync not requested WHEN sync files are unreadable THEN sync anyway`() { - val initParams = testInitParams(forceSync = false) val cacheFile = ProjectSyncHelper.cacheFileForProject(File(initParams.directory)) @@ -91,7 +95,8 @@ class ToolingApiServerImplTest { every { // simulate a successful cache write RootModelBuilder.build( - any(), any() + any(), + any(), ) } returns cacheFile From 95176fceb2727267f965e060cf725cd21f7f82ca Mon Sep 17 00:00:00 2001 From: David Schachter Date: Sun, 6 Sep 2026 23:19:08 -0700 Subject: [PATCH 2/6] ADFA-5514: plot the Gradle daemon, the process that actually holds the build BaseEditorActivity has always had a colour ready for PROC_GRADLE_DAEMON and nothing ever passed it: watchProcess was called with the IDE and the tooling server and never the daemon, so the branch colouring it green was unreachable and the legend showed two entries. Measured on a Pixel 6 Pro during a build, the one it left out is the big one -- IDE 702 MB, tooling server 165 MB, daemon 779 MB. The process that runs the compiler, holds the most memory, and is the likeliest reason a build is slow or gets killed on a small device was the one the chart could not show. The client has no handle on the daemon, but the server does: it is the tooling server's own child, which killDescendantProcesses already relies on. So the pid is pushed from the server rather than discovered by scanning, over two new client notifications beside the build events that already exist. Three things the ticket left open, settled by measurement rather than assumption: - Which descendant. Matched on the GradleDaemon main class, not "the only child": this build runs Kotlin compilation with kotlin.compiler.execution.strategy=daemon, so a second JVM is a sibling of the one holding the build's heap. - When to stop. Not on build finish, which is what the ticket assumed. A daemon outlives the build that spawned it -- still resident at 777 MB 144 s after one finished, despite daemon.forceKill -- and an idle daemon holding that much is exactly what a user on a 4 GB phone needs to see. It is unwatched when the process exits, off ProcessHandle.onExit. - Whether it is readable at all. It is, with no new mechanism: same uid as the app, and an ordinary child JVM like the tooling server, which the existing Debug.getMemoryInfo reflection path already reads. Also guards readUsages against a pid that has gone away. Debug.getMemoryInfo leaves its output untouched for a dead process, so sampling one repeats its last reading forever -- a flat line at 800 MB for a daemon that is gone. The IDE and the tooling server live as long as the editor, so this was unreachable until something that comes and goes was plotted. Verified on a Pixel 6 Pro: the daemon was identified within 10 s of the build starting, the chart drew three lines -- Gradle Tooling 163.70MB, Gradle Daemon 795.86MB, IDE 583.18MB, against a measured RSS of 797296 kB -- and killing the daemon fired the exit event and dropped the line without leaving a frozen one behind. Ten unit tests, seven for which descendant is the daemon and when the client hears about it, three for the dead-pid guard; the two guard tests that can fail were run against the unguarded code to confirm they do. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_017CCQUU7tBzZL61EmQhJP8j --- .../activities/editor/BaseEditorActivity.kt | 23 +++ .../handlers/EditorBuildEventListener.kt | 10 + .../services/builder/GradleBuildService.kt | 37 ++++ .../androidide/utils/MemoryUsageWatcher.kt | 41 +++- .../utils/MemoryUsageWatcherLivenessTest.kt | 114 +++++++++++ .../tooling/impl/GradleDaemonWatcher.kt | 170 ++++++++++++++++ .../tooling/impl/ToolingApiServerImpl.kt | 12 ++ .../tooling/impl/GradleDaemonWatcherTest.kt | 189 ++++++++++++++++++ .../tooling/api/ForwardingToolingApiClient.kt | 8 + .../tooling/api/IToolingApiClient.kt | 25 +++ .../testing/tooling/ToolingApiTestLauncher.kt | 8 + 11 files changed, 627 insertions(+), 10 deletions(-) create mode 100644 app/src/test/java/com/itsaky/androidide/utils/MemoryUsageWatcherLivenessTest.kt create mode 100644 subprojects/tooling-api-impl/src/main/java/com/itsaky/androidide/tooling/impl/GradleDaemonWatcher.kt create mode 100644 subprojects/tooling-api-impl/src/test/java/com/itsaky/androidide/tooling/impl/GradleDaemonWatcherTest.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 d1957709da..b19adefcd0 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 @@ -1038,6 +1038,29 @@ abstract class BaseEditorActivity : resetMemUsageChart() } + /** + * Plots the Gradle daemon, reported by the tooling server once a build has spawned it. + * + * The daemon is the largest of the three watched processes -- larger than the IDE and the + * tooling server together on a Compose project -- and it is the likeliest reason a build is slow + * or is killed on a small device. Until ADFA-5514 it was the one process the chart did not show. + */ + fun watchGradleDaemon(pid: Int) { + memoryUsageWatcher.watchProcess(pid, PROC_GRADLE_DAEMON) + resetMemUsageChart() + } + + /** + * Stops plotting the Gradle daemon, which has exited. + * + * Not on build finish: a daemon outlives the build that spawned it and goes on holding its heap + * while idle, which is the number worth showing on a device that is short of memory. + */ + fun unwatchGradleDaemon() { + memoryUsageWatcher.unwatchProcess(PROC_GRADLE_DAEMON) + resetMemUsageChart() + } + protected fun resetMemUsageChart() { val processes = memoryUsageWatcher.getMemoryUsages() val datasets = diff --git a/app/src/main/java/com/itsaky/androidide/handlers/EditorBuildEventListener.kt b/app/src/main/java/com/itsaky/androidide/handlers/EditorBuildEventListener.kt index ba7a9975b1..eea032ed13 100644 --- a/app/src/main/java/com/itsaky/androidide/handlers/EditorBuildEventListener.kt +++ b/app/src/main/java/com/itsaky/androidide/handlers/EditorBuildEventListener.kt @@ -77,6 +77,16 @@ class EditorBuildEventListener : GradleBuildService.EventListener { this.enabled = false } + override fun onGradleDaemonStarted(pid: Int) { + checkActivity("onGradleDaemonStarted") ?: return + activity.watchGradleDaemon(pid) + } + + override fun onGradleDaemonExited(pid: Int) { + checkActivity("onGradleDaemonExited") ?: return + activity.unwatchGradleDaemon() + } + override fun prepareBuild(buildInfo: BuildInfo) { checkActivity("prepareBuild") ?: return diff --git a/app/src/main/java/com/itsaky/androidide/services/builder/GradleBuildService.kt b/app/src/main/java/com/itsaky/androidide/services/builder/GradleBuildService.kt index 1182029806..2098e780e6 100644 --- a/app/src/main/java/com/itsaky/androidide/services/builder/GradleBuildService.kt +++ b/app/src/main/java/com/itsaky/androidide/services/builder/GradleBuildService.kt @@ -420,6 +420,16 @@ class GradleBuildService : ) } + override fun onGradleDaemonStarted(pid: Int) { + log.info("Gradle daemon started: pid {}", pid) + eventListener?.onGradleDaemonStarted(pid) + } + + override fun onGradleDaemonExited(pid: Int) { + log.info("Gradle daemon exited: pid {}", pid) + eventListener?.onGradleDaemonExited(pid) + } + override fun onBuildSuccessful(result: BuildResult) { updateNotification(getString(R.string.build_status_sucess), false) @@ -760,6 +770,14 @@ class GradleBuildService : runOnUiThread { listener.onBuildSuccessful(tasks) } } + override fun onGradleDaemonStarted(pid: Int) { + runOnUiThread { listener.onGradleDaemonStarted(pid) } + } + + override fun onGradleDaemonExited(pid: Int) { + runOnUiThread { listener.onGradleDaemonExited(pid) } + } + override fun onProgressEvent(event: ProgressEvent) { runOnUiThread { listener.onProgressEvent(event) } } @@ -823,6 +841,25 @@ class GradleBuildService : */ fun onBuildSuccessful(tasks: List) + /** + * Called when the Gradle daemon has been identified by the tooling server. + * + * Defaulted, because a daemon is only of interest to a listener that plots it and every + * other implementer would otherwise gain two empty methods. + * + * @param pid The process id of the Gradle daemon. + * @see IToolingApiClient.onGradleDaemonStarted + */ + fun onGradleDaemonStarted(pid: Int) = Unit + + /** + * Called when the Gradle daemon has exited. + * + * @param pid The process id of the daemon that exited. + * @see IToolingApiClient.onGradleDaemonExited + */ + fun onGradleDaemonExited(pid: Int) = Unit + /** * Called when a progress event is received from the Tooling API server. * 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 0e531ae964..615de3d1d4 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 @@ -36,6 +37,7 @@ import kotlinx.coroutines.launch import kotlinx.coroutines.newSingleThreadContext import kotlinx.coroutines.withContext import org.slf4j.LoggerFactory +import java.io.File import java.util.concurrent.ConcurrentHashMap import java.util.concurrent.atomic.AtomicBoolean @@ -115,7 +117,8 @@ class MemoryUsageWatcher( } } - private fun readUsages() { + @VisibleForTesting + internal fun readUsages() { val activityManager = BaseApplication.baseInstance.getSystemService() if (activityManager == null) { log.error("ActivityManager is null") @@ -134,15 +137,24 @@ class MemoryUsageWatcher( 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 + // A dead process still has an entry here until whoever is watching it says otherwise, and + // Debug.getMemoryInfo leaves memInfo untouched for one -- so sampling it again would + // repeat the last reading forever and draw a flat line for a process that no longer + // exists. The Gradle daemon made this reachable: unlike the IDE and the tooling server it + // comes and goes, and it is the largest of the three (ADFA-5514). Plot a zero instead, + // which is both true and visibly the end of that process. + val usageBytes = + if (!isProcessAlive(pid)) { + 0L + } else { + 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." + // values are in kB, convert to bytes + proc.memInfo.totalPss * 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 @@ -160,6 +172,15 @@ class MemoryUsageWatcher( } } + /** + * Whether [pid] still names a live process. + * + * `/proc` rather than `ProcessHandle`, which Android only gained recently, or a signal probe, + * which needs a permission this does not have. + */ + @VisibleForTesting + internal var isProcessAlive: (Int) -> Boolean = { pid -> File("/proc/$pid").exists() } + /** * Watches the memory usage of the given process. * diff --git a/app/src/test/java/com/itsaky/androidide/utils/MemoryUsageWatcherLivenessTest.kt b/app/src/test/java/com/itsaky/androidide/utils/MemoryUsageWatcherLivenessTest.kt new file mode 100644 index 0000000000..6250691793 --- /dev/null +++ b/app/src/test/java/com/itsaky/androidide/utils/MemoryUsageWatcherLivenessTest.kt @@ -0,0 +1,114 @@ +/* + * This file is part of AndroidIDE. + * + * AndroidIDE is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * AndroidIDE is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with AndroidIDE. If not, see . + */ + +package com.itsaky.androidide.utils + +import com.google.common.truth.Truth.assertThat +import org.junit.After +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import java.io.File + +/** + * What the chart plots for a process that has gone away. + * + * The IDE and the tooling server live as long as the editor does, so this never mattered until the + * Gradle daemon was plotted too (ADFA-5514): it is the one watched process that comes and goes, and + * the biggest, so a stale reading for it is the most misleading of the three. + */ +@RunWith(RobolectricTestRunner::class) +class MemoryUsageWatcherLivenessTest { + private val watchers = mutableListOf() + + @After + fun tearDown() { + watchers.forEach { it.stopWatching() } + watchers.clear() + } + + private fun watcher() = MemoryUsageWatcher().also(watchers::add) + + private fun newestSample( + watcher: MemoryUsageWatcher, + pid: Int, + ): Long { + val history = watcher.getMemoryUsage(pid)!!.usageHistory + return history[history.size - 1] + } + + @Test + fun `a dead process plots zero rather than repeating its last reading`() { + val watcher = watcher() + watcher.watchProcess(DEAD_PID, "Gradle Daemon") + + // What the last successful sample left behind. Debug.getMemoryInfo leaves its output + // untouched for a pid that no longer exists, so without a liveness check every later sample + // reads this same figure back -- a flat line at 800MB for a daemon that has died, which is + // worse than no line at all. + watcher.getMemoryUsage(DEAD_PID)!!.memInfo.dalvikPss = STALE_PSS_KB + watcher.isProcessAlive = { false } + watcher.readUsages() + + assertThat(newestSample(watcher, DEAD_PID)).isEqualTo(0L) + } + + @Test + fun `liveness is decided per process, not for the sample as a whole`() { + val watcher = watcher() + watcher.watchProcess(DEAD_PID, "Gradle Daemon") + watcher.watchProcess(LIVE_PID, "IDE") + val asked = mutableListOf() + + watcher.isProcessAlive = { pid -> + asked += pid + pid == LIVE_PID + } + watcher.readUsages() + + // A dead daemon must not stop the IDE's own line being sampled. + assertThat(asked).containsExactly(DEAD_PID, LIVE_PID) + assertThat(newestSample(watcher, DEAD_PID)).isEqualTo(0L) + } + + @Test + fun `the default check really reads proc`() { + val watcher = watcher() + + // Guards the tests above: they replace isProcessAlive wholesale, so nothing else here would + // notice if the real one stopped answering. + assertThat(watcher.isProcessAlive(LIVE_PID)).isTrue() + assertThat(watcher.isProcessAlive(DEAD_PID)).isFalse() + } + + private companion object { + /** Above any pid the kernel will hand out, so `/proc` cannot have an entry for it. */ + const val DEAD_PID = Int.MAX_VALUE + + /** Stands in for the reading a dead process would otherwise repeat forever. */ + const val STALE_PSS_KB = 800 * 1024 + + /** + * The test JVM itself, which is certainly alive. + * + * Read from `/proc/self`, which is the same source the check itself uses. Not + * `Process.myPid()`: Robolectric answers that with 0, which is not a pid this process has and + * collides with anything else standing in for "no such process". + */ + val LIVE_PID = File("/proc/self").canonicalFile.name.toInt() + } +} diff --git a/subprojects/tooling-api-impl/src/main/java/com/itsaky/androidide/tooling/impl/GradleDaemonWatcher.kt b/subprojects/tooling-api-impl/src/main/java/com/itsaky/androidide/tooling/impl/GradleDaemonWatcher.kt new file mode 100644 index 0000000000..9fd4f457e8 --- /dev/null +++ b/subprojects/tooling-api-impl/src/main/java/com/itsaky/androidide/tooling/impl/GradleDaemonWatcher.kt @@ -0,0 +1,170 @@ +/* + * 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.tooling.impl + +import org.slf4j.LoggerFactory +import java.io.File +import java.util.concurrent.Executors +import java.util.concurrent.ScheduledExecutorService +import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicInteger + +/** + * Finds the Gradle daemon this server drives and reports it to the client. + * + * The daemon is the largest memory consumer of the three processes the IDE plots, and until + * ADFA-5514 it was the one the memory chart could not show: the client has no handle on it. The + * server does -- the daemon is its own child, which [Main.killDescendantProcesses] already relies on + * to shut it down. + * + * The Tooling API spawns the daemon asynchronously, a moment after a build starts and only when + * there is no reusable one already running, so there is no point in the build at which the pid can + * simply be read. This polls for it over a bounded window instead, and stops as soon as it finds + * one. + */ +internal class GradleDaemonWatcher( + private val onStarted: (Int) -> Unit, + private val onExited: (Int) -> Unit, + private val descendants: () -> List = { + ProcessHandle.current().descendants().toList() + }, + private val scheduler: ScheduledExecutorService = defaultScheduler(), +) { + /** The daemon currently reported to the client, or [NO_PID] when there is none. */ + private val watched = AtomicInteger(NO_PID) + + /** + * Looks for a daemon, if one is not already being reported. + * + * Called when a build starts. Cheap and idempotent while a daemon is known: a daemon survives + * the build that spawned it and is reused by the next one, so the usual case is a single + * comparison and no scan at all. + */ + fun onBuildStarted() { + if (watched.get() != NO_PID) { + return + } + + var attempts = 0 + lateinit var poll: Runnable + poll = + Runnable { + if (watched.get() != NO_PID) { + return@Runnable + } + + val found = runCatching { findDaemon(descendants()) }.getOrNull() + if (found != null) { + report(found) + return@Runnable + } + + if (++attempts < MAX_POLL_ATTEMPTS) { + scheduler.schedule(poll, POLL_INTERVAL_MS, TimeUnit.MILLISECONDS) + } else { + log.info("Gave up looking for a Gradle daemon after {} attempts", attempts) + } + } + scheduler.schedule(poll, POLL_INTERVAL_MS, TimeUnit.MILLISECONDS) + } + + private fun report(handle: ProcessHandle) { + val pid = handle.pid().toInt() + if (!watched.compareAndSet(NO_PID, pid)) { + return + } + + log.info("Gradle daemon identified: pid {}", pid) + runCatching { onStarted(pid) } + .onFailure { err -> log.warn("Failed to report Gradle daemon {}", pid, err) } + + // The daemon is killed on server shutdown and can also die on its own -- an idle timeout, or + // the platform reclaiming it under memory pressure, which on a small device is precisely the + // case worth plotting. Either way the client has to be told, or it goes on charting a pid + // that no longer exists. + handle.onExit().thenRun { + if (watched.compareAndSet(pid, NO_PID)) { + log.info("Gradle daemon {} exited", pid) + runCatching { onExited(pid) } + .onFailure { err -> log.warn("Failed to report exit of Gradle daemon {}", pid, err) } + } + } + } + + fun shutdown() { + scheduler.shutdownNow() + } + + companion object { + private val log = LoggerFactory.getLogger(GradleDaemonWatcher::class.java) + + const val NO_PID = -1 + + /** + * The daemon's main class, which is what tells it apart from any other JVM the build starts. + * + * Not "the only child": Gradle can run the Kotlin compiler in a daemon of its own, and that + * one is a sibling of this process rather than the one holding the build's heap. + */ + const val DAEMON_MAIN_CLASS = "org.gradle.launcher.daemon.bootstrap.GradleDaemon" + + private const val POLL_INTERVAL_MS = 500L + + /** Bounded at roughly a minute, which is far longer than a daemon takes to come up. */ + const val MAX_POLL_ATTEMPTS = 120 + + private fun defaultScheduler(): ScheduledExecutorService = + Executors.newSingleThreadScheduledExecutor { runnable -> + Thread(runnable, "GradleDaemonWatcher").apply { isDaemon = true } + } + + /** + * The Gradle daemon among [candidates], or `null` if none of them is one. + */ + fun findDaemon(candidates: List): ProcessHandle? = + candidates.firstOrNull { handle -> + handle.isAlive && isDaemonCommandLine(commandLineOf(handle)) + } + + fun isDaemonCommandLine(commandLine: String?): Boolean = commandLine?.contains(DAEMON_MAIN_CLASS) == true + + /** + * The command line of [handle], as a single string. + * + * `ProcessHandle.info()` is the portable route, but it reads the command line through the + * platform's own process listing and comes back empty often enough -- for processes it + * considers foreign, and on restricted systems -- that it cannot be the only one. `/proc` is + * authoritative here, and readable: the daemon is a child of this process and runs under the + * same uid. + */ + private fun commandLineOf(handle: ProcessHandle): String? { + val info = runCatching { handle.info().commandLine().orElse(null) }.getOrNull() + if (!info.isNullOrBlank()) { + return info + } + return runCatching { + // Arguments are NUL-separated in /proc, so they have to be joined back up before + // anything can be matched across them. + File("/proc/${handle.pid()}/cmdline") + .readBytes() + .toString(Charsets.UTF_8) + .replace('\u0000', ' ') + }.getOrNull() + } + } +} diff --git a/subprojects/tooling-api-impl/src/main/java/com/itsaky/androidide/tooling/impl/ToolingApiServerImpl.kt b/subprojects/tooling-api-impl/src/main/java/com/itsaky/androidide/tooling/impl/ToolingApiServerImpl.kt index 9a05bacaac..dd5e0b8c26 100644 --- a/subprojects/tooling-api-impl/src/main/java/com/itsaky/androidide/tooling/impl/ToolingApiServerImpl.kt +++ b/subprojects/tooling-api-impl/src/main/java/com/itsaky/androidide/tooling/impl/ToolingApiServerImpl.kt @@ -357,6 +357,17 @@ internal class ToolingApiServerImpl : IToolingApiServer { } } + /** + * Finds the Gradle daemon and reports it to the client, so the memory chart can plot the process + * that actually holds the build's heap (ADFA-5514). + */ + private val daemonWatcher by lazy { + GradleDaemonWatcher( + onStarted = { pid -> client?.onGradleDaemonStarted(pid) }, + onExited = { pid -> client?.onGradleDaemonExited(pid) }, + ) + } + private fun notifyBuildFailure(result: BuildResult) { client?.onBuildFailed(result) } @@ -457,6 +468,7 @@ internal class ToolingApiServerImpl : IToolingApiServer { } isBuildInProgress = true + daemonWatcher.onBuildStarted() try { action() } finally { diff --git a/subprojects/tooling-api-impl/src/test/java/com/itsaky/androidide/tooling/impl/GradleDaemonWatcherTest.kt b/subprojects/tooling-api-impl/src/test/java/com/itsaky/androidide/tooling/impl/GradleDaemonWatcherTest.kt new file mode 100644 index 0000000000..d9d87bb421 --- /dev/null +++ b/subprojects/tooling-api-impl/src/test/java/com/itsaky/androidide/tooling/impl/GradleDaemonWatcherTest.kt @@ -0,0 +1,189 @@ +/* + * 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.tooling.impl + +import com.google.common.truth.Truth.assertThat +import io.mockk.every +import io.mockk.mockk +import io.mockk.verify +import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.JUnit4 +import java.util.Optional +import java.util.concurrent.CompletableFuture +import java.util.concurrent.ScheduledExecutorService +import java.util.concurrent.TimeUnit + +/** + * Which of the tooling server's children is the Gradle daemon, and when the client hears about it. + * + * Measured on a Pixel 6 Pro during a build: the IDE at 702 MB, the tooling server at 165 MB and the + * daemon at 779 MB -- the largest of the three, and the one the chart could not show (ADFA-5514). + */ +@RunWith(JUnit4::class) +class GradleDaemonWatcherTest { + private val started = mutableListOf() + private val exited = mutableListOf() + + /** + * A handle whose command line is what a real daemon's looks like on device. + * + * The full line carries the daemon jar and its heap settings; the main class is the part that + * identifies it. + */ + private fun handle( + pid: Long, + commandLine: String?, + alive: Boolean = true, + exit: CompletableFuture = CompletableFuture(), + ): ProcessHandle { + val info = mockk() + every { info.commandLine() } returns Optional.ofNullable(commandLine) + return mockk().also { + every { it.pid() } returns pid + every { it.isAlive } returns alive + every { it.info() } returns info + every { it.onExit() } returns exit + } + } + + private fun daemon( + pid: Long, + exit: CompletableFuture = CompletableFuture(), + ) = handle(pid, DAEMON_COMMAND_LINE, exit = exit) + + /** Runs whatever is scheduled straight away, so a test does not have to wait out the poll. */ + private fun immediateScheduler(): ScheduledExecutorService = + mockk(relaxed = true).also { scheduler -> + every { scheduler.schedule(any(), any(), any()) } answers { + firstArg().run() + mockk(relaxed = true) + } + } + + private fun watcher( + vararg children: ProcessHandle, + scheduler: ScheduledExecutorService = immediateScheduler(), + ) = GradleDaemonWatcher( + onStarted = started::add, + onExited = exited::add, + descendants = { children.toList() }, + scheduler = scheduler, + ) + + @Test + fun `the daemon is picked out of the server's other children`() { + // The Kotlin compiler can run in a daemon of its own, a sibling of the Gradle daemon rather + // than the process holding the build's heap. Taking "the only child" would pick either. + val kotlinDaemon = handle(2L, "/usr/bin/java -cp kotlin-daemon.jar org.jetbrains.kotlin.daemon.KotlinCompileDaemon") + val gradleDaemon = daemon(3L) + + watcher(kotlinDaemon, gradleDaemon).onBuildStarted() + + assertThat(started).containsExactly(3) + } + + @Test + fun `a build with no daemon of its own reports nothing`() { + watcher(handle(2L, "/usr/bin/java -jar something-else.jar")).onBuildStarted() + + assertThat(started).isEmpty() + } + + @Test + fun `a dead child is not reported, however it identifies itself`() { + val corpse = handle(3L, DAEMON_COMMAND_LINE, alive = false) + + watcher(corpse).onBuildStarted() + + assertThat(started).isEmpty() + } + + @Test + fun `the daemon is reported once, not once per build`() { + val watcher = watcher(daemon(3L)) + + watcher.onBuildStarted() + watcher.onBuildStarted() + watcher.onBuildStarted() + + // A daemon outlives the build that spawned it and is reused by the next one. Reporting it + // again would re-watch a pid already being plotted. + assertThat(started).containsExactly(3) + } + + @Test + fun `the client is told when the daemon exits`() { + val exit = CompletableFuture() + val handle = daemon(3L, exit = exit) + + watcher(handle).onBuildStarted() + exit.complete(handle) + + assertThat(exited).containsExactly(3) + } + + @Test + fun `a daemon replacing one that exited is reported in its turn`() { + val firstExit = CompletableFuture() + val first = daemon(3L, exit = firstExit) + var children = listOf(first) + val watcher = + GradleDaemonWatcher( + onStarted = started::add, + onExited = exited::add, + descendants = { children }, + scheduler = immediateScheduler(), + ) + + watcher.onBuildStarted() + firstExit.complete(first) + children = listOf(daemon(4L)) + watcher.onBuildStarted() + + // Gradle starts a fresh daemon when the old one is gone -- after an idle timeout, or after + // the platform reclaimed it, which on a small device is the case worth plotting. + assertThat(started).containsExactly(3, 4).inOrder() + assertThat(exited).containsExactly(3) + } + + @Test + fun `the search gives up rather than polling for the life of the server`() { + val scheduler = immediateScheduler() + + watcher(handle(2L, "/usr/bin/java -jar not-a-daemon.jar"), scheduler = scheduler).onBuildStarted() + + assertThat(started).isEmpty() + // Bounded: one initial schedule plus the retries, and no more. + verify(atMost = MAX_SCHEDULES) { + scheduler.schedule(any(), any(), any()) + } + } + + private companion object { + /** What the daemon's command line looks like on device, trimmed to the identifying part. */ + const val DAEMON_COMMAND_LINE = + "/usr/bin/java -Xmx4620m -XX:MaxMetaspaceSize=384m -cp " + + "/data/data/com.itsaky.androidide/files/home/.cg/gradle-dists/gradle-8.14.3/lib/" + + "gradle-daemon-main-8.14.3.jar " + + GradleDaemonWatcher.DAEMON_MAIN_CLASS + " 8.14.3" + + /** Every poll attempt, plus the initial schedule. */ + const val MAX_SCHEDULES = GradleDaemonWatcher.MAX_POLL_ATTEMPTS + 1 + } +} diff --git a/subprojects/tooling-api/src/main/java/com/itsaky/androidide/tooling/api/ForwardingToolingApiClient.kt b/subprojects/tooling-api/src/main/java/com/itsaky/androidide/tooling/api/ForwardingToolingApiClient.kt index ff5e24c5e1..a6bf243111 100644 --- a/subprojects/tooling-api/src/main/java/com/itsaky/androidide/tooling/api/ForwardingToolingApiClient.kt +++ b/subprojects/tooling-api/src/main/java/com/itsaky/androidide/tooling/api/ForwardingToolingApiClient.kt @@ -53,6 +53,14 @@ class ForwardingToolingApiClient( client?.onBuildFailed(result) } + override fun onGradleDaemonStarted(pid: Int) { + client?.onGradleDaemonStarted(pid) + } + + override fun onGradleDaemonExited(pid: Int) { + client?.onGradleDaemonExited(pid) + } + override fun onProgressEvent(event: ProgressEvent) { client?.onProgressEvent(event) } diff --git a/subprojects/tooling-api/src/main/java/com/itsaky/androidide/tooling/api/IToolingApiClient.kt b/subprojects/tooling-api/src/main/java/com/itsaky/androidide/tooling/api/IToolingApiClient.kt index 24308676ed..7c50893e6d 100644 --- a/subprojects/tooling-api/src/main/java/com/itsaky/androidide/tooling/api/IToolingApiClient.kt +++ b/subprojects/tooling-api/src/main/java/com/itsaky/androidide/tooling/api/IToolingApiClient.kt @@ -80,6 +80,31 @@ interface IToolingApiClient { @JsonNotification fun onBuildFailed(result: BuildResult) + /** + * Called when the Gradle daemon this server drives has been identified. + * + * The daemon is a separate process, spawned by the Tooling API a moment after a build starts, + * and it is the largest memory consumer of the three -- larger than the IDE and the tooling + * server together on a Compose project. Only the server can name it: the daemon is its own + * child, and the client has no handle on it (ADFA-5514). + * + * Reported once per daemon, not once per build. A daemon outlives the build that spawned it and + * goes on holding its heap while idle, which is exactly what a user on a small device needs to + * see. + * + * @param pid The process id of the Gradle daemon. + */ + @JsonNotification + fun onGradleDaemonStarted(pid: Int) + + /** + * Called when the Gradle daemon reported by [onGradleDaemonStarted] has exited. + * + * @param pid The process id of the daemon that exited. + */ + @JsonNotification + fun onGradleDaemonExited(pid: Int) + /** * Called when a [ProgressEvent] is received from Gradle build. * diff --git a/testing/tooling/src/main/java/com/itsaky/androidide/testing/tooling/ToolingApiTestLauncher.kt b/testing/tooling/src/main/java/com/itsaky/androidide/testing/tooling/ToolingApiTestLauncher.kt index 916987e40a..b4cfc26b78 100644 --- a/testing/tooling/src/main/java/com/itsaky/androidide/testing/tooling/ToolingApiTestLauncher.kt +++ b/testing/tooling/src/main/java/com/itsaky/androidide/testing/tooling/ToolingApiTestLauncher.kt @@ -352,6 +352,14 @@ object ToolingApiTestLauncher { ) } + override fun onGradleDaemonStarted(pid: Int) { + log.info("Gradle daemon started: {}", pid) + } + + override fun onGradleDaemonExited(pid: Int) { + log.info("Gradle daemon exited: {}", pid) + } + override fun onBuildSuccessful(result: BuildResult) { onBuildResult(result) } From 3cf9483418123745609a04ea196675f92c68af47 Mon Sep 17 00:00:00 2001 From: David Schachter Date: Mon, 7 Sep 2026 05:00:47 -0700 Subject: [PATCH 3/6] ADFA-5514: fix three defects found reviewing the daemon plot Sampler NPE. readUsages looked the process up twice and asserted on the second read. unwatchProcess runs on the main thread from a build event, so the entry can go between them -- and the daemon is the process that gets unwatched. Use the value already in hand. Exit and start could cross. ProcessHandle.onExit fires on a process-reaper thread while starts are reported from the poll, and freeing the slot is what lets the next poll find a replacement daemon. So a start for the new daemon could reach the client ahead of the exit for the old one, and the client, which unwatched by name, would drop the line it had just been told to draw. Both reports now come off the watcher's own thread, and the client unwatches by pid, so a late exit for a dead pid is a no-op rather than wrong. That means onBuildStarted can no longer skip the scan when a daemon is already known: the answer differs precisely in the window where an exit is still queued, and skipping there would leave a fresh daemon unplotted until the build after next. The poll makes the test instead, on the thread that owns the state. Its initial schedule is now guarded, since it is submitted from the build's thread and the scheduler rejects work after shutdown. Lost lines on a configuration change. A rotation replaces the activity and its MemoryUsageWatcher but not the service or the processes it drives, and both pids arrive on one-shot callbacks the replacement has already missed -- the tooling server's on the start it did not request, the daemon's on the build that spawned it. The chart came back plotting the IDE alone. The service remembers both pids and the activity re-adopts them on connect. The tooling server half is pre-existing, not new to this ticket; it is the same defect and one line to fix here. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_017CCQUU7tBzZL61EmQhJP8j --- .../activities/editor/BaseEditorActivity.kt | 11 +++-- .../editor/ProjectHandlerActivity.kt | 20 +++++++++ .../handlers/EditorBuildEventListener.kt | 2 +- .../services/builder/GradleBuildService.kt | 25 +++++++++++ .../androidide/utils/MemoryUsageWatcher.kt | 5 ++- .../utils/MemoryUsageWatcherLivenessTest.kt | 39 +++++++++++++++++ .../tooling/impl/GradleDaemonWatcher.kt | 42 +++++++++++++------ .../tooling/impl/GradleDaemonWatcherTest.kt | 31 +++++++++++++- 8 files changed, 156 insertions(+), 19 deletions(-) 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 b19adefcd0..4789934682 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 @@ -1051,13 +1051,18 @@ abstract class BaseEditorActivity : } /** - * Stops plotting the Gradle daemon, which has exited. + * Stops plotting the Gradle daemon [pid], which has exited. * * Not on build finish: a daemon outlives the build that spawned it and goes on holding its heap * while idle, which is the number worth showing on a device that is short of memory. + * + * By pid rather than by name, so a late exit cannot take out its successor's line. Removing "the + * Gradle daemon" would: a daemon that dies as the next build starts one is two reports racing + * over one row, and [watchProcess]'s `unique` has already dropped the old pid by then, so this + * is a no-op in exactly the case where the name would have been wrong. */ - fun unwatchGradleDaemon() { - memoryUsageWatcher.unwatchProcess(PROC_GRADLE_DAEMON) + fun unwatchGradleDaemon(pid: Int) { + memoryUsageWatcher.unwatchProcess(pid) resetMemUsageChart() } diff --git a/app/src/main/java/com/itsaky/androidide/activities/editor/ProjectHandlerActivity.kt b/app/src/main/java/com/itsaky/androidide/activities/editor/ProjectHandlerActivity.kt index 26ed2966e3..588118eeb5 100644 --- a/app/src/main/java/com/itsaky/androidide/activities/editor/ProjectHandlerActivity.kt +++ b/app/src/main/java/com/itsaky/androidide/activities/editor/ProjectHandlerActivity.kt @@ -690,6 +690,25 @@ abstract class ProjectHandlerActivity : BaseEditorActivity() { } } + /** + * Re-adds the processes the service already knows about to this activity's memory watcher. + * + * A configuration change replaces the activity and its [MemoryUsageWatcher] but not the service + * or the processes it is driving, and both pids are reported on one-shot callbacks that a + * replacement listener has already missed -- the tooling server's on the start it did not + * request, the daemon's on the build that spawned it. Without this the chart came back after a + * rotation plotting the IDE alone, which is the smallest of the three. + */ + private fun readoptWatchedProcesses(service: GradleBuildService) { + service.toolingServerPid?.let { pid -> + memoryUsageWatcher.watchProcess(pid, PROC_GRADLE_TOOLING) + } + service.gradleDaemonPid?.let { pid -> + memoryUsageWatcher.watchProcess(pid, PROC_GRADLE_DAEMON) + } + resetMemUsageChart() + } + protected fun onGradleBuildServiceConnected(service: GradleBuildService) { log.info("Connected to Gradle build service") @@ -697,6 +716,7 @@ abstract class ProjectHandlerActivity : BaseEditorActivity() { editorViewModel.isBoundToBuildSerice = true Lookup.getDefault().update(BuildService.KEY_BUILD_SERVICE, service) service.setEventListener(mBuildEventListener) + readoptWatchedProcesses(service) if (service.isToolingServerStarted()) { if (service.isBuildInProgress) { diff --git a/app/src/main/java/com/itsaky/androidide/handlers/EditorBuildEventListener.kt b/app/src/main/java/com/itsaky/androidide/handlers/EditorBuildEventListener.kt index eea032ed13..207c5de5e9 100644 --- a/app/src/main/java/com/itsaky/androidide/handlers/EditorBuildEventListener.kt +++ b/app/src/main/java/com/itsaky/androidide/handlers/EditorBuildEventListener.kt @@ -84,7 +84,7 @@ class EditorBuildEventListener : GradleBuildService.EventListener { override fun onGradleDaemonExited(pid: Int) { checkActivity("onGradleDaemonExited") ?: return - activity.unwatchGradleDaemon() + activity.unwatchGradleDaemon(pid) } override fun prepareBuild(buildInfo: BuildInfo) { diff --git a/app/src/main/java/com/itsaky/androidide/services/builder/GradleBuildService.kt b/app/src/main/java/com/itsaky/androidide/services/builder/GradleBuildService.kt index 2098e780e6..a820792dcc 100644 --- a/app/src/main/java/com/itsaky/androidide/services/builder/GradleBuildService.kt +++ b/app/src/main/java/com/itsaky/androidide/services/builder/GradleBuildService.kt @@ -119,6 +119,27 @@ class GradleBuildService : */ private var toolingApiClient: ForwardingToolingApiClient? = null private var toolingServerRunner: ToolingServerRunner? = null + + /** + * The Gradle daemon's pid, or `null` when no daemon is known to be running. + * + * Remembered here and not merely forwarded, because the listener is an activity. A daemon is + * reported once, when a build spawns it, and then outlives that build; an activity recreated + * after that -- a rotation, a font-scale change -- gets a listener that hears about new daemons + * only, so its memory chart silently loses the largest of the three processes. It reads this + * instead. See [onGradleDaemonStarted]. + */ + var gradleDaemonPid: Int? = null + private set + + /** + * The tooling server's pid, or `null` while no started server has one. + * + * Same reason as [gradleDaemonPid]: [startToolingServer] reports the pid to whoever asked for + * the start, so an activity that finds the server already up never hears it. + */ + val toolingServerPid: Int? + get() = toolingServerRunner?.takeIf { it.isStarted }?.pid private var outputReaderJob: Job? = null private var notificationManager: NotificationManager? = null private var server: IToolingApiServer? = null @@ -422,11 +443,15 @@ class GradleBuildService : override fun onGradleDaemonStarted(pid: Int) { log.info("Gradle daemon started: pid {}", pid) + gradleDaemonPid = pid eventListener?.onGradleDaemonStarted(pid) } override fun onGradleDaemonExited(pid: Int) { log.info("Gradle daemon exited: pid {}", pid) + if (gradleDaemonPid == pid) { + gradleDaemonPid = null + } eventListener?.onGradleDaemonExited(pid) } 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 615de3d1d4..bc067b051c 100644 --- a/app/src/main/java/com/itsaky/androidide/utils/MemoryUsageWatcher.kt +++ b/app/src/main/java/com/itsaky/androidide/utils/MemoryUsageWatcher.kt @@ -155,7 +155,10 @@ class MemoryUsageWatcher( // values are in kB, convert to bytes proc.memInfo.totalPss * 1024L } - memoryUsage[pid]!!.apply { + // [proc], not a second lookup: unwatchProcess runs on the main thread and can drop the + // entry between the two, and the Gradle daemon is unwatched from a build event + // (ADFA-5514), so the window is real rather than theoretical. + proc.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 diff --git a/app/src/test/java/com/itsaky/androidide/utils/MemoryUsageWatcherLivenessTest.kt b/app/src/test/java/com/itsaky/androidide/utils/MemoryUsageWatcherLivenessTest.kt index 6250691793..114d0364de 100644 --- a/app/src/test/java/com/itsaky/androidide/utils/MemoryUsageWatcherLivenessTest.kt +++ b/app/src/test/java/com/itsaky/androidide/utils/MemoryUsageWatcherLivenessTest.kt @@ -85,6 +85,45 @@ class MemoryUsageWatcherLivenessTest { assertThat(newestSample(watcher, DEAD_PID)).isEqualTo(0L) } + @Test + fun `a process unwatched while it is being sampled does not take the sampler down with it`() { + val watcher = watcher() + watcher.watchProcess(DEAD_PID, "Gradle Daemon") + watcher.watchProcess(LIVE_PID, "IDE") + + // The daemon is unwatched from a build event, on the main thread, while readUsages runs on + // the sampling thread. Reading the map twice per process left a window between the two in + // which the entry could be dropped, and the second read asserted it was there. + watcher.isProcessAlive = { pid -> + if (pid == DEAD_PID) { + watcher.unwatchProcess(DEAD_PID) + } + true + } + + watcher.readUsages() + + assertThat(watcher.getMemoryUsage(DEAD_PID)).isNull() + assertThat(watcher.getMemoryUsage(LIVE_PID)).isNotNull() + } + + @Test + fun `unwatching a daemon by pid leaves the one that replaced it alone`() { + val watcher = watcher() + watcher.watchProcess(DEAD_PID, "Gradle Daemon") + + // A new build starts a new daemon. watchProcess is unique by name, so the old pid is gone + // from the map before its exit is even reported. + watcher.watchProcess(LIVE_PID, "Gradle Daemon") + + // The exit of the old one arrives afterwards, which is the order the tooling server's + // reaper thread and its poll can produce. By pid this is a no-op; by name it would blank + // the line for the daemon that is actually running. + watcher.unwatchProcess(DEAD_PID) + + assertThat(watcher.getMemoryUsage(LIVE_PID)).isNotNull() + } + @Test fun `the default check really reads proc`() { val watcher = watcher() diff --git a/subprojects/tooling-api-impl/src/main/java/com/itsaky/androidide/tooling/impl/GradleDaemonWatcher.kt b/subprojects/tooling-api-impl/src/main/java/com/itsaky/androidide/tooling/impl/GradleDaemonWatcher.kt index 9fd4f457e8..d3bbe9bebd 100644 --- a/subprojects/tooling-api-impl/src/main/java/com/itsaky/androidide/tooling/impl/GradleDaemonWatcher.kt +++ b/subprojects/tooling-api-impl/src/main/java/com/itsaky/androidide/tooling/impl/GradleDaemonWatcher.kt @@ -52,14 +52,16 @@ internal class GradleDaemonWatcher( * Looks for a daemon, if one is not already being reported. * * Called when a build starts. Cheap and idempotent while a daemon is known: a daemon survives - * the build that spawned it and is reused by the next one, so the usual case is a single - * comparison and no scan at all. + * the build that spawned it and is reused by the next one, so the usual case is one scheduled + * task that reads an int and returns. + * + * The "is one known already" test is deliberately left to the poll rather than made here. Every + * change to [watched] happens on the scheduler, so asking there is asking after the exit of a + * daemon that has just died has been dealt with -- and a build starting in that window is + * exactly the case where the answer differs and a fresh daemon would otherwise go unplotted + * until the build after next. */ fun onBuildStarted() { - if (watched.get() != NO_PID) { - return - } - var attempts = 0 lateinit var poll: Runnable poll = @@ -80,7 +82,10 @@ internal class GradleDaemonWatcher( log.info("Gave up looking for a Gradle daemon after {} attempts", attempts) } } - scheduler.schedule(poll, POLL_INTERVAL_MS, TimeUnit.MILLISECONDS) + // Guarded: this one is submitted from the build's thread, and the scheduler rejects work + // once [shutdown] has run. A build outliving the watcher must not fail over the chart. + runCatching { scheduler.schedule(poll, POLL_INTERVAL_MS, TimeUnit.MILLISECONDS) } + .onFailure { err -> log.warn("Failed to schedule the Gradle daemon search", err) } } private fun report(handle: ProcessHandle) { @@ -97,15 +102,28 @@ internal class GradleDaemonWatcher( // the platform reclaiming it under memory pressure, which on a small device is precisely the // case worth plotting. Either way the client has to be told, or it goes on charting a pid // that no longer exists. + // + // Hop onto the scheduler to say so. onExit runs on a process-reaper thread while starts are + // reported from the poll, so the two could cross: freeing the slot is what lets the next + // poll find a new daemon, and a start for the new one could reach the client before the + // exit for the old one. The client would then be told to stop watching a daemon it had just + // been told to start. Both reports come off one thread now, in order. handle.onExit().thenRun { - if (watched.compareAndSet(pid, NO_PID)) { - log.info("Gradle daemon {} exited", pid) - runCatching { onExited(pid) } - .onFailure { err -> log.warn("Failed to report exit of Gradle daemon {}", pid, err) } - } + runCatching { scheduler.execute { reportExit(pid) } } + .onFailure { err -> log.warn("Failed to queue exit of Gradle daemon {}", pid, err) } } } + private fun reportExit(pid: Int) { + if (!watched.compareAndSet(pid, NO_PID)) { + return + } + + log.info("Gradle daemon {} exited", pid) + runCatching { onExited(pid) } + .onFailure { err -> log.warn("Failed to report exit of Gradle daemon {}", pid, err) } + } + fun shutdown() { scheduler.shutdownNow() } diff --git a/subprojects/tooling-api-impl/src/test/java/com/itsaky/androidide/tooling/impl/GradleDaemonWatcherTest.kt b/subprojects/tooling-api-impl/src/test/java/com/itsaky/androidide/tooling/impl/GradleDaemonWatcherTest.kt index d9d87bb421..2b6ac12aa5 100644 --- a/subprojects/tooling-api-impl/src/test/java/com/itsaky/androidide/tooling/impl/GradleDaemonWatcherTest.kt +++ b/subprojects/tooling-api-impl/src/test/java/com/itsaky/androidide/tooling/impl/GradleDaemonWatcherTest.kt @@ -67,13 +67,19 @@ class GradleDaemonWatcherTest { exit: CompletableFuture = CompletableFuture(), ) = handle(pid, DAEMON_COMMAND_LINE, exit = exit) - /** Runs whatever is scheduled straight away, so a test does not have to wait out the poll. */ - private fun immediateScheduler(): ScheduledExecutorService = + /** + * Runs whatever is scheduled straight away, so a test does not have to wait out the poll. + * + * [execute] is stubbed separately because the exit path uses it rather than [schedule], and a + * test that wants to see what is queued there needs to hold it back. + */ + private fun immediateScheduler(execute: (Runnable) -> Unit = Runnable::run): ScheduledExecutorService = mockk(relaxed = true).also { scheduler -> every { scheduler.schedule(any(), any(), any()) } answers { firstArg().run() mockk(relaxed = true) } + every { scheduler.execute(any()) } answers { execute(firstArg()) } } private fun watcher( @@ -162,6 +168,27 @@ class GradleDaemonWatcherTest { assertThat(exited).containsExactly(3) } + @Test + fun `an exit is handed to the watcher's own thread rather than reported from the reaper's`() { + val deferred = ArrayDeque() + val exit = CompletableFuture() + val handle = daemon(3L, exit = exit) + + watcher(handle, scheduler = immediateScheduler(execute = deferred::add)).onBuildStarted() + exit.complete(handle) + + // ProcessHandle.onExit fires on a process-reaper thread, while a start is reported from the + // poll. Reporting an exit from there lets the two cross: freeing the slot is what lets the + // next poll find a replacement daemon, so a start for the new one could reach the client + // ahead of the exit for the old one -- and the client would drop the line it had just been + // told to draw. Everything the client hears comes off the one thread instead. + assertThat(exited).isEmpty() + + deferred.forEach(Runnable::run) + + assertThat(exited).containsExactly(3) + } + @Test fun `the search gives up rather than polling for the life of the server`() { val scheduler = immediateScheduler() From 7149ff7e0d0991e2dc88b39cd7d28edffcb00b07 Mon Sep 17 00:00:00 2001 From: David Schachter Date: Mon, 7 Sep 2026 05:09:44 -0700 Subject: [PATCH 4/6] ADFA-5514: log which processes a recreated editor re-adopts The re-adoption has no unit test -- its call site is an activity's service callback -- so this is what makes it checkable on device, and it says which pids the chart is about to plot after the editor comes back. Silent when there is nothing to re-adopt, which is every cold start. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_017CCQUU7tBzZL61EmQhJP8j --- .../activities/editor/ProjectHandlerActivity.kt | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/app/src/main/java/com/itsaky/androidide/activities/editor/ProjectHandlerActivity.kt b/app/src/main/java/com/itsaky/androidide/activities/editor/ProjectHandlerActivity.kt index 588118eeb5..28c014f88d 100644 --- a/app/src/main/java/com/itsaky/androidide/activities/editor/ProjectHandlerActivity.kt +++ b/app/src/main/java/com/itsaky/androidide/activities/editor/ProjectHandlerActivity.kt @@ -700,12 +700,15 @@ abstract class ProjectHandlerActivity : BaseEditorActivity() { * rotation plotting the IDE alone, which is the smallest of the three. */ private fun readoptWatchedProcesses(service: GradleBuildService) { - service.toolingServerPid?.let { pid -> - memoryUsageWatcher.watchProcess(pid, PROC_GRADLE_TOOLING) - } - service.gradleDaemonPid?.let { pid -> - memoryUsageWatcher.watchProcess(pid, PROC_GRADLE_DAEMON) + val tooling = service.toolingServerPid + val daemon = service.gradleDaemonPid + if (tooling == null && daemon == null) { + return } + + logger.info("Re-adopting watched processes: tooling server {}, Gradle daemon {}", tooling, daemon) + tooling?.let { memoryUsageWatcher.watchProcess(it, PROC_GRADLE_TOOLING) } + daemon?.let { memoryUsageWatcher.watchProcess(it, PROC_GRADLE_DAEMON) } resetMemUsageChart() } From 6df435622ef8194b9b093530992b2f5f52d0bece Mon Sep 17 00:00:00 2001 From: David Schachter Date: Mon, 7 Sep 2026 05:45:41 -0700 Subject: [PATCH 5/6] ADFA-5514: give the dead-pid test a control, and stop reading proc in a clinit The dead-pid test asserted a zero, which is also what a watcher that read nothing at all would hold -- so it did not distinguish the guard working from the setup never taking effect. It now reads the stale figure back first, with the same process called alive, so the zero that follows is a decision. (The stale read does happen: the control passes at 800MB.) LIVE_PID parsed /proc/self in a companion initialiser, so a platform without /proc took the whole class down with an ExceptionInInitializerError -- four unrelated tests failing for a reason none of them is about. Only one test needs a pid /proc really has; it reads one itself and skips if there is none. The others just need a second pid, since they replace the liveness check anyway. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_017CCQUU7tBzZL61EmQhJP8j --- .../utils/MemoryUsageWatcherLivenessTest.kt | 47 +++++++++++++------ 1 file changed, 32 insertions(+), 15 deletions(-) diff --git a/app/src/test/java/com/itsaky/androidide/utils/MemoryUsageWatcherLivenessTest.kt b/app/src/test/java/com/itsaky/androidide/utils/MemoryUsageWatcherLivenessTest.kt index 114d0364de..4708a56fb5 100644 --- a/app/src/test/java/com/itsaky/androidide/utils/MemoryUsageWatcherLivenessTest.kt +++ b/app/src/test/java/com/itsaky/androidide/utils/MemoryUsageWatcherLivenessTest.kt @@ -19,6 +19,7 @@ package com.itsaky.androidide.utils import com.google.common.truth.Truth.assertThat import org.junit.After +import org.junit.Assume.assumeTrue import org.junit.Test import org.junit.runner.RunWith import org.robolectric.RobolectricTestRunner @@ -61,6 +62,14 @@ class MemoryUsageWatcherLivenessTest { // reads this same figure back -- a flat line at 800MB for a daemon that has died, which is // worse than no line at all. watcher.getMemoryUsage(DEAD_PID)!!.memInfo.dalvikPss = STALE_PSS_KB + + // The control, and it is not optional: a zero on its own proves nothing, because a zero is + // also what a watcher that read nothing at all would hold. Called alive, the same setup + // reads the stale figure back -- so the zero below is a decision rather than a default. + watcher.isProcessAlive = { true } + watcher.readUsages() + assertThat(newestSample(watcher, DEAD_PID)).isEqualTo(STALE_PSS_KB * 1024L) + watcher.isProcessAlive = { false } watcher.readUsages() @@ -71,17 +80,17 @@ class MemoryUsageWatcherLivenessTest { fun `liveness is decided per process, not for the sample as a whole`() { val watcher = watcher() watcher.watchProcess(DEAD_PID, "Gradle Daemon") - watcher.watchProcess(LIVE_PID, "IDE") + watcher.watchProcess(OTHER_PID, "IDE") val asked = mutableListOf() watcher.isProcessAlive = { pid -> asked += pid - pid == LIVE_PID + pid == OTHER_PID } watcher.readUsages() // A dead daemon must not stop the IDE's own line being sampled. - assertThat(asked).containsExactly(DEAD_PID, LIVE_PID) + assertThat(asked).containsExactly(DEAD_PID, OTHER_PID) assertThat(newestSample(watcher, DEAD_PID)).isEqualTo(0L) } @@ -89,7 +98,7 @@ class MemoryUsageWatcherLivenessTest { fun `a process unwatched while it is being sampled does not take the sampler down with it`() { val watcher = watcher() watcher.watchProcess(DEAD_PID, "Gradle Daemon") - watcher.watchProcess(LIVE_PID, "IDE") + watcher.watchProcess(OTHER_PID, "IDE") // The daemon is unwatched from a build event, on the main thread, while readUsages runs on // the sampling thread. Reading the map twice per process left a window between the two in @@ -104,7 +113,7 @@ class MemoryUsageWatcherLivenessTest { watcher.readUsages() assertThat(watcher.getMemoryUsage(DEAD_PID)).isNull() - assertThat(watcher.getMemoryUsage(LIVE_PID)).isNotNull() + assertThat(watcher.getMemoryUsage(OTHER_PID)).isNotNull() } @Test @@ -114,23 +123,30 @@ class MemoryUsageWatcherLivenessTest { // A new build starts a new daemon. watchProcess is unique by name, so the old pid is gone // from the map before its exit is even reported. - watcher.watchProcess(LIVE_PID, "Gradle Daemon") + watcher.watchProcess(OTHER_PID, "Gradle Daemon") // The exit of the old one arrives afterwards, which is the order the tooling server's // reaper thread and its poll can produce. By pid this is a no-op; by name it would blank // the line for the daemon that is actually running. watcher.unwatchProcess(DEAD_PID) - assertThat(watcher.getMemoryUsage(LIVE_PID)).isNotNull() + assertThat(watcher.getMemoryUsage(OTHER_PID)).isNotNull() } @Test fun `the default check really reads proc`() { - val watcher = watcher() - // Guards the tests above: they replace isProcessAlive wholesale, so nothing else here would // notice if the real one stopped answering. - assertThat(watcher.isProcessAlive(LIVE_PID)).isTrue() + // + // This is the only test that needs a pid `/proc` really has, so it reads one here rather + // than in a companion initialiser. There it took the whole class down with an + // ExceptionInInitializerError on any platform without `/proc` -- four unrelated tests + // failing for a reason none of them is about -- instead of skipping the one that cares. + val selfPid = File("/proc/self").canonicalFile.name.toLongOrNull() + assumeTrue("no /proc on this platform", selfPid != null) + + val watcher = watcher() + assertThat(watcher.isProcessAlive(selfPid!!.toInt())).isTrue() assertThat(watcher.isProcessAlive(DEAD_PID)).isFalse() } @@ -142,12 +158,13 @@ class MemoryUsageWatcherLivenessTest { const val STALE_PSS_KB = 800 * 1024 /** - * The test JVM itself, which is certainly alive. + * A second watched process. * - * Read from `/proc/self`, which is the same source the check itself uses. Not - * `Process.myPid()`: Robolectric answers that with 0, which is not a pid this process has and - * collides with anything else standing in for "no such process". + * Any number will do: every test that uses it replaces [MemoryUsageWatcher.isProcessAlive], + * so nothing asks `/proc` about it. Not `Process.myPid()`, which Robolectric answers with 0 + * -- not a pid this process has, and a collision with anything standing in for "no such + * process". */ - val LIVE_PID = File("/proc/self").canonicalFile.name.toInt() + const val OTHER_PID = 4243 } } From dc6aff0bce40cfb1beac50960f635d12367121cc Mon Sep 17 00:00:00 2001 From: David Schachter Date: Mon, 7 Sep 2026 10:18:53 -0700 Subject: [PATCH 6/6] ADFA-5514: publish both re-adopted pids across threads Review found gradleDaemonPid written on the tooling API's RPC reader thread and read on the main thread while the activity binds. Without a memory barrier a recreated editor can read a stale null and leave the daemon off the chart -- the exact failure the field was added to prevent. ToolingServerRunner.pid has the same shape and was not flagged: startAsync writes it from a coroutine on runnerScope, and the same re-adoption reads it on the main thread. Both are volatile now, since fixing one and leaving the other would fix half of one feature. No test: a missing happens-before edge is not reproducible on demand, and a test that passes either way would pin nothing. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_017CCQUU7tBzZL61EmQhJP8j --- .../androidide/services/builder/GradleBuildService.kt | 6 ++++++ .../androidide/services/builder/ToolingServerRunner.kt | 8 ++++++++ 2 files changed, 14 insertions(+) diff --git a/app/src/main/java/com/itsaky/androidide/services/builder/GradleBuildService.kt b/app/src/main/java/com/itsaky/androidide/services/builder/GradleBuildService.kt index a820792dcc..b0c29b2ea3 100644 --- a/app/src/main/java/com/itsaky/androidide/services/builder/GradleBuildService.kt +++ b/app/src/main/java/com/itsaky/androidide/services/builder/GradleBuildService.kt @@ -128,7 +128,13 @@ class GradleBuildService : * after that -- a rotation, a font-scale change -- gets a listener that hears about new daemons * only, so its memory chart silently loses the largest of the three processes. It reads this * instead. See [onGradleDaemonStarted]. + * + * Volatile because the two ends are on different threads: the tooling API's client callbacks + * write it on the RPC reader thread, and [ProjectHandlerActivity] reads it on the main thread + * while binding. Without it a recreated activity can read a stale `null` and quietly leave the + * daemon off the chart -- the very failure this field exists to prevent. */ + @Volatile var gradleDaemonPid: Int? = null private set diff --git a/app/src/main/java/com/itsaky/androidide/services/builder/ToolingServerRunner.kt b/app/src/main/java/com/itsaky/androidide/services/builder/ToolingServerRunner.kt index 1cf20319a5..52271db207 100644 --- a/app/src/main/java/com/itsaky/androidide/services/builder/ToolingServerRunner.kt +++ b/app/src/main/java/com/itsaky/androidide/services/builder/ToolingServerRunner.kt @@ -48,6 +48,14 @@ internal class ToolingServerRunner( private var listener: OnServerStartListener?, private var observer: Observer?, ) { + /** + * The server process's pid, or `null` before it has started. + * + * Volatile for the same reason as [GradleBuildService.gradleDaemonPid]: [startAsync] writes it + * from a coroutine on [runnerScope], and the editor reads it on the main thread when it + * re-adopts the watched processes after being recreated. + */ + @Volatile internal var pid: Int? = null private var job: Job? = null private var _isStarted = AtomicBoolean(false)