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

For #2:
Command issuing and buffer swapping occur on the RenderThread. The break between these threads happens during the SYNC operation, when we copy the displaylist commands over to the RenderThread.
And yes, all of these info on API24 and forward is much more dependable than pre-24, because we really didn't have anything before FrameMetrics except the sheer delta between draws. And it becomes even more accurate on API31 with the addition of GPU time and the DEADLINE metric in FrameMetrics, which allows us to calculate the overrun metric for JankStats.

For #1, and the bug overall:
This is an interesting request that led me down a rathole trying to figure out where some of these numbers actually come from, because it is not at all obvious.
And it's also not obvious how to 'fix' the problem, or provide what you are asking for.
For one thing, the TOTAL_DURATION reported by FrameMetrics on API24 is significantly different than that reported in API31, because the latter includes data from the GPU, which we did not have on prior releases.  So while we could (and arguably should, given some of the stuff below) report this duration, it could be misleading if developers attempt to compare these same values across releases. Meanwhile, "cpuDuration" always means the same thing, whether on API24 or API31, so it is at least more accurate and consistent, if perhaps not all of the data you want and that you are currently getting from FrameMetrics.

What complicates this slightly further is that the data we report *should* get you what you are currently getting from FrameMetrics.TOTAL_DURATION... except that it's off by a little bit. That is, although the docs claim otherwise (leading to a docs fix that is currently in the works), you cannot simply add all of the other FrameMetrics durations (as we do to calculate cpuDuration) to get TOTAL_DURATION. Instead, TOTAL_DURATION is literally the time at which the frame was done (which includes GPU processing on API31, but not on prior releases) minus the time at which the frame started (VSYNC). This is *mostly* the same as cpuDuration on API24-30, but there is a slight variation due to overhead in syncing the displaylist to the RenderThread which is not accounted for, thus cpuDuration is generally slightly smaller than FrameMetrics.TOTAL_DURATION.
Also, note that some of the phases in a frame happen concurrently, overlapping to some extent. This is especially true wrt GPU-vs-CPU operations, thus TOTAL_DURATION will never (especially in 31+) equal the sum of the other durations because some phases happen in parallel.

Pondering what to do here. We could expose an additional FrameMetric to account for that additional delay (and thus be able to more accurately calculate the total duration, modulo the concurrency mentioned above). But this would only be available in later releases, making it irrelevant for pre-31 cpuDuration calculations.
We might, therefore, want to provide the totalDuration, as requested, though that comes with the complication mentioned above where it will be significantly different between pre- and post-31 releases (which you must already have experienced using FrameMetrics directly, right?)

Anyway, working on this, but just to let you know why it's not a simple fix. It's really more of a decision of how best to proceed that will be the least confusing for people using this API across releases.

Fix in progress, with the following changes:

1) frameDurationCpuNanos
This value is now being calculated more accurately on all API versions (24-31). It is equivalent to FrameMetrics.TOTAL_DURATION prior to API 31, and equivalent to TOTAL_DURATION - GPU + SWAP (because SWAP is double-counted for CPU and GPU in FrameMetrics) on API31.
This new calculation accounts for the fact that there is one piece of data missing from FrameMetrics, which is the amount of time waiting for RenderThread to become available. This is accounted for in TOTAL_DURATION, but is not in the other metrics, so you cannot actually calculate the total time spent in the CPU from the other metrics (thus the previous calculation of frameDurationCpuNanos was off by that amount).

2) frameDurationTotalNanos
As requested, we are exposing the 'total' duration for a frame from JankStats.
*However*
As noted in #3 above, the concept of "total" is so very different between the various API versions (specifically, pre-24, 24-30, and 31+) that it makes it problematic exposing it as a property across all of those versions since one cannot compare values across those releases, making consistency of data impossible.
Instead, we expose this new metric only on api 31 (in FrameDataApi31), since that is the first version in which 'total' is accurate, representing both CPU and GPU durations.

For the purposes of using JankStats instead of FrameMetrics across releases, you should be able to use frameDurationCpuNanos to replace FrameMetrics.TOTAL_DURATION on API versions 24-30, then use frameDurationTotalNanos on version 31+. I believe that should give you the functionality and numbers you used to get through FrameMetrics.

Project: platform/frameworks/support
Branch: androidx-main

commit 1de6215c6bd9e887e3d94556e9ac55cfb7b8c797
Author: Chet Haase <chet@google.com>
Date:   Fri Aug 26 16:56:05 2022

    Fix cpuDuration calculation and expose totalDuration
   
    A missing metric (the time taken to queue the data sync to RenderThread)
    in FrameMetrics made the cpuDuration data in JankStats less accurate than
    it could be, and made it harder to determine the equivalent of
    "totalDuration" in comparison to data one could get from FrameMetrics.
    Further complicating the problem was the overlap of SWAP duration
    in the calculation of both GPU and CPU durations on API31, making it
    even trickier to align all of these similar-yet-different metrics.
   
    There was a request (see the bug) to expose totalDuration for JankStats
    overall, but this is problematic since it is not possible to expose it
    consistently across different releases (on 31 it includes GPU, on 24 it
    does not, and on 16 it has even less data behind it). Instead of
    exposing it overall, we now expose it just in API31, in the
    FrameDataApi31 structure, where iut has all of the data it needs to
    provide an accurate measure of total duration.
   
    Bug: 243694893
    Test: Ran JankStats on API24 and 31
    RelNote: cpuDuration now more accurate, also new totalDuration on API31
    Change-Id: I59ce8c67f06a168f96893375c8aeca5516a55d81

M       metrics/metrics-performance/src/main/java/androidx/metrics/performance/JankStatsApi31Impl.kt
M       metrics/metrics-performance/api/public_plus_experimental_current.txt
M       metrics/metrics-performance/api/restricted_current.txt
M       metrics/metrics-performance/src/main/java/androidx/metrics/performance/FrameDataApi31.kt
M       metrics/metrics-performance/src/main/java/androidx/metrics/performance/JankStatsApi24Impl.kt
M       metrics/metrics-performance/src/androidTest/java/androidx/metrics/performance/test/JankStatsTest.kt
M       metrics/metrics-performance/api/current.txt

https://android-review.googlesource.com/2199726

Thanks for the quick fix!!! We believe the take away is the following:

> use frameDurationCpuNanos to replace FrameMetrics.TOTAL_DURATION on API versions 24-30, then use frameDurationTotalNanos on version 31+

I do have a follow-up question: while looking at the following trace (https://developer.android.com/static/studio/images/profile/jank_detection-frame-lifecycle-threads.png), I am wondering if "the sheer delta between draws in main thread" is a good rough representation of how much the user perceived as a frame's rendering time or the delta still misses some execution time on the render thread in parallel to the main thread.

I don't know off-hand from looking at that trace (since it's just a big blank area), but yeah, we don't have visibility pre-FrameMetrics in where the time was going. The perceived time between draws is still a decent overall metric, because that will end up being visible to the user (jank == longer delays between draws). So it's not like a huge delay in GPU rendering (for example) would end up being masked by the fact that we couldn't detect it due to the lack of FrameMetrics data. I'm not certain it would show up on that exact frame, however; I wonder if some GPU delays would show up on later frames, when that later frame went to send data to the GPU and had to wait to send it down, or was able to send it, but it just queued up waiting for the GPU to finish doing all of those earlier operations. Regardless, it's all decent data (I believe) in terms of knowing when there are general frame performance problems, it's just a little harder to diagnose where in the pipeline those problems occurred.

That's great to hear since we are taking a high level experimental approach to test frame render optimizations or investigate regressions. We often look at the aggregated frame drop or render data between the experimental groups. The "draw interval" measurement so far fits our needs and is indeed quite sensitive to the changes too.