Status Update
Comments
cc...@google.com <cc...@google.com> #2
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
dv...@gmail.com <dv...@gmail.com> #3
beyond datasource, I have found other problems:
-
InMemoryTrace(it's restricted to library scope , better to no strict it) is better than androidx.traceing.trace because of no binder call cost(it will have less cost when under heavy load, such made by stress-ng; test macrobenchmark without load usually useless in device maker company, device maker company always get this critical path more higher scheduler priorty.), however flush period for every 5s will cause trace_processor_shell ignore the InMemoryTraces which is not happened in last 5 sec. If we want to avoid that, when start trace_processor_shell , should pass in a --full-sort parameters to avoid that.
-
the buffer size PerfettoConfig use is too small, If there's heavy load , there are many sched traces that will cause data loss. maybe should shorten write file period and enlarge the buffer size. the other way is to filter out sched in atrace, however it will cause no thread name , If there is any TraceMetric's subclass depends on that , that will cause error.
cc...@google.com <cc...@google.com> #4
Uploaded a change to do the simpler version of this, which just allows top level customization of config:
A "proper" solution which lets Metrics contribute DataSources to be merged together is complex, as it requires central coordination of underlying buffer ids and sizes, at minimum. For now, we'll just let advanced users fully customize this.
We can see what feedback on the simpler approach is in 1.3, and see if there's still need for something more advanced.
cc...@google.com <cc...@google.com> #5
InMemoryTrace
My instinct is to avoid using InMemoryTrace in macrobenchmark entirely, as traces can be arbitrarily long, so the cost of --full-sort is likely not worth the benefit, at least in how macrobenchmark uses it. InMemoryTrace is most useful for microbenchmark - injecting trace events after the fact, at 0 cost during very sensitive measurement, and with limited trace durations.
You can consider using androidx.tracing:tracing-perfetto
if you want additional tracepoints, though this adds startup complexity and requires
Probably worth a separate bug if we want to discuss further.
ap...@google.com <ap...@google.com> #6
Branch: androidx-main
commit 5d5db95b1b823a1a174fbfc11fa6589c5988f378
Author: Chris Craik <ccraik@google.com>
Date: Thu Apr 11 15:04:05 2024
Enable passing custom PerfettoConfig to Macrobenchmark
Test: MacrobenchmarkTest
Test: ./gradlew benchmark:benchmark-macro:lintDebug
Fixes: 309841164
Bug: 304038384
Relnote: "Added experimental MacrobenchmarkRule#measureRepeated
variant which takes a custom PerfettoConfig for fully customized
Perfetto trace recording. Note that incorrectly configured configs may
cause built in Metric classes to fail."
Change-Id: Idfd3d0c071005f04f8be9975c6379e6095416775
M benchmark/benchmark-common/src/main/java/androidx/benchmark/perfetto/PerfettoConfig.kt
M benchmark/benchmark-macro-junit4/api/current.txt
M benchmark/benchmark-macro-junit4/api/restricted_current.txt
M benchmark/benchmark-macro-junit4/build.gradle
M benchmark/benchmark-macro-junit4/src/main/java/androidx/benchmark/macro/junit4/MacrobenchmarkRule.kt
M benchmark/benchmark-macro/src/androidTest/java/androidx/benchmark/macro/MacrobenchmarkTest.kt
M benchmark/benchmark-macro/src/main/java/androidx/benchmark/macro/Macrobenchmark.kt
Description
Component used: Metric Version used: latest Devices/Android versions reproduced on: Android 13, Oneplus 11
If this is a bug in the library, we would appreciate if you could attach:
When I add some new metric, I need add new Perfetto trace datasource to capture.
But there is no way to modify the PerfettoConfig class , default is PerfettonConfig.benchmark
Can you modify the macrobenchmark architecture , let Metric class has a changes to add datasource used in benchmarktest?