[PERF] Enable Android CoreCLR and device startup testing #112939

LoopedBard3 · 2025-02-26T05:40:40Z

Added runs to build the Sample Android app with runtime flavor CoreCLR, added back in the device startup jobs for android, and completed some cleanup both to make things more concise and to make the flow work with our current system.

Test run: https://dev.azure.com/dnceng/internal/_build/results?buildId=2650816&view=results
Associated performance PR: dotnet/performance#4738

dotnet-policy-service · 2025-02-26T05:41:10Z

Tagging subscribers to this area: @dotnet/runtime-infrastructure
See info in area-owners.md if you want to be subscribed.

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (3)

eng/pipelines/performance/templates/perf-build-jobs.yml:79

Please confirm that the variable $(_BuildConfig) is defined in the pipeline context or replace it with a fixed value, as an undefined variable could lead to build configuration issues.

buildArgs: -s clr.runtime+clr.alljits+clr.corelib+clr.nativecorelib+clr.tools+clr.packages+libs -c $(_BuildConfig)

eng/pipelines/performance/templates/build-perf-sample-apps.yml:2

Verify that the removal of several parameters (such as osSubgroup, archType, buildConfig, etc.) does not break any dependent steps or external references elsewhere in the pipeline.

-   osSubgroup: ''

eng/pipelines/performance/templates/perf-ios-scenarios-build-jobs.yml:17

Confirm that removing the explicit artifact definitions in favor of only passing hybridGlobalization does not affect downstream artifact naming conventions or related processes.

-              artifactName: iOSMonoarm64

caaavik-msft

Just one comment, apart from that should be good to merge

caaavik-msft · 2025-02-26T06:17:51Z

eng/pipelines/performance/templates/build-perf-sample-apps.yml

-  runtimeFlavor: ''
-  helixQueues: ''
-  targetRid: ''
+  flavor: 'mono' # Currently only used for Android Hello World app


should we call it runtimeFlavor for consistency with other yaml files and scripts?

I initially started out with it being runtimeFlavor but ended up hitting an error in the yaml as it collided with a separately set runtimeFlavor. In my testing, this colliding runtimeFlavor didn't seem to get properly passed through to the build-perf-sample-apps either. I will add a comment about that.

Thinking about it some more and looking at some deeper parts of the pipeline, we end up saving what is essentially this value in RuntimeType. Would it make sense to use that as the name here to make a clear connection between the two? At a high level, I think it seems like an alright name substitution but maybe there is some context I am missing.

Thanks for documenting this. I am bit apprehensive about the fact that we have flavor and runtimeFlavor. Is something that we can fix long term or the cost would be too big?

matouskozak · 2025-02-26T07:43:02Z

eng/pipelines/performance/templates/perf-ios-scenarios-build-jobs.yml

-              artifactName: iOSMonoarm64
-              archiveExtension: '.tar.gz'
-              archiveType: tar
-              tarCompression: gz


Why is this no longer needed?

All except the artifact name were not being used at all in the app build yaml, so they were just left over from when I picked up the project. For the artifact name, we were using it with the configuration information to generate the names of the artifacts, but since it was only used for iOS and only a handful of places, I decided to remove it with the other options.

matouskozak

LGTM!

eng/testing/performance/android_scenarios.proj

eng/pipelines/performance/templates/build-perf-sample-apps.yml

ivanpovazan · 2025-02-26T09:20:44Z

eng/pipelines/performance/templates/build-perf-sample-apps.yml

  hybridGlobalization: False

 steps:
 # Build Android sample app
  - ${{ if eq(parameters.osGroup, 'android') }}:
-    - script: make run TARGET_ARCH=arm64 DEPLOY_AND_RUN=false
+    - script: make run TARGET_ARCH=arm64 DEPLOY_AND_RUN=false RUNTIME_FLAVOR=${{ parameters.flavor }}


Do we want to have this parametrised?

I imagined we would have separate steps for Mono and for CoreCLR for the fact that different runtimes will have different configurations, e.g., CoreCLR does not have interpreter, Mono does, CoreCLR has R2R, Mono doesn't, etc.

I suggest splitting it up into separate steps.

I don't have a strong preference for whether to parameterize this or not. Being that this uses a make file and only builds the app, I think parameterizing this part does not really have an impact on the rest of the flow. I also don't know if the other configurations need specific extra setup. @matouskozak, do you have an idea of whether running make for the other build configurations needs specific extra setup steps?

I believe that the runtime build doesn't change, we just set the makefile variable to enable the configuration. The issue might be that there are separate variables for CoreCLR and Mono as Ivan mentioned...

CoreCLR has R2R, Mono has AOT, Mono has interpreter (CoreCLR not).

That makes sense, I will separate them out between mono and coreclr to help make the additional options for each more clear in the future.

ivanpovazan · 2025-02-26T09:25:40Z

eng/pipelines/performance/templates/build-perf-sample-apps.yml

@@ -48,7 +34,7 @@ steps:
      displayName: 'Publish binlog'
      inputs:
        pathtoPublish: $(Build.SourcesDirectory)/src/mono/sample/iOS/msbuild.binlog
-        artifactName:  ${{ parameters.artifactName }}
+        artifactName:  iOSMonoArm64NoLLVMNoStripSymbolsBuildLog


As a future task, I think all the namings should be refactored

Do you have an idea or preference for how the names should be refactored? We should have decent leeway to name the artifacts as how we want.

I don't have a strong preference, but here are couple of things to consider:

unifying the naming of the apps, e.g., sometimes we call it Sample, sometimes, HelloWorld, but it is actually called HelloiOS

clean up which variations we care about, for example HYBRID_GLOBALIZATION is always true on iOS, we don't need to pass it include it in the naming etc.

have a consistent naming scheme (e.g., <app_name>_<target_os>-<target_arch>_<config>_<symbols>_<runtime_flavor>_<runtime_flavor_switch1>_...) examples:

HelloiOS_ios-arm64_Release_Symbols_Mono_AOT_LLVM_Interpreter

HelloiOS_ios-arm64_Release_NoSymbols_CoreCLR_R2R

HelloiOS_ios-arm64_Release_Symbols_NativeAOT

These are just some ideas, it is not mandatory but would simplify maintenance and usage.
Before changing anything (as part of another PR) I suggest we first agree on how exactly we would like the change to look like.

eng/testing/performance/android_scenarios.proj

ivanpovazan

LGTM, thanks!

… HelloWorld app and updated android_scenarios to only run the BDN app tests for Mono RuntimeType. Also cleaned up the sample apps build file, removing the unnecessary parameters. Remove unneeded xharness setup code. Updated per feedback. Removed the noanimation device startup test, added binlog upload to android build steps, and split mono and coreclr build steps. Rename the mobile sample app build parameter from flavor to runtimeType to better match up with it's downstream connections.

LoopedBard3 · 2025-03-04T00:04:25Z

Pushed to fix the merge conflicts.

LoopedBard3 added the area-Infrastructure label Feb 26, 2025

LoopedBard3 self-assigned this Feb 26, 2025

LoopedBard3 marked this pull request as ready for review February 26, 2025 06:00

LoopedBard3 requested review from Copilot, DrewScoggins, matouskozak, caaavik-msft and e-kharion February 26, 2025 06:00

Copilot AI reviewed Feb 26, 2025

View reviewed changes

caaavik-msft approved these changes Feb 26, 2025

View reviewed changes

matouskozak approved these changes Feb 26, 2025

View reviewed changes

matouskozak mentioned this pull request Feb 25, 2025

Enable dotnet/runtime Android sample app measurements dotnet/performance#4734

Open

5 tasks

matouskozak requested review from ivanpovazan and jkurdek February 26, 2025 08:14

matouskozak reviewed Feb 26, 2025

View reviewed changes

eng/testing/performance/android_scenarios.proj Outdated Show resolved Hide resolved

matouskozak mentioned this pull request Feb 26, 2025

[PERF] all build jobs used for performance testing are now possible to run inividually #112902

Merged

ivanpovazan reviewed Feb 26, 2025

View reviewed changes

LoopedBard3 mentioned this pull request Feb 26, 2025

Enable Android CoreCLR and device startup testing dotnet/performance#4738

Merged

ivanpovazan approved these changes Feb 28, 2025

View reviewed changes

e-kharion approved these changes Mar 3, 2025

View reviewed changes

LoopedBard3 force-pushed the PerfAddAndroidBuildsAndTests branch from dbc1a9c to d35f029 Compare March 3, 2025 22:58

LoopedBard3 merged commit 2b3b2d6 into dotnet:main Mar 4, 2025
23 checks passed

LoopedBard3 deleted the PerfAddAndroidBuildsAndTests branch March 4, 2025 00:05

matouskozak mentioned this pull request Mar 7, 2025

new perf-build pipeline added #113221

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[PERF] Enable Android CoreCLR and device startup testing #112939

[PERF] Enable Android CoreCLR and device startup testing #112939

LoopedBard3 commented Feb 26, 2025 •

edited

Loading

dotnet-policy-service bot commented Feb 26, 2025

caaavik-msft left a comment

caaavik-msft Feb 26, 2025

LoopedBard3 Feb 26, 2025

LoopedBard3 Feb 26, 2025

jkurdek Feb 28, 2025

matouskozak Feb 26, 2025

LoopedBard3 Feb 26, 2025

matouskozak left a comment

ivanpovazan Feb 26, 2025

LoopedBard3 Feb 26, 2025

matouskozak Feb 26, 2025

LoopedBard3 Feb 26, 2025

ivanpovazan Feb 26, 2025

LoopedBard3 Feb 26, 2025

ivanpovazan Feb 28, 2025

ivanpovazan left a comment

LoopedBard3 commented Mar 4, 2025

[PERF] Enable Android CoreCLR and device startup testing #112939

[PERF] Enable Android CoreCLR and device startup testing #112939

Conversation

LoopedBard3 commented Feb 26, 2025 • edited Loading

dotnet-policy-service bot commented Feb 26, 2025

Choose a reason for hiding this comment

caaavik-msft left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matouskozak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ivanpovazan left a comment

Choose a reason for hiding this comment

LoopedBard3 commented Mar 4, 2025

LoopedBard3 commented Feb 26, 2025 •

edited

Loading