Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nightly builds with baseline profiles #1173

Merged
merged 27 commits into from
Sep 19, 2024
Merged

Nightly builds with baseline profiles #1173

merged 27 commits into from
Sep 19, 2024

Conversation

keyboardsurfer
Copy link
Member

@keyboardsurfer keyboardsurfer commented Jan 22, 2024

Change-Id: I162cd9eca724c30d839add912359482cbce7c68e

Thanks for submitting a pull request. Please include the following information.

What I have done and why
Enabled baseline profile generation for new nightly build type.
With KVM, GMD run smoothly and BP generation is not negatively affected any longer.
Enabling nightly builds for this keeps PR builds fast while allowing for baseline profiles to be generated regularly as well.

Enabled Startup Profiles for smaller classes.dex and faster app startup

Fixes #<issue_number_goes_here>

Do tests pass?

  • Run local tests on DemoDebug variant: ./gradlew testDemoDebug
  • Check formatting: ./gradlew --init-script gradle/init.gradle.kts spotlessApply

@keyboardsurfer keyboardsurfer changed the title DO NOT MERGE: Test Baseline Profile generation on KVM config Enable baseline profile generation as part of regular builds Jan 22, 2024
@keyboardsurfer keyboardsurfer changed the title Enable baseline profile generation as part of regular builds Nightly builds with baseline profiles Jan 22, 2024
@keyboardsurfer
Copy link
Member Author

RFR. @JoseAlcerreca & @dturner PTAL.

Copy link
Contributor

@mlykotom mlykotom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm generally ok with this, but it would be nice if Don/Jose took a look as they maintain the project.

.github/workflows/Nightly.yaml Outdated Show resolved Hide resolved
.github/workflows/Nightly.yaml Outdated Show resolved Hide resolved
.github/workflows/Nightly.yaml Outdated Show resolved Hide resolved
app/src/main/baseline-prof.txt Outdated Show resolved Hide resolved
Copy link
Contributor

@JoseAlcerreca JoseAlcerreca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I started the review assuming this nightly was only for BP generation and only realized you were creating a full nightly. I think the main build should be our SOT for status, so just make this one about BP.

.github/workflows/Nightly.yaml Outdated Show resolved Hide resolved
.github/workflows/Nightly.yaml Outdated Show resolved Hide resolved
.github/workflows/Nightly.yaml Outdated Show resolved Hide resolved
.github/workflows/Nightly.yaml Outdated Show resolved Hide resolved
.github/workflows/Nightly.yaml Outdated Show resolved Hide resolved
.github/workflows/Nightly.yaml Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Copy link

Combined test coverage report

Overall Project 40.4% 🍏

There is no coverage information present for the Files changed

Copy link

Combined test coverage report

Overall Project 40.4% 🍏

There is no coverage information present for the Files changed

Copy link

Combined test coverage report

Overall Project 40.4% 🍏

There is no coverage information present for the Files changed

@keyboardsurfer
Copy link
Member Author

@JoseAlcerreca could this be a flaky test?

com.google.samples.apps.nowinandroid.ui.SnackbarScreenshotTests > snackbarShown_tablet FAILED
    java.lang.AssertionError at SnackbarScreenshotTests.kt:236

If so, could you show me how to fix it or approve the PR so it can be merged?

@JoseAlcerreca
Copy link
Contributor

Let's merge #1395 first and make sure this one passes

@keyboardsurfer
Copy link
Member Author

SGTM

Copy link

Combined test coverage report

Overall Project 40.37% 🍏

There is no coverage information present for the Files changed

Copy link

Combined test coverage report

Overall Project 40.4% 🍏

There is no coverage information present for the Files changed

3 similar comments
Copy link

Combined test coverage report

Overall Project 40.4% 🍏

There is no coverage information present for the Files changed

Copy link

Combined test coverage report

Overall Project 40.4% 🍏

There is no coverage information present for the Files changed

Copy link

Combined test coverage report

Overall Project 40.4% 🍏

There is no coverage information present for the Files changed

Copy link

github-actions bot commented Sep 5, 2024

Combined test coverage report

Overall Project 42.34% 🍏

There is no coverage information present for the Files changed

Copy link

github-actions bot commented Sep 5, 2024

Combined test coverage report

Overall Project 42.34% 🍏

There is no coverage information present for the Files changed

Change-Id: I85f431fa4ceb73be87cba997eb8808961d4197cd
Change-Id: I79d0cfd8e7002b5c526fd3cf10867b0cd40a99eb
Change-Id: Ib6cbe7d9a69b51490aa36dc45d04faa2d141c9ab
Change-Id: I4eda2fc52a3ddcae026fbae57b0299bb539abeff
Change-Id: I112cb9b7a16a7c8b25751d226ce93cb69cd9cdcb
Change-Id: I605e220d073ea25638a19dcca4e682e68606eda5
Change-Id: I1c5ea67943f20c6f42ee46a76d4787013c247b95
Change-Id: Ibf64e73238d660772757c01b7d1595e0aa9bc8a4
b/287312019

Change-Id: I3c89605e1707ac25d3726e37e9254085cd698191
Copy link

Combined test coverage report

Overall Project 42.32% 🍏

There is no coverage information present for the Files changed

Change-Id: Ief864552cde2fd2969c56e57974bb8e403d229a7
Copy link

Combined test coverage report

Overall Project 42.32% 🍏

There is no coverage information present for the Files changed

Change-Id: Id7903de5b5f79c150a514ea186630d4f85f1b268
Copy link

Combined test coverage report

Overall Project 42.29% 🍏

There is no coverage information present for the Files changed

Change-Id: I50a03a73fd3f35260d4635e7fa7ff80144aa4ead
Copy link

Combined test coverage report

Overall Project 42.42% 🍏

There is no coverage information present for the Files changed

Copy link

Combined test coverage report

Overall Project 42.43% 🍏

There is no coverage information present for the Files changed

Added custom metrics to better understand how effecetive a baseline
profile is. These TraceSectionMetrics keep track of JIT compilations
as well as class initializations which should go down when a BP is
properly applied to the app.

Change-Id: Idd1044bdccc3cbaf64aba8d7a909c4d150f82341
Copy link

Combined test coverage report

Overall Project 42.33% 🍏

There is no coverage information present for the Files changed

@keyboardsurfer keyboardsurfer merged commit c01a129 into main Sep 19, 2024
4 checks passed
@lihenggui
Copy link
Contributor

It seems that not enabling the KVM before setting up GMD causes this problem.

https://github.com/android/nowinandroid/actions/runs/10953338005/job/30413441493

* What went wrong:
Execution failed for task ':benchmarks:pixel6Api33Setup'.
> A failure occurred while executing com.android.build.gradle.internal.tasks.ManagedDeviceInstrumentationTestSetupTask$ManagedDeviceSetupRunnable
   > com.android.build.gradle.internal.AvdSnapshotHandler$EmulatorSnapshotCannotCreatedException: Gradle was not able to complete device setup for: dev33_default_x86_64_Pixel_6
     The emulator failed to open the managed device to generate the snapshot.
     This is because the emulator closed unexpectedly (1),
     try updating the emulator and ensure a device can be run from Android Studio.

@keyboardsurfer keyboardsurfer deleted the bw/bpGenOnKvm branch September 23, 2024 11:18
@keyboardsurfer
Copy link
Member Author

@lihenggui the issue was caused by GMD requiring a lot more processing power than what the GitHub runners offer in the free tier. So the relevant step is this one

- name: Setup GMD
        run: ./gradlew :benchmarks:pixel6Api33Setup
          --info
          -Pandroid.experimental.testOptions.managedDevices.emulator.showKernelLogging=true
          -Pandroid.testoptions.manageddevices.emulator.gpu="swiftshader_indirect"

Which has to be executed before the entire build is kicked off.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants