Skip full compilation if the app is already full aot compiled [230453509]

Fixed

Bug

Status Update

No update yet.

Description

py...@squareup.com

created issue #1

Apr 26, 2022 07:34PM

We're running several macrobenchmark tests with full AOT compilation. Our app is large so full AOT compilation takes a long time. It would be nice if the macrobenchmark code that triggers full AOT compilation had a check for "is this app already full AOT compiled? If yes, nothing to do"

This would be harder to do with partial compilation unless you were able to cache the previous profile inputs and compare it against the new ones.

Comments

cc...@google.com <cc...@google.com> #2May 11, 2022 06:33PM

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

py...@squareup.com <py...@squareup.com> #3May 11, 2022 09:41PM

Note: ideally there's be a way to turn off all install related behavior without having to pass in an arg flag. If I want to handle installation & optimization without benchmark getting in the way, there should be an API for that. Arguably that's what Compilation.None should do, or there should be a Compilation.Interpreted and a Compilation.NoOp.

py...@squareup.com <py...@squareup.com> #4May 11, 2022 09:43PM

Also, generally clearing profiles doesn't make much sense for Full compilation mode. I understand this makes sense for profile based benchmarks, but not really for full where the output shouldn't vary.

cc...@google.com <cc...@google.com> #5May 11, 2022 09:51PM

ideally there's be a way to turn off all install related behavior without having to pass in an arg flag.

We can't change APIs in an RC, but we'll consider it for 1.2

Arguably that's what Compilation.None should do, or there should be a Compilation.Interpreted and a Compilation.NoOp.

None means "none of my app should be compiled", so at least a reset (if compilation has happened before) should occur. Compilation.NoOp or equivalent sounds reasonable.

Also, generally clearing profiles doesn't make much sense for Full compilation mode.

I assume so too, I would want to validate that with ART and test it, but either way, we opted for full reinstallation for every test in 1.1 since it's predictable and consistent.

ma...@google.com <ma...@google.com> Jul 11, 2022 04:24PM

Reassigned to ma...@google.com.

ap...@google.com <ap...@google.com> #6Jul 14, 2022 03:58AM

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

commit 78a3847fafbb5a810b60b27b1361400bf8dcfa58
Author: Marcello Albano <maralb@google.com>
Date: Mon Jul 11 14:07:07 2022

Added a new compilation mode `CompilationMode.Ignore` to skip profile reset and compilation.

Bug: 230453509
Test: ./gradlew :benchmark:integration-tests:macrobenchmark:cC -Pandroid.testInstrumentationRunnerArguments.class=androidx.benchmark.integration.macrobenchmark.CompilationModeTest

This change adds a new compilation mode `CompilationMode.Ignore` that will skip profile reset and compilation. This can be used to customize the benchmark loop.

Change-Id: Ibbcf8e8c627d227e0f290bddf464b67d9b6653bc
Relnote: Added a new compilation mode `CompilationMode.Ignore` to skip profile reset and compilation.

A benchmark/benchmark-macro/src/main/java/androidx/benchmark/macro/ExperimentalMacrobenchmarkApi.kt
M benchmark/benchmark-macro/src/main/java/androidx/benchmark/macro/CompilationMode.kt
M benchmark/benchmark-macro/src/androidTest/java/androidx/benchmark/macro/MacrobenchmarkTest.kt
A benchmark/integration-tests/macrobenchmark/src/androidTest/java/androidx/benchmark/integration/macrobenchmark/CompilationModeTest.kt
M benchmark/benchmark-macro/api/public_plus_experimental_current.txt

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

ma...@google.com <ma...@google.com> Jul 14, 2022 04:50PM

Marked as fixed.