Add Android Startup Perfetto trace support by LoopedBard3 · Pull Request #4776 · dotnet/performance

LoopedBard3 · 2025-03-18T21:40:13Z

Add support for getting perfetto traces for the android runs. This will start out as a primarily manual thing to help users of our scenario testing when doing manual runs, or manually setup pipeline runs.

…testing. Still need to add copy of the file to the upload dir if running in helix.

LoopedBard3 · 2025-03-18T23:46:37Z

Tested successfully locally and initial use is just in manual runs.

e-kharion · 2025-03-19T07:47:22Z

src/scenarios/shared/runner.py

+                        # Wait until the total time taken to start the app is greater than the max startup time + 3
+                        # This is to ensure that the trace has captured the entire startup process
+                        getLogger().info("Ensuring the trace capture has been completed.")
+                        while time.time() - perfetto_start_time_sec < max_startup_time_sec + 3:


where the 3 seconds come from?
Would it make a sense to try verify the trace capturing finished somehow?
This way it feels potentially fragile

Completely moved away from using the seconds and instead switched to killing the tracing after we finish the startup command. This background start and then kill seems to be the recommended approach after further digging in the docs: https://perfetto.dev/docs/concepts/config#android.

e-kharion · 2025-03-19T07:49:36Z

src/scenarios/shared/runner.py

+                        pull_trace_cmd = xharness_adb() + [
+                            'pull',
+                            perfetto_device_save_file,
+                            os.path.join(os.getcwd(), const.TRACEDIR, f'perfetto_startup_trace_{self.packagename}_{time.time()}.trace')


nit: Would it make sense to use formatted time instead of a number here? Would somebody be checking the files manually?

Would somebody be checking the files manually?

The idea is that users would upload the captured trace to https://ui.perfetto.dev and analyzed the results there.

Thanks. Does the filename format matter anyhow for the tool? If not (meaning the filename would not get processed by a machine) than some more human friendly time format make more sense to me. Still a nit, though.

Does the filename format matter anyhow for the tool?

Unless we do some very weird filename it shouldn't matter. Human friendly file name would be nice :)

Great idea, I have made the change to instead have a nicely formatted datetime instead. It will be in my next update.

e-kharion · 2025-03-19T07:55:25Z

src/scenarios/shared/runner.py

+
+                        perfetto_cmd = xharness_adb() + [
+                            'shell',
+                            f'perfetto --background --txt -o {perfetto_device_save_file} --time {max_startup_time_sec + 3}s -b 64mb sched freq idle am wm gfx view binder_driver hal dalvik camera input res memory'


where the +3 comes from?

I have no idea about the usual "magnitude" of the max_startup_time_sec - but if it's just some kind of reserve, it might be better to add some percentage to the original value (in case the value might vary significantly)?

We generally want to run the tracing for the startup duration, that is what is calculated in:
max_startup_time_sec = int(max(int(re.search(r"TotalTime: (\d+)", str(result)).group(1)) for result in allResults) / 1000).

+3s is probably just to introduce some slack if the startup would take unexpectedly longer.

I would maybe add the max_startup_time_sec + 3 as a separate variable value with the comment from below why we use + 3 to make it a bit clearer.

I have no idea about the usual "magnitude" of the max_startup_time_sec

In scenarios we are measuring the startup ranges from hundreds of milliseconds to low seconds. Making it, for example, 1.5x of the max startup should be good enough as well.

Completely moved away from using the seconds and instead switched to killing the tracing after we finish the startup command. This background start and then kill seems to be the recommended approach after further digging in the docs: https://perfetto.dev/docs/concepts/config#android. Still kept a max time for the tracing at 2x the max found startup, up to 60 seconds. At 60 seconds it throws an error as an arbitrary but, I think reasonable, cut off.

e-kharion

LGTM in general, left few comments/questions

e-kharion · 2025-03-19T08:02:41Z

src/scenarios/shared/runner.py

+
+                    try:
+                        # Get the current value of persist.traced.enable
+                        getLogger().info("Getting current persist.traced.enable value")


super nit: I know you follow the pattern here, but I feel that decomposing the code a bit would improve the readability/maintainability significantly. For example separating all the commands into some helper commands library class and only call those here would make the code much nicer in my opinion.

I agree with this assessment, so I made a task here: #4784.

matouskozak

Thanks a lot Parker!

…time human readable, and switching from catching the trace by taking the max startup and adding 3 seconds to instead just killing perfetto once startup testing is complete.

caaavik-msft · 2025-03-20T23:46:57Z

src/scenarios/shared/runner.py

+
+                        # Stop perfetto now that the app has started. Sending a Terminate signal should be enough per the longer trace capturing guidance here: https://perfetto.dev/docs/concepts/config#android.
+                        getLogger().info("Stopping perfetto trace capture")
+                        stop_perfetto_cmd = xharness_adb() + [


Should the perfetto killall also be in the finally block?

Additionally, it might make sense to run a precautionary perfetto killall before running the kill command at the top. I know a couple times we have been bitten by processes not being killed.

Having a kill before the execution is a great idea. I have updated the code per these comments. I included a perfetto kill in the finally block, though am not super worried about it as the time parameter in the call starting the trace should ensure it ends eventually.

…art and to kill as a finally command if we failed before the normal kill spot.

LoopedBard3 added the enhancement New feature or request label Mar 18, 2025

LoopedBard3 self-assigned this Mar 18, 2025

LoopedBard3 and others added 2 commits March 18, 2025 14:41

First pass at adding perfetto support for the android runs for local …

0e6ce40

…testing. Still need to add copy of the file to the upload dir if running in helix.

add perfetto config

fc53ef5

LoopedBard3 force-pushed the startup_perfetto_support branch from d340e3b to fc53ef5 Compare March 18, 2025 21:41

LoopedBard3 added 2 commits March 18, 2025 14:43

Cleanup for PR.

9bcd4fd

Make sure to reset persist.trace.enable if necessary.

4782db4

LoopedBard3 marked this pull request as ready for review March 18, 2025 23:22

LoopedBard3 requested a review from matouskozak March 18, 2025 23:22

LoopedBard3 marked this pull request as draft March 18, 2025 23:23

LoopedBard3 removed the request for review from matouskozak March 18, 2025 23:23

Add perfetto to android-startup-scenarios doc.

9b1f4ed

LoopedBard3 requested review from DrewScoggins, caaavik-msft, e-kharion and matouskozak March 18, 2025 23:45

LoopedBard3 marked this pull request as ready for review March 18, 2025 23:46

e-kharion reviewed Mar 19, 2025

View reviewed changes

e-kharion previously approved these changes Mar 19, 2025

View reviewed changes

e-kharion reviewed Mar 19, 2025

View reviewed changes

matouskozak previously approved these changes Mar 19, 2025

View reviewed changes

LoopedBard3 mentioned this pull request Mar 20, 2025

Improve Device scenario runner.py code by moving shared calls into separate helpers/functions #4784

Open

Update per PR feedback. Includes updating the trace name to make the …

d77604a

…time human readable, and switching from catching the trace by taking the max startup and adding 3 seconds to instead just killing perfetto once startup testing is complete.

LoopedBard3 dismissed stale reviews from matouskozak and e-kharion via d77604a March 20, 2025 22:03

caaavik-msft requested changes Mar 20, 2025

View reviewed changes

Add perfetto pkills to make sure perfetto is not running before we st…

dcfd0ff

…art and to kill as a finally command if we failed before the normal kill spot.

matouskozak approved these changes Mar 24, 2025

View reviewed changes

LoopedBard3 requested a review from caaavik-msft March 24, 2025 23:28

caaavik-msft approved these changes Mar 25, 2025

View reviewed changes

LoopedBard3 merged commit 56bd5d9 into dotnet:main Mar 25, 2025
77 of 84 checks passed

LoopedBard3 deleted the startup_perfetto_support branch March 25, 2025 17:01

Comments

Conversation

LoopedBard3 commented Mar 18, 2025

Uh oh!

LoopedBard3 commented Mar 18, 2025

Uh oh!

e-kharion Mar 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

e-kharion left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matouskozak left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

e-kharion Mar 19, 2025 •

edited

Loading