Enable minification for microbenchmarks in androidx + compose [184378053]

Fixed

Bug

Status Update

No update yet.

Description

du...@google.com

created issue #1

Apr 2, 2021 08:18PM

Ideally we can track improvements for minified code (post r8 transforms) as compose sometimes make optimizations depending on these, e.g., changes to enable aot to inline.

It's currently impossible to apply:

release {
  testCoverageEnabled = false
  minifyEnabled true
  shrinkResources true
  proguardFiles getDefaultProguardFile("proguard-android-optimize.txt"), 'proguard.pro'
}

to a benchmark module, even if has been converted to an app module because it fails with either:

04-02 12:11:50.143 22825 22846 D Benchmark: launching Benchmark IsolationActivity
04-02 12:11:50.148 22825 22846 E TestRunner: failed: Test mechanism
04-02 12:11:50.148 22825 22846 E TestRunner: ----- begin exception -----
04-02 12:11:50.149 22825 22846 E TestRunner: java.lang.RuntimeException: Could not launch activity

BroadcastQueue: Background execution not allowed: receiving Intent { act=android.intent.action.PACKAGE_REMOVED dat=package:androidx.compose.runtime.benchmark flg=0x4000010 (has extras) } to com.android.musicfx/.Compatibility$Receiver

(Wembley on R)

junit.framework.AssertionFailedError: Failed to grant permissions, see logcat for details
at junit.framework.Assert.fail(Assert.java:50)
at androidx.test.runner.permission.PermissionRequester.requestPermissions(PermissionRequester.java:111)
at androidx.test.rule.GrantPermissionRule$RequestPermissionStatement.evaluate(GrantPermissionRule.java:133)
at org.junit.rules.RunRules.evaluate(RunRules.java:20)
at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)

Comments

cc...@google.com <cc...@google.com> Apr 9, 2021 11:01PM

Reassigned to cc...@google.com.

cc...@google.com <cc...@google.com> #2Apr 10, 2021 12:53AM

Yigit, do you have time to fix it?

reemission of the same liveData is racy

cc...@google.com <cc...@google.com> #3Apr 15, 2021 06:55PM

Reassigned to ch...@google.com.

yea i'll take it.

cc...@google.com <cc...@google.com> #4Apr 15, 2021 10:11PM

Thanks for the detailed analysis. This may not be an issue anymore since we've started using Main.immediate there but I' not sure; I'll try to create a test case.

Thu Apr 15 2021 15:10:03 GMT-0700 (Pacific Daylight Time).png

62 KB

View

Download

ch...@google.com <ch...@google.com> #5Apr 16, 2021 04:23PM

just emitting same live data reproduces the issue.

@Test
fun raceTest() {
val subLiveData = MutableLiveData(1)
val subject = liveData(testScope.coroutineContext) {
emitSource(subLiveData)
emitSource(subLiveData) //crashes
}
subject.addObserver().apply {
testScope.advanceUntilIdle()
}
}

du...@google.com <du...@google.com> #6Apr 17, 2021 05:32AM

With 2.2.0-alpha04 (that use Main.immediate), the issue seems to be still there (I tested it by calling emitSource() twice, like your test case)

du...@google.com <du...@google.com> #7Apr 17, 2021 05:32AM

yea sorry immediate does not fix it.
I actually have a WIP fix for it:

https://android-review.googlesource.com/c/platform/frameworks/support/+/1112186

if your case is the one i found (emitting same LiveData multiple times, as shown in #5) you can work around it by adding a dummy transformation.

val subLiveData = MutableLiveData(1)
val subject = liveData(testScope.coroutineContext) {
emitSource(subLiveData.map {it })
emitSource(subLiveData.map {it} )
}

cc...@google.com <cc...@google.com> #8Apr 21, 2021 06:48PM

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

commit af12e75e6b4110f48e44ca121466943909de8f06
Author: Yigit Boyar <yboyar@google.com>
Date: Tue Sep 03 12:58:11 2019

Fix coroutine livedata race condition

This CL fixes a bug in liveData builder where emitting same
LiveData source twice would make it crash because the second
emission registry could possibly happen before first one is
removed as source.

We fix it by using a suspending dispose function. It does feel
a bit hacky but we cannot make DisposableHandle.dispose async
and we do not want to block there. This does not mean that there
is a problem if developer disposes it manually since our emit
functions take care of making sure it disposes (and there is
no other way to add source to the underlying MediatorLiveData)

Bug: 140249349
Test: BuildLiveDataTest#raceTest_*

Change-Id: I0b464c242a583da4669af195cf2504e2adc4de40

M lifecycle/lifecycle-livedata-ktx/api/2.2.0-alpha05.txt
M lifecycle/lifecycle-livedata-ktx/api/current.txt
M lifecycle/lifecycle-livedata-ktx/api/public_plus_experimental_2.2.0-alpha05.txt
M lifecycle/lifecycle-livedata-ktx/api/public_plus_experimental_current.txt
M lifecycle/lifecycle-livedata-ktx/api/restricted_2.2.0-alpha05.txt
M lifecycle/lifecycle-livedata-ktx/api/restricted_current.txt
M lifecycle/lifecycle-livedata-ktx/src/main/java/androidx/lifecycle/CoroutineLiveData.kt
M lifecycle/lifecycle-livedata-ktx/src/test/java/androidx/lifecycle/BuildLiveDataTest.kt

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

https://goto.google.com/android-sha1/af12e75e6b4110f48e44ca121466943909de8f06

cm...@google.com <cm...@google.com> #9Jun 4, 2021 11:07AM

Just checking I follow your logic in #3 - the challenge is that you want all the test entry points to be roots for R8 to keep - which currently is not supported for app tests (and is potentially a bit odd for 'normal' app users as having the production shrunk build depending on tests might be suprising) and that moving them to the main sourceset breaks studio being able to run them.

So possibilities look like androidx only

Create an app with the tests with different build types for shrunk/not shrunk
- For studio support split the @Test bodies in to a class in src/main and add keep rules for that class, or manually curate keep rules. agp/studio
Fix studio running tests that are in main in an app
Support tracing @Tests in androidTest when shrinking the app in AGP
Add special support to AGP for shrinking library tests
- Shrinker support in library is
Add test only module support for shrinking and library targets.

cc...@google.com <cc...@google.com> #10Jun 4, 2021 07:46PM

Yes, (not sure what you mean by androidx only, since some of them are general solutions) though I'd add another possibility is the AGP support for macrobenchmarks source directory within existing modules, as we discussed a while back.

Also, the more general bug for R8 + Microbenchmark is here: b/171496730, this bug was filed to track investigating temporary, compose-specific workarounds.

cc...@google.com <cc...@google.com> Oct 19, 2023 07:39PM

Reassigned to cc...@google.com.

cc...@google.com <cc...@google.com> #11Dec 4, 2023 10:48PM

This is now possible with AGP changes, I was able to get simple tests running with minification on in a standalone project.

Once the required version of AGP lands in AndroidX, I'll add an example for local experimentation with R8 in microbenchmarks, and we can consider options for how we want to use this in CI.

These are the rules I used to test the integration in a standalone project:

# basic protection against junit/androidx.test reflection, shouldn't affect library/test code
-keepclasseswithmembers class androidx.test.** { *; }
-keepclasseswithmembers class org.junit.** { *; }
-dontwarn com.google.errorprone.annotations.MustBeClosed

## keep test classes
-keepclasseswithmembers @org.junit.runner.RunWith class * { *; }

## needed for org.junit.Test annotation to be discoverable by reflection
-keepattributes *Annotation*

ap...@google.com <ap...@google.com> #12Mar 11, 2024 08:12PM

Marked as fixed.

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

commit cecc57fb5899034f12f886a2784129098d059a8d
Author: Chris Craik <ccraik@google.com>
Date: Tue Mar 05 16:46:59 2024

Experimental R8 support in microbenchmarks

Off by default in the AndroidX repo, but can be experimentally used by
external. As this isn't run in presubmit/postsubmit, it may require
adding additional rules for any specific test.

```
(androidTest.enableMinification = false)
840,772 ns 389 allocs Trace LazyListScrollingBenchmark.scrollProgrammatically_newItemComposed[LazyColumn]
837,786 ns 389 allocs Trace LazyListScrollingBenchmark.scrollProgrammatically_newItemComposed[LazyRow]

(androidTest.enableMinification = true)
658,050 ns 390 allocs Trace LazyListScrollingBenchmark.scrollProgrammatically_newItemComposed[LazyColumn]
651,393 ns 390 allocs Trace LazyListScrollingBenchmark.scrollProgrammatically_newItemComposed[LazyRow]
```
(Mokey, locked clocks)

Test: LazyListScrollingBenchmark.scrollProgrammatically_newItemComposed
Fixes: 184378053
Relnote: """Experimental R8 support in microbench via embedded proguard
rules. Note that this support is experimental, and requires AGP 8.3
for minification of library module tests. Use the following to enable
in your benchmark module's `build.gradle`:
```
android {
buildTypes.release.androidTest.enableMinification = true
}
```
"""

Change-Id: I738a3294c5ded7b336ed0f49d0615eb9231cce51

M benchmark/benchmark-junit4/build.gradle
A benchmark/benchmark-junit4/proguard-rules.pro
M compose/benchmark-utils/build.gradle
A compose/benchmark-utils/proguard-rules.pro
M compose/foundation/foundation/benchmark/build.gradle

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

cc...@google.com <cc...@google.com> #13Mar 11, 2024 09:03PM

With the above CL, this can be enabled locally with the following in your build.gradle:

    android {
        buildTypes.release.androidTest.enableMinification = true
    }

No further proguard customization needed in most cases.

pr...@google.com <pr...@google.com> #14Mar 20, 2024 08:26PM

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

androidx.benchmark:benchmark-junit4:1.3.0-alpha02
androidx.compose.foundation:foundation:1.7.0-alpha05
androidx.compose.foundation:foundation-android:1.7.0-alpha05
androidx.compose.foundation:foundation-desktop:1.7.0-alpha05