Benchmark output in studio removes leading whitespace [227205461]

Fixed

Bug

Status Update

No update yet.

Description

cc...@google.com

created issue #1

Mar 28, 2022 05:30PM

This removes the left alignment benchmark does in its output to make multiple tests like up.

Android Studio Dolphin | 2021.3.1 Canary 7
Build #AI-213.6777.52.2113.8305692, built on March 15, 2022
Runtime version: 11.0.13+0-b1751.21-8125866 amd64
VM: OpenJDK 64-Bit Server VM by JetBrains s.r.o.
Linux 5.15.15-1rodete2-amd64
GC: G1 Young Generation, G1 Old Generation
Memory: 8192M
Cores: 16
Registry:
    external.system.auto.import.disabled=true
    ide.text.editor.with.preview.show.floating.toolbar=false


Current Desktop: X-Cinnamon

Mon Mar 28 2022 10:28:37 GMT-0700 (Pacific Daylight Time).png

111 KB

View

Download

Comments

ml...@google.com <ml...@google.com> #2Nov 8, 2022 12:36PM

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> #3Nov 8, 2022 07:19PM

+Yuki who may know more about where that wiring happens

I guess we could hack around this with a leading character, but we'd still need Studio to prune it for us if we didn't want to leave it there forever. We don't know on device which version of studio/agp we're running with, so we'd be stuck with it forever.

ml...@google.com <ml...@google.com> #4Nov 8, 2022 07:37PM

I was thinking we could potentially leave it in the output. I have a feeling when you have multiple benchmarks at once, it may even be more readable to see what results belong to which benchmark?

NavigationFramesBenchmark_startupNoComp
⊢  frameDurationCpuMs   P50    9.5,   P90   26.4,   P95   45.8,   P99  133.4
⊢      frameOverrunMs   P50   -6.5,   P90   25.6,   P95   79.1,   P99  118.0
⊢   Traces: Iteration 0 1 2 3 4 5

NavigationFramesBenchmark_startupPartial
⊢  frameDurationCpuMs   P50    9.5,   P90   26.4,   P95   45.8,   P99  133.4
⊢      frameOverrunMs   P50   -6.5,   P90   25.6,   P95   79.1,   P99  118.0
⊢   Traces: Iteration 0 1 2 3 4 5

But this is completely subjective .. if we can fix it, then let's do it! :)

hu...@google.com <hu...@google.com> #5Nov 9, 2022 10:50PM

I checked the AmInstrumentationParser and found that it trims the status value. This parser class is part of UTP core classes.

private fun storeCurrentValue() {
  val key = currentKey ?: return
  val statusValue = currentStatusValue
  if (statusValue != null) {
    getOrCreateCurrentTestRecord().storeStatus(key, statusValue.toString().trim())
    currentStatusValue = null
    return
  }
  val resultValue = currentResultValue
  if (resultValue != null) {
    bundle[key] = resultValue.toString().trim()
    currentResultValue = null
  }
}

We can update the parser however you need to wait for the new AGP version (and we need to assess any possible regressions). If you have workaround solutions in benchmark lib side, that would be much easier.

mz...@google.com <mz...@google.com> #6Nov 11, 2022 05:21PM

Tracking internally with b/258813488. OOO til 11/21, but can work on this then.

gi...@google.com <gi...@google.com> #7Nov 14, 2022 05:26PM

Assigned to gi...@google.com.

Chris could we left align the attribute names and do something like %20s or %-20s?

mz...@google.com <mz...@google.com> #8Nov 30, 2022 08:35PM

Whitespace is now preserved (internal bug b/258813488). Once Android Studio picks up the changelist this should be fixed accordingly.

is...@google.com <is...@google.com> Jun 9, 2023 07:42AM

Status: New

em...@google.com <em...@google.com> #9Sep 14, 2023 06:12PM

This is waiting for a UTP update in AGP.

hu...@google.com <hu...@google.com> Feb 7, 2024 12:27AM

Marked as fixed, reassigned to ya...@google.com.