-
Notifications
You must be signed in to change notification settings - Fork 315
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
Support minification with better consumer proguard rules #2167
Comments
@ChrisCraik keeping is not just about speed, more about size. If you keep the whole of In some cases androidx.test is so large, that the tests themselves don't even fit in the first dex, so we need other keep rules (can't find the reference 😔) to keep tests and some test framework files in the first dex. @implementor, please only keep what's absolutely necessary. And let R8 do its job for the rest. On a related note, please don't use |
@TWiStErRob are the size concerns from androidx.test itself or its dependencies? I took a quick look at the assembled apks from this repo's gradle-tests, and androidx.test.* itself contributes at most ~ 5K methods. Most androidx.test libraries should intentionally have a relatively lean set of dependencies. The assembled size of our espresso-core unit test apk is 1.6 MB for example. The outliers on dependency size are likely androidx.test.espresso:contrib, accessibility and androidx.test.ext:truth. That being said I think its reasonable to ask us to put a bit of work into ensuring we have a more targeted minimal set of keep rules. Also correct me if I'm wrong but the first dex concerns are only relevant if you are targeting android platforms that do not support native multidex (APIS < 21). Currently androidx.test has a min sdk of 19, but bumping this up to 21 is on the not-too-far-off horizon. |
To clarify, I wasn't proposing an ideal set of rules, just a set that got things working immediately for my use case - microbenchmarking. With those rules, the attached sample comes out to 5K methods, and without them, 21K methods. I mentioned speed instead of size because size isn't important to the use case of microbenchmarking (hot, compiled codepaths as opposed to e.g. startup), and by supporting R8 we (and users) can expect to see roughly 25% perf boost, so numbers get that much more accurate. It's this speed benefit that's motivating us to ship the suboptimal ruleset above, but note that benchmarks are in self-contained benchmark-only modules due to being compiled differently, so it's unlikely to affect non-benchmark scenarios. I agree that a more minimal ruleset would be ideal, but those are easier to maintain and keep minimal if they're upstream in androidx.test |
Description
Currently, androidx.test doesn't bundle in sufficient proguard rules, which makes testing with minification difficult, especially in self-instrumenting tests which otherwise don't suffer from the test apk / app apk retention issues.
Note that AGP 8.3 recently added support for test minification
In androidx.benchmark we're looking to add these rules on our own for the time being, but it would be preferable and more generally useful to other libraries interested in minification if these could be handled directly by
androidx.test
.Steps to Reproduce
Run sample, commenting out the testProguardRules in
build.gradle
Note that sample includes simple case (ExampleTest) and more complex case (ExampleBenchmark). Benchmark gradle plugin already sets
testBuildType="release"
Expected Results
Self-instrumenting
com.android.library
module tests don't require additional proguard rules by defaultActual Results
Several failures, requiring several workarounds in proguard file (not including those from b/328649293, which are an AGP issue):
AndroidX Test and Android OS Versions
androidx.test.ext:junit:1.1.5
androidx.test:runner:1.5.2
The above rules could likely be made much more minimal, but in microbenchmark, we know androidx.test/junit/kotlin-test are never on the critical path, so we keep aggressively.
Link to a public git repo demonstrating the problem:
See attached repro project: androidxTestR8MicroSample.zip
The text was updated successfully, but these errors were encountered: