IssueTracker

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

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

commit b15ef41ce16efc5c3b7d3dfe0d04157ffa474ff2
Author: Chris Craik <ccraik@google.com>
Date:   Tue Oct 08 15:06:18 2019

    Add tracing to benchmark
   
    Bug:142058671
    Test: tests in benchmark-benchmark, with systrace
   
    Change-Id: Ic7c35ad8c8b18e478ad2a19627df92e3241d8a0a

M       benchmark/common/api/1.0.0-rc01.txt
M       benchmark/common/api/current.txt
M       benchmark/common/api/public_plus_experimental_1.0.0-rc01.txt
M       benchmark/common/api/public_plus_experimental_current.txt
M       benchmark/common/api/restricted_1.0.0-rc01.txt
M       benchmark/common/api/restricted_current.txt
M       benchmark/common/src/main/java/androidx/benchmark/BenchmarkState.kt
A       benchmark/common/src/main/java/androidx/benchmark/TraceCompat.kt
M       benchmark/junit4/src/main/java/androidx/benchmark/junit4/BenchmarkRule.kt

https://android-review.googlesource.com/1136816
https://goto.google.com/android-sha1/b15ef41ce16efc5c3b7d3dfe0d04157ffa474ff2

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

commit 345f86d34f7e373f464c0d8185e392b067a2de4a
Author: Chris Craik <ccraik@google.com>
Date:   Wed Oct 09 17:37:17 2019

    Bump thread priority of benchmarks and JIT during benchmarks
   
    The JIT thread is so low priority that other parallel tasks can starve
    it, especially for the first few benchmarks when a process runs.
   
    The system can spin up significant background work right after install
    and/or instrumentation start, and on locked devices with only two big
    cores, there aren't enough CPUs to go around - warmup and benchmark
    both complete before relevant JIT is complete.
   
    Now, we bump the priority of both the benchmark and JIT thread.
    Tracing benchmarks show that the JIT thread goes much faster, which
    should significantly reduce the chance we capture results on unjitted
    code.
   
    This may also motivate us to use CPU affinity + locked small cores in
    the future, we can keep monitoring.
   
    Test: ./gradlew benchmark:b-c:cC
    Test: ./gradlew benchmark:b-b:cC
    Test: ./gradlew recyclerview:r-b:cC
   
    This CL also adds more logging, and unifies all logging under
    "benchmark" tag. This logging was very useful in discovering and
    diagnosing the priority problem, since it showed the edge cases where
    jit finished *during* the measure pass.
   
    Bug: 140773023
    Bug: 142058671
   
    Change-Id: If542e3cb8867165cf7b4688090ee534e68a23562

M       benchmark/common/src/androidTest/java/androidx/benchmark/BenchmarkStateTest.kt
M       benchmark/common/src/main/java/androidx/benchmark/BenchmarkState.kt
A       benchmark/common/src/main/java/androidx/benchmark/ThreadPriority.kt
M       benchmark/common/src/main/java/androidx/benchmark/WarmupManager.kt
M       benchmark/junit4/src/main/java/androidx/benchmark/junit4/BenchmarkRule.kt

https://android-review.googlesource.com/1138018
https://goto.google.com/android-sha1/345f86d34f7e373f464c0d8185e392b067a2de4a

As part of trying to reland Owen's looping arch change, I verified that it does significantly improve this problem, see attached .json files - specifically the measured numbers.

E.g. Parameterized benchmark, before (both variants):
                    "runs": [
                        18,
                        12,
                        12,
                        12,
                        12,
                    "runs": [
                        17,
                        5,
                        5,
                        6,
                        5,

After:
                    "runs": [
                        14,
                        11,
                        11,
                        11,
                        11,
                    "runs": [
                        4,
                        4,
                        4,
                        4,
                        4,


Or TrivialJavaBenchmark, Before:
                    "runs": [
                        24,
                        12,
                        12,
                        12,
                        12,
After:

                     runs": [
                        11,
                        11,
                        11,
                        11,
                        11,


Let's mark this fixed once the warmup rearch lands again.

In addition, we've lowered back down our measurements for our smallest (noop) benchmarks. Looks like making warmup and measurement more similar means we've fully jitted a lot more code. Since tiny benchmarks measure quickly, we were likely not giving the code in measurement codepaths time to jit.

I'd guess much of the remaining cost for the first loop is likely branch mispredictions for the loop early return itself (used only during measurement), but that only seems to only significantly affect the first benchmark (the first 'after' number above in ParameterizedBenchmark).