[BPGP] When running ./gradlew assemble, it runs GMD setup task [283447020]

Fixed

Bug

Status Update

No update yet.

Description

ml...@google.com

created issue #1

May 19, 2023 02:16PM

Component Used: BPGP 1.2.0-alpha14

Our performance samples CI pipeline was running ./gradlew build, which triggers pixel6Api31Setup GMD task for the :baselineProfile plugin that applies BPGP.

I tried using build -x test or even assemble -x test but this task is always triggerd and fails (probably due to using ubuntu-latest).

Can we run this task only when generateBaselineProfile task is running?

Comments

ma...@google.com <ma...@google.com> #2May 19, 2023 08:24PM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit c60f33e229e31ab328ef6b59dab63b264954831c
Author: Alexandre Elias <aelias@google.com>
Date: Fri Jul 10 16:23:09 2020

Semantics no-op cleanups

Partly in response to lmr's broad code review, I did a pass of
superficial API/implementation cleanup. The main changes are:

- I changed each Boolean SemanticsProperty where false is equivalent to
not being present to take "Unit" instead. This is conceptually
clearer: it avoids questions like "can I cancel out a semantics from a
merged child by setting it to false?" Because "property = Unit" looks
weird, I also changed the style of these to "property()".

- I moved the Semantics id generator closer to where it's used, in
SemanticsModifierCore. I made it internal and an AtomicInt.
(Note that integer ids are heavily used in the Android
AccessibilityNodeInfo APIs so I can't simply remove them entirely.)

- I deleted dead code. Some examples include SemanticsHintOverrides,
a public API not connected to anything, and SemanticsPropertyKey
merge() open method which is never called. (In both cases I have
a different plan in mind for accessibility.)

Fixes: 145951226
Fixes: 145955412
Test: existing tests
Relnote: "Single-value semantics properties now use a calling style.
For example, 'semantics { hidden = true }' is now written as:
'semantics { hidden() }'."

Change-Id: Ic1afd12ea22c926babc9662f1804d80b33aa0cfc

M ui/integration-tests/benchmark/src/androidTest/java/androidx/ui/benchmark/test/LayoutNodeModifierBenchmark.kt
M ui/ui-core/api/0.1.0-dev15.txt
M ui/ui-core/api/current.txt
M ui/ui-core/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-core/api/public_plus_experimental_current.txt
M ui/ui-core/api/restricted_0.1.0-dev15.txt
M ui/ui-core/api/restricted_current.txt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/graphics/vector/VectorTest.kt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/semantics/SemanticsTests.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidActuals.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeView.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeViewAccessibilityDelegateCompat.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidPopup.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/Expect.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsConfiguration.kt
D ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsHintOverrides.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsModifier.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsNode.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsOwner.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsWrapper.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/semantics/SemanticsProperties.kt
M ui/ui-foundation/api/0.1.0-dev15.txt
M ui/ui-foundation/api/current.txt
M ui/ui-foundation/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-foundation/api/public_plus_experimental_current.txt
M ui/ui-foundation/api/restricted_0.1.0-dev15.txt
M ui/ui-foundation/api/restricted_current.txt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Clickable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Dialog.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Scroller.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Selectable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Toggleable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/semantics/FoundationSemanticsProperties.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ButtonTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CardTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CheckboxScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/RadioButtonScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ScaffoldTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SnackbarTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SurfaceTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ripple/RippleIndicationTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/textfield/TextFieldScreenshotTest.kt
M ui/ui-material/src/main/java/androidx/ui/material/AppBar.kt
M ui/ui-material/src/main/java/androidx/ui/material/TextFieldImpl.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/AssertsTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/CallSemanticsActionTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ErrorMessagesTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/FindersTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/PrintToStringTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ScrollToTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/TextActionsTest.kt
M ui/ui-test/src/main/java/androidx/ui/test/Actions.kt
M ui/ui-test/src/main/java/androidx/ui/test/Filters.kt
M ui/ui-text/api/0.1.0-dev15.txt
M ui/ui-text/api/current.txt
M ui/ui-text/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-text/api/public_plus_experimental_current.txt
M ui/ui-text/api/restricted_0.1.0-dev15.txt
M ui/ui-text/api/restricted_current.txt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/CoreTextField.kt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/TextSemanticsProperties.kt

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

ml...@google.com <ml...@google.com> #3May 19, 2023 08:33PM

Pls check android/performance-samples where it happens (check opened PRs)

On Fri, May 19, 2023, 22:24 maralb <buganizer-system+maralb@google.com>
wrote:

- Show quoted text -

ma...@google.com <ma...@google.com> May 23, 2023 06:53PM

Accepted by ma...@google.com.

ma...@google.com <ma...@google.com> #4May 24, 2023 04:46PM

I think the issue here is that calling assemble will not just build the variants but also "prepare" the configurations with any required artifact. As a result, the configurations exposed to retrieve the baseline profile artifact (in the producer module) will lazily trigger baseline profile generation.

ma...@google.com <ma...@google.com> #5May 24, 2023 05:35PM

There is no easy way to do this. This is because you may have your own tasks triggering generateBaselineProfile, or you want simply to run the tests to generate the profile without actually updating the consumer. Or because you do expect that calling assemble will update the profiles.

I would simply rely on documentation here.

The right syntax to disable those tasks (so that profiles won't be generated when calling assemble) in that project is:

./gradlew assemble -x :baselineProfile:pixel6Api31Setup -x :baselineProfile:pixel6Api31NonMinifiedReleaseAndroidTest

We could add a property if it makes it easier, since it's an issue specific to ubuntu github runners, something along the lines of:

-Pandroidx.baselineprofiles.skipgeneration

That basically does the same of the -x options I mentioned above.

ml...@google.com <ml...@google.com> #6May 24, 2023 09:09PM

While I'm ok with adding the parameters, it feels like a hard-to-understand problem for a newcomer that just applies the plugin and suddenly their build or assemble tasks start failing (or start taking way longer).

ma...@google.com <ma...@google.com> #7May 24, 2023 09:21PM

I agree but there isn't much we can do. This is also not related to AGP but to how gradle works: assemble is a task that also "prepares" the configurations (that in gradle are used to carry artifacts). When triggering the configuration we use to expose the baseline profile artifacts the GMD device setup also starts, since it's part of the task chain to produce the artifact.

The only option we've is to decide when to disable those specific tasks, so that the configuration won't have any artifact and don't generate/update the baseline profile.

The property I've implemented is one way. The other way I've explored is to check what task the user invoked but this can potentially break other flows. For example, consider if the user launched:

./gradlew :baselineprofile:assemble :app:generateBaselineProfile :app:assemble

The result is that the baseline profile wouldn't be generated (or updated), the app would be assembled and there would be no failure. Other examples could be cases in which the user defined its own task and adds a dependency on the generation task.

Disabling some tasks based on what the user launched might ultimately be worse in other settings. I think a good trade-off is documentation and the above mentioned property but I'm open to ideas.

Message last modified on May 24, 2023 10:19PM

ap...@google.com <ap...@google.com> #8May 27, 2023 12:02AM

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

commit 79938912e132e8fb24ece4e24d067637e8ea5fe0
Author: Marcello Albano <maralb@google.com>
Date: Wed May 24 11:42:52 2023

Added option to skip baseline profile generation

This PR introduces a new property
`androidx.baselineprofile.skipgeneration` used by the baseline profile
producer plugin. When the property is on it will disable test tasks and
provide empty artifact for baseline profiles. This is helpful for
Github CI projects that don't support KVM.

Enabling this property has the same effect of:
./gradlew assemble \
-x :baselineProfile:pixel6Api31NonMinifiedReleaseAndroidTest \
-x :baselineProfile:collectNonMinifiedReleaseBaselineProfile

Test: ./gradlew :benchmark:benchmark-baseline-profile-gradle-plugin:test
Bug:

b/283447020

Change-Id: I37fda3f4b5761d8229c83be7d7a2cda197e8dbc8
Relnote: Added property to skip baseline profile generation. Usage: `./gradlew assemble -Pandroidx.baselineprofile.skipgeneration`.

M benchmark/baseline-profile-gradle-plugin/src/main/kotlin/androidx/baselineprofile/gradle/consumer/task/MergeBaselineProfileTask.kt
M benchmark/baseline-profile-gradle-plugin/src/main/kotlin/androidx/baselineprofile/gradle/producer/BaselineProfileProducerPlugin.kt
A benchmark/baseline-profile-gradle-plugin/src/main/kotlin/androidx/baselineprofile/gradle/producer/BaselineProfileProducerProperties.kt
M benchmark/baseline-profile-gradle-plugin/src/main/kotlin/androidx/baselineprofile/gradle/producer/tasks/CollectBaselineProfileTask.kt
M benchmark/baseline-profile-gradle-plugin/src/main/kotlin/androidx/baselineprofile/gradle/producer/tasks/InstrumentationTestTaskWrapper.kt
M benchmark/baseline-profile-gradle-plugin/src/test/kotlin/androidx/baselineprofile/gradle/consumer/BaselineProfileConsumerPluginTest.kt
M benchmark/baseline-profile-gradle-plugin/src/test/kotlin/androidx/baselineprofile/gradle/producer/BaselineProfileProducerPluginTest.kt

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

ma...@google.com <ma...@google.com> May 30, 2023 05:45PM

Marked as fixed.

pr...@google.com <pr...@google.com> #9Jun 8, 2023 01:31AM

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

androidx.benchmark:benchmark-baseline-profile-gradle-plugin:1.2.0-alpha15