Rework measureRepeated to be based on coroutines [389149423]

Assigned

Feature Request

Status Update

No update yet.

Description

cc...@google.com

created issue #1

Jan 10, 2025 10:38PM

Currently, there are several problems with the architecture of BenchmarkState/BenchmarkRule:

BenchmarkState has extremely complex state management due to attempting to expose a single keepRunning API
Trace capture happens within BenchmarkRule instead of measureRepeated (to allow it to support the Java API)
Yielding the main thread from within measureRepeatedOnMainThread() is awkward, and happens within the measurement loop
sleep after thermal throttling is detected doesn't yield, so can cause ANRs with measureRepeatedOnMainThread (b/311242861)
configuration (e.g. profiler, metric type, looping behavior) and data output (json, instrumentation results, traces) are distributed, and not easily simplified (ideally measureRepeated would return all of this data)

This bug tracks an effort to rewrite much of the top level control flow of BenchmarkRule / BenchmarkState to be based on coroutines, and address all the above problems.

Comments

ap...@google.com <ap...@google.com> #2Jan 16, 2025 09:18PM

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

cc...@google.com <cc...@google.com> #3Jan 16, 2025 10:06PM

Shorter term follow up tasks:

@Deprecate runWithTimingDisabled (not done in this CL to limit file changes)
Add more clarifying docs for new BenchmarkState implementation (in code comments as well as in the arch doc)
Remove BenchmarkRuleLegacy / BenchmarkStateLegacy (after confirmed no significant regressions)

Longer term:

Add replacements for removed BenchmarkState experimental APIs in MicrobenchmarkConfig (and probably need a MicrobenchmarkOutput)

ap...@google.com <ap...@google.com> #4Jan 28, 2025 06:41PM

Project: platform/frameworks/support
Branch: androidx-main
Author: Chris Craik <ccraik@google.com>
Link: https://android-review.googlesource.com/3460203

Deprecate runWithTimingDisabled in favor of runWithMeasurementDisabled

Expand for full commit details

Deprecate runWithTimingDisabled in favor of runWithMeasurementDisabled 
 
Bug: 389149423 
Fixes: 149979716 
Test: Existing / presubmit 
 
Opens up MicrobenchmarkScope since that's necessary to expose 
runWithMeasurementDisabled properly. Inline functions of a restricted 
superclass cannot be exposed by a subclass. 
 
Relnote: "Deprecate runWithTimingDisabled in favor of runWithMeasurementDisabled, which more clearly describes the behavior - all metrics are paused. Additionally, expose the MicrobenchmarkScope superclass since redeclaring the runWithMeasurementDisabled function to open access isn't possible, since it's inline." 
 
Change-Id: I9e23b0dfcff18f162ca0d2517734f3038870b59c

Files:

M benchmark/benchmark-common/api/current.txt
M benchmark/benchmark-common/api/restricted_current.txt
M benchmark/benchmark-common/src/main/java/androidx/benchmark/Microbenchmark.kt
M benchmark/benchmark-common/src/main/java/androidx/benchmark/MicrobenchmarkPhase.kt
M benchmark/benchmark-junit4/api/current.txt
M benchmark/benchmark-junit4/api/restricted_current.txt
M benchmark/benchmark-junit4/src/main/java/androidx/benchmark/junit4/BenchmarkRule.kt
M benchmark/benchmark-junit4/src/main/java/androidx/benchmark/junit4/BenchmarkRuleLegacy.kt
M benchmark/benchmark/src/androidTest/java/androidx/benchmark/benchmark/CpuEventCounterBenchmark.kt
M benchmark/benchmark/src/androidTest/java/androidx/benchmark/benchmark/PerfettoOverheadBenchmark.kt
M benchmark/benchmark/src/androidTest/java/androidx/benchmark/benchmark/PerfettoSdkOverheadBenchmark.kt
M benchmark/benchmark/src/androidTest/java/androidx/benchmark/benchmark/SampleCustomBenchmark.kt
M benchmark/benchmark/src/androidTest/java/androidx/benchmark/benchmark/SleepyBenchmark.kt
M benchmark/samples/src/main/java/androidx/benchmark/samples/BenchmarkRuleSample.kt
M compose/benchmark-utils/src/main/java/androidx/compose/testutils/benchmark/BenchmarkFirstExtensions.kt
M compose/benchmark-utils/src/main/java/androidx/compose/testutils/benchmark/BenchmarksExtensions.kt
M compose/foundation/foundation/benchmark/src/androidTest/java/androidx/compose/foundation/benchmark/ScrollerBenchmark.kt
M compose/foundation/foundation/benchmark/src/androidTest/java/androidx/compose/foundation/benchmark/focusable/FocusableInLazyListBenchmark.kt
M compose/foundation/foundation/benchmark/src/androidTest/java/androidx/compose/foundation/benchmark/lazy/LazyBenchmarkCommon.kt
M compose/foundation/foundation/benchmark/src/androidTest/java/androidx/compose/foundation/benchmark/lazy/LazySwitchingStatesBenchmark.kt
M compose/foundation/foundation/benchmark/src/androidTest/java/androidx/compose/foundation/benchmark/lazy/ReuseBenchmark.kt
M compose/foundation/foundation/benchmark/src/androidTest/java/androidx/compose/foundation/benchmark/text/empirical/baselines/StaticLayoutBaseline.kt
M compose/integration-tests/macrobenchmark/src/main/java/androidx/compose/integration/macrobenchmark/TrivialStartupBenchmark.kt
M compose/material/material-ripple/benchmark/src/androidTest/java/androidx/compose/material/ripple/benchmark/RippleBenchmark.kt
M compose/material/material/benchmark/src/androidTest/java/androidx/compose/material/benchmark/view/AndroidTextViewBenchmark.kt
M compose/material3/benchmark/src/androidTest/java/androidx/compose/material3/benchmark/MaterialBenchmarkExtensions.kt
M compose/runtime/runtime/compose-runtime-benchmark/src/androidTest/java/androidx/compose/runtime/benchmark/ComposeBenchmarkBase.kt
M compose/runtime/runtime/compose-runtime-benchmark/src/androidTest/java/androidx/compose/runtime/benchmark/KotlinCoroutinesBenchmark.kt
M compose/runtime/runtime/compose-runtime-benchmark/src/androidTest/java/androidx/compose/runtime/benchmark/SnapshotStateObserverBenchmark.kt
M compose/runtime/runtime/compose-runtime-benchmark/src/androidTest/java/androidx/compose/runtime/benchmark/collection/MutableVectorBenchmark.kt
M compose/ui/ui-text/benchmark/src/androidTest/java/androidx/compose/ui/text/benchmark/FrameworkTextLayoutBenchmark.kt
M compose/ui/ui-text/benchmark/src/androidTest/java/androidx/compose/ui/text/benchmark/NonLinearFontScalingBenchmark.kt
M compose/ui/ui-text/benchmark/src/androidTest/java/androidx/compose/ui/text/benchmark/ParagraphBenchmark.kt
M compose/ui/ui-text/benchmark/src/androidTest/java/androidx/compose/ui/text/benchmark/ParagraphWithLineHeightBenchmark.kt
M compose/ui/ui-text/benchmark/src/androidTest/java/androidx/compose/ui/text/benchmark/font/PlatformFontLookup.kt
M compose/ui/ui-text/benchmark/src/androidTest/java/androidx/compose/ui/text/benchmark/input/EditProcessorBenchmark.kt
M compose/ui/ui-text/benchmark/src/androidTest/java/androidx/compose/ui/text/benchmark/input/GapBufferBenchmark.kt
M compose/ui/ui/benchmark/src/androidTest/java/androidx/compose/ui/benchmark/BenchmarkModifierExtensions.kt
M compose/ui/ui/benchmark/src/androidTest/java/androidx/compose/ui/benchmark/LifecycleAwareWindowRecomposerBenchmark.kt
M compose/ui/ui/benchmark/src/androidTest/java/androidx/compose/ui/benchmark/NestedScrollingBenchmark.kt
M compose/ui/ui/benchmark/src/androidTest/java/androidx/compose/ui/benchmark/accessibility/TextAccessibilityBenchmark.kt
M compose/ui/ui/benchmark/src/androidTest/java/androidx/compose/ui/benchmark/accessibility/TextFieldAccessibilityBenchmark.kt
M compose/ui/ui/benchmark/src/androidTest/java/androidx/compose/ui/benchmark/focus/ParameterizedFocusBenchmark.kt
M compose/ui/ui/benchmark/src/androidTest/java/androidx/compose/ui/benchmark/spatial/RectListBenchmark.kt
M datastore/datastore-benchmark/src/androidTest/java/androidx/datastore/core/MultiProcessDataStoreTest.kt
M datastore/datastore-benchmark/src/androidTest/java/androidx/datastore/core/SingleProcessDatastoreTest.kt
M emoji2/emoji2-benchmark/src/androidTest/java/androidx/emoji2/benchmark/text/CachedEmojiCompatInitBenchmark.kt
M emoji2/emoji2-benchmark/src/androidTest/java/androidx/emoji2/benchmark/text/EmojiProcessBenchmark.kt
M profileinstaller/profileinstaller-benchmark/src/androidTest/java/androidx/profileinstaller/benchmark/ProfileInstallerTranscodeBenchmark.kt
M room/benchmark/src/androidTest/java/androidx/room/benchmark/AmbiguousColumnResolverBenchmark.kt
M wear/compose/compose-foundation/benchmark/src/androidTest/java/androidx/wear/compose/foundation/benchmark/ScalingLazyColumnBenchmark.kt
M wear/compose/compose-material/benchmark/src/androidTest/java/androidx/wear/compose/material/benchmark/PositionIndicatorBenchmark.kt
M wear/compose/compose-material/benchmark/src/androidTest/java/androidx/wear/compose/material/benchmark/SLCBasedBenchmarkHelper.kt
M work/work-benchmark/src/androidTest/java/androidx/work/benchmark/InitializeBenchmark.kt

Hash: f95ae3f3d30cb7486c27a0857856dbfd15920c3a
Date: Wed Jan 22 16:04:15 2025

gr...@google.com <gr...@google.com> #5Feb 11, 2025 11:39AM

Good news! Wear Compose has been updated to ab/12983551 in

https://critique.corp.google.com/cl/725560567 and this bug has been affected.

Issue 389149423

Description

Issue summary

Comments

ap...@google.com <ap...@google.com> #2Jan 16, 2025 09:18PM

cc...@google.com <cc...@google.com> #3Jan 16, 2025 10:06PM

ap...@google.com <ap...@google.com> #4Jan 28, 2025 06:41PM

gr...@google.com <gr...@google.com> #5Feb 11, 2025 11:39AM

Add comment

Issue metadata