Failed to stop [ProcessPid(processName=perfetto, pid=...)] [323601788]

Fixed

Bug

Status Update

No update yet.

Description

cc...@google.com

created issue #1

Feb 2, 2024 11:57PM

Split off independent issue from reports in b/244621251#comment6 and b/244621251#comment7

Have an internal report of this at b/307649002

From that conversation, issue is likely that Macrobenchmark's perfetto death timeout of 3 seconds doesn't align with perfetto's 5s for stopping producers.

We should first try and resolve that, and can sort out what appears to be a producer problem after.

Comments

ap...@google.com <ap...@google.com> #2Feb 5, 2024 03:28PM

Project: platform/frameworks/support
Branch: androidx-main

commit adc821747c2a7f56b6708344dcb0dcc1c5707c90
Author: Rahul Ravikumar <rahulrav@google.com>
Date: Tue Apr 26 13:55:46 2022

Workaround compilation reset unavailable on user builds

* cmd package compile --reset does not work on user builds.
* To work around this, we copy the apk to the tmp directory, and then kick off an un-install and a re-install.
* A side-effect of uninstalling / re-installing is that ART profiles end up in vfy, which is the state we expect.
* Added `compilation.enabled` as an instrumentation test argument, to disable this behavior if the benchmark wants
full control on CompilationModes.

Test: SmallListStartupBenchmark
Fixes:

b/230665435

Bug:

b/230518212

Relnote: "Fixed an issue where compiled methods were not correctly being reset between each macrobenchmark on unrooted builds. This unfortunately requires reinstalling the apk each iteration, which will clear application data for each macrobenchmark."

Before:

SmallListStartupBenchmark_startup[startup=COLD,compilation=None]
startupMs min 198.5, median 210.5, max 222.6
timeToInitialDisplayMs min 196.6, median 208.0, max 220.5

SmallListStartupBenchmark_startup[startup=COLD,compilation=BaselineProfile]
startupMs min 190.7, median 205.3, max 212.2
timeToInitialDisplayMs min 188.8, median 202.2, max 210.3

After the change:

SmallListStartupBenchmark_startup[startup=COLD,compilation=None]
startupMs min 223.9, median 230.1, max 243.8
timeToInitialDisplayMs min 222.1, median 228.3, max 242.0

SmallListStartupBenchmark_startup[startup=COLD,compilation=BaselineProfile]
startupMs min 205.2, median 213.5, max 228.7
timeToInitialDisplayMs min 203.3, median 211.7, max 227.0

Change-Id: I31c740019640da0d0913066bb9ac59ab5c2a9627

M benchmark/benchmark-macro/src/main/java/androidx/benchmark/macro/MacrobenchmarkScope.kt
M benchmark/benchmark-macro/src/main/java/androidx/benchmark/macro/CompilationMode.kt
M benchmark/benchmark-common/src/main/java/androidx/benchmark/Arguments.kt

https://android-review.googlesource.com/2076902

cc...@google.com <cc...@google.com> #3Mar 4, 2024 06:55PM

Should we fix --reset? I haven't tested, but it looks like it's just a matter of changing enforceSystemOrRoot to enforceSystemOrRootOrShell on the line Rahul indicated.

As Nicolas argues in http://b/230518212#comment9 this doesn't seem like a big security regression - at worst it may make apps slower, right? I see a similar permission given to force dexopting a package. Should we loop in someone from security team on this?

lu...@meta.com <lu...@meta.com> #4Mar 12, 2024 04:44PM

Yes, we should still fix --reset such that it works on user builds. Maybe allow the shell to clear caches, and not the app's themselves ? The goal is to be able to make benchmarks easier.

Message last modified on Mar 12, 2024 04:44PM

cc...@google.com <cc...@google.com> #5Mar 13, 2024 08:07PM

This bug is for the behavior in macrobenchmark and the workaround.

The issue of opening up that behavior is being tracked at b/230518212

lu...@gmail.com <lu...@gmail.com> #6Jun 1, 2024 06:10PM

This fix made all compilation=none and compilation=warmup measurements slower in CI dashboard screenshot

E.g. androidx.compose.integration.macrobenchmark.SmallListStartupBenchmark_startup_startup_COLD_compilation_None_ went from 340 -> 380ms.

As these were rooted devices, this indicates --reset (done in our rooted CI) does not put the device into the same state as fully reinstallation (after CL landed).

Message last modified on Jun 1, 2024 06:11PM

ke...@netflix.com <ke...@netflix.com> #7Jul 5, 2024 07:01PM

Filed b/231457688 to investigate this performance gap.

ke...@netflix.com <ke...@netflix.com> #8Jul 5, 2024 07:06PM

Note from perf gap investigation in #comment7 - it's coincidental, caused by shader compilation, tracked in b/231455742

ke...@netflix.com <ke...@netflix.com> #9Jul 5, 2024 07:17PM

In the latest stable version of the library (compared to beta verison), it looks we are reinstalling the apk during each iteration, which is clearing application data for each macrobenchmark. Issue with that approach is, when measuring the performance (esp. FrameTimingMetric) for screens deep inside, the onboarding flow of the app needs to be repeated every time which is not ideal. We just want to measure the performance improvements for users who are updating to the latest versions. Can we not just clear the clear ART profiles between runs. Can you please advice how this can be achieved. Do let me know if you need more details on this issue.

ke...@netflix.com <ke...@netflix.com> #10Jul 5, 2024 07:24PM

Unfortunately it's not possible to just clear ART profiles prior to Android T on user builds. This is why we need to re-install the APK.

On the next iteration of Macrobenchmark (1.2.0-alpha01), we will try and clear profile caches if the session is rooted.

ke...@netflix.com <ke...@netflix.com> #11Jul 5, 2024 07:35PM

Thanks for the quick and prompt response. Sorry which version of android are you referring here when you say Android T

ke...@netflix.com <ke...@netflix.com> #12Jul 5, 2024 08:53PM

Android T is the development name for Android 13.

al...@intive.com <al...@intive.com> Sep 3, 2024 10:17AM

test-run-1.log

93 KB

View

Download

test-run-2.log

207 KB

View

Download

deleted

0 B

deleted

0 B

al...@intive.com <al...@intive.com> #13Sep 4, 2024 11:36AM

Hi, coming from the android docs here (

https://developer.android.com/topic/performance/baselineprofiles/overview) and just wanted to be clear on a point.

"Resetting ART profile caches isn't allowed on user (non-rooted) builds. To work around this, androidx.benchmark:benchmark-macro-junit4:1.1.0 includes a fix that reinstalls the app during the benchmark (issue)."

When the docs say a "fix that reinstalls the app during the benchmark", and in the above comment, where you talk about 'clearing application data for each macrobenchmark', I just wanted to be clear this relates to running benchmarks only. It does not mean that when we upload our baseline profile with our APK version for the first time, our users will lose their application data? I'm new to this area of development so thanks for patience with the potentially rudimentary enquiry...!

al...@intive.com <al...@intive.com> #14Sep 4, 2024 03:41PM

The app data is cleared only when running the Macrobenchmark locally on the device as part of the verification/generation process. No action is done to either the published app, or to apps on other devices.

When you upload the baseline profile with the APK/AAB, it's already in the generated (binary) form and it won't clear any user data.

mn...@google.com <mn...@google.com> #15Jan 14, 2025 02:30AM

I was able to occasionally reproduce this with my Pixel 5 with ToT on ScrollerBenchmark as well as TrivialKotlinBenchmark. Unfortunately I can't seem to reproduce this anymore, but here are some details anyway:

Device:

Google Pixel 5
Android 14

ke...@netflix.com <ke...@netflix.com> #16Jan 16, 2025 02:33PM

Nice work debugging in #13.
I can confirm I have the same issue on a Pixel 6 running Android 15. Same steps...same error. perfetto process can't be killed.
This seems like a bug in macrobenchmark? Or the Pixel 6 perfetto statsd autostartup.

Given this is a pretty popular test device, it should be easily reproducible...

sh...@gmail.com <sh...@gmail.com> #17Jan 23, 2025 09:17PM

Experiencing the same issue.

Stacktrace:

----- begin exception -----
01-23 21:15:05.513 938 964 E TestRunner: java.lang.IllegalStateException: Failed to stop [ProcessPid(processName=perfetto, pid=5165)]
Device:
Pixel 9 Pro XL | Android 15

ap...@google.com <ap...@google.com> #18Jan 24, 2025 07:16PM

Marked as fixed.

Project: platform/frameworks/support
Branch: androidx-main
Author: Chris Craik <ccraik@google.com>
Link: https://android-review.googlesource.com/3463345

Avoid crashes from external perfetto trace capture

Expand for full commit details

Avoid crashes from external perfetto trace capture 
 
Fixes: 323601788 
Test: ShellTest#killProcessesAndWait_failure 
Relnote: "Speculative fix for crashes: 'Failed to stop [ProcessPid(processName=perfetto, pid=...)]'. Now Benchmark will log a message instead of crashing when a backgroudn Perfetto process isn't able to be stopped before running the benchmark." 
 
From bugreport details, it appears that background perfetto traces are 
often triggered which cannot be terminated by shell. Log an error 
rather than crashing the benchmark. 
 
Change-Id: I37d3e254f7f3e49c53387dae89bb5bed7d2c9bf5

Files:

M benchmark/benchmark-common/src/androidTest/java/androidx/benchmark/PerfettoHelperTest.kt
M benchmark/benchmark-common/src/androidTest/java/androidx/benchmark/ShellTest.kt
M benchmark/benchmark-common/src/main/java/androidx/benchmark/Shell.kt
M benchmark/benchmark-common/src/main/java/androidx/benchmark/perfetto/PerfettoHelper.kt
M benchmark/benchmark-macro/src/androidTest/java/androidx/benchmark/macro/perfetto/AndroidxTracingTraceTest.kt
M benchmark/benchmark-macro/src/androidTest/java/androidx/benchmark/macro/perfetto/PerfettoCaptureSweepTest.kt

Hash: a1d00d7f334608a7d4975b209b4569cfdadb2539
Date: Thu Jan 23 16:10:27 2025

pr...@google.com <pr...@google.com> #19Feb 12, 2025 06:40PM

The following release(s) address this bug.It is possible this bug has only been partially addressed:

androidx.benchmark:benchmark-common:1.4.0-alpha08
androidx.benchmark:benchmark-macro:1.4.0-alpha08