Benchmarks - Regression between Merge "Add onRectChanged modifier and corresponding callback support to rect tracking" into androidx-main and Merge "Preemptively suppress deprecation for to be deprecated KotlinMultiplatformAndroidTarget" into androidx-main [372994338]

Fixed

Bug

Status Update

No update yet.

Description

pe...@skia-infra-public.iam.gserviceaccount.com

created issue on behalf of an...@google.com #1

Oct 13, 2024 03:40AM

Perf Regression (High) found, matching 3 tracked metrics from benchmarks.

To triage this regression, see the guide at go/androidx-bench-triage.

Test classes affected:

 androidx.compose.foundation.benchmark.SimpleComponentImplementationBenchmark

Test methods affected:

 component_modifiersOnly_layout
 component_redrawOnly_layout
 component_twoLayoutNodes_layout

Devices affected:

 sargo

API Level:

 API_31_S

Comments

lp...@google.com <lp...@google.com> #2Oct 14, 2024 01:05PM

Reassigned to le...@google.com.

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

le...@google.com <le...@google.com> #3Oct 14, 2024 07:52PM

Investigating locally, I see a ~22% regression. It appears that ~18% of this regression is a result of adding recalculateWindowPosition() on this line: https://cs.android.com/androidx/platform/frameworks/support/+/androidx-main:compose/ui/ui/src/androidMain/kotlin/androidx/compose/ui/platform/AndroidComposeView.android.kt;l=1492?q=androidcomposeview

I thought that this method was fairly cheap and cached, but it does not appear to be. I will look into mitigating this.

ap...@google.com <ap...@google.com> #4Oct 17, 2024 04:11AM

Marked as fixed.

Project: platform/frameworks/support
Branch: androidx-main
Author: Leland Richardson <lelandr@google.com>
Link: https://android-review.googlesource.com/3309717

Add onRectChanged modifier and corresponding callback support to rect tracking

Expand for full commit details

Add onRectChanged modifier and corresponding callback support to rect tracking

Fixes: 372765423
Fixes: 372757007
Fixes: 372994338

Relnote:"""
Added Modifier.onRectChanged API which is an API that allows one to subscribe to the root/window/screen-relative position and size of a
LayoutNode. The API solves many use cases that the existing onGloballyPositioned modifier does, however it does so with much less
overhead and the API comes with facilities to debounce and throttle the callback per what one's use case demands.
"""

Change-Id: Id28c7c6c52ce3b18c9ac8c83cde48111a312cfc9

Files:

M compose/ui/ui/api/current.txt
M compose/ui/ui/api/restricted_current.txt
A compose/ui/ui/src/androidInstrumentedTest/kotlin/androidx/compose/ui/layout/OnGlobalRectChangedTest.kt
M compose/ui/ui/src/androidInstrumentedTest/kotlin/androidx/compose/ui/layout/OnGloballyPositionedTest.kt
A compose/ui/ui/src/androidMain/kotlin/androidx/compose/ui/Actual.android.kt
M compose/ui/ui/src/androidMain/kotlin/androidx/compose/ui/platform/AndroidComposeView.android.kt
M compose/ui/ui/src/androidUnitTest/kotlin/androidx/compose/ui/spatial/RectListTest.kt
A compose/ui/ui/src/androidUnitTest/kotlin/androidx/compose/ui/spatial/ThrottledCallbacksTest.kt
M compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/Expect.kt
A compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/layout/OnRectChangedModifier.kt
M compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/node/MeasureAndLayoutDelegate.kt
A compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/spatial/RectInfo.kt
M compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/spatial/RectList.kt
M compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/spatial/RectManager.kt
A compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/spatial/ThrottledCallbacks.kt
M compose/ui/ui/src/commonStubsMain/kotlin/androidx/compose/ui/Actual.commonStubs.kt

Hash: ce33454f34929182e1109f6254144d390f27e7c8
Date: Wed Oct 16 13:18:34 2024

pr...@google.com <pr...@google.com> #5Oct 30, 2024 05:37PM

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

androidx.compose.ui:ui:1.8.0-alpha05
androidx.compose.ui:ui-android:1.8.0-alpha05
androidx.compose.ui:ui-jvmstubs:1.8.0-alpha05
androidx.compose.ui:ui-linuxx64stubs:1.8.0-alpha05

Issue 372994338

Description

Issue summary

Comments

lp...@google.com <lp...@google.com> #2Oct 14, 2024 01:05PM

le...@google.com <le...@google.com> #3Oct 14, 2024 07:52PM

ap...@google.com <ap...@google.com> #4Oct 17, 2024 04:11AM

pr...@google.com <pr...@google.com> #5Oct 30, 2024 05:37PM

Add comment

Issue metadata