IntervalStates time range is incorrect [213245198]

Fixed

Bug

Status Update

No update yet.

Description

cc...@google.com

created issue #1

Jan 5, 2022 01:06AM

JankStats#logFrameData does the following:

var frameStates =
            metricsStateHolder.state?.getIntervalStates(startTime, startTime + actualDuration)
            ?: emptyList()

Which is incorrect on API 24+, since actualDuration will include renderthread time that may overlap the next frame. Instead, that function should receive an additional arg to use here:

var frameStates =
            metricsStateHolder.state?.getIntervalStates(startTime, startTime + **uiThreadDuration**)
            ?: emptyList()

So it only looks at UI thread time.

Similarly, I'm not sure if startTime is the right value - I think it should be max(lastFrameUiEndTime, thisFrameIntendedVsync) so that in considers code that runs before the frame, but doesn't overlap with the previous frame.

Comments

ch...@google.com <ch...@google.com> #2Jan 6, 2022 10:45PM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit b90079595f33f58fece04026a97faa0d243acdb1
Author: Yuichi Araki <yaraki@google.com>
Date: Wed Sep 18 16:55:49 2019

Change the way to detect mismatch between POJO and query

This fixes cursor mismatch warnings with expandProjection.

Bug: 140759491
Test: QueryMethodProcessorTest
Change-Id: I7659002e5e0d1ef60fc1af2a625c0c36da0664d8

M room/compiler/src/main/kotlin/androidx/room/processor/QueryMethodProcessor.kt
M room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
M room/compiler/src/main/kotlin/androidx/room/solver/query/result/PojoRowAdapter.kt
M room/compiler/src/test/kotlin/androidx/room/processor/QueryMethodProcessorTest.kt
M room/compiler/src/test/kotlin/androidx/room/testing/TestProcessor.kt

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

https://goto.google.com/android-sha1/b90079595f33f58fece04026a97faa0d243acdb1

ch...@google.com <ch...@google.com> #3Mar 15, 2022 10:02PM

The following changes were cherrypicked through Coastguard (

https://android-build.googleplex.com/coastguard/)
Release Track: (

https://android-build.googleplex.com/coastguard/dashboard/6093116828680192/#/cherrypickHistory/all)
Created: 2019-10-14 17:23:42.610596+00:00
Changes: aosp/1123258

ch...@google.com <ch...@google.com> #4Mar 16, 2022 12:04AM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit bdde5a1a970ddc9007b28de4aa29d60ffa588f08
Author: Yigit Boyar <yboyar@google.com>
Date: Thu Apr 16 16:47:05 2020

Re-factor how errors are dismissed when query is re-written

This CL changes how we handle errors/warnings if query is
re-written.
There was a bug in expandProjection where we would report warnings
for things that Room already fixes automatically (

b/140759491

).
The solution to that problem (I7659002e5e0d1ef60fc1af2a625c0c36da0664d8)
solved it by deferring validating of columns until after re-write
decision is made. Unfortunately, this required changing PojoRowAdapter
to have a dummy mapping until it is validating, make it hard to use
as it does have a non-null mapping which is not useful.

This CL partially reverts that change and instead rely on the log
deferring logic we have in Context. This way, we don't need to break
the stability of PojoRowAdapter while still having the ability to
drop warnings that room fixes. This will also play nicer when we
have different query re-writing options that can use more information
about the query results.

Bug: 153387066
Bug: 140759491
Test: existing tests pass
Change-Id: I2ec967c763d33d7a3ff02c1a13c6953b460d1e5f

M room/compiler/src/main/kotlin/androidx/room/log/RLog.kt
M room/compiler/src/main/kotlin/androidx/room/processor/QueryMethodProcessor.kt
M room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
M room/compiler/src/main/kotlin/androidx/room/solver/query/result/PojoRowAdapter.kt

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

ap...@google.com <ap...@google.com> #5Mar 29, 2022 05:30PM

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

commit f069d601844f0b3f02ad5be2a491cc22d62456e2
Author: Chet Haase <chet@google.com>
Date: Tue Mar 15 16:51:14 2022

Refine startTime value

Two minor fixes to refine the time used to make the start time
of a frame:
- INTENDED_VSYNC_TIMESTAMP: On API 26+, this time more accurately
represents when the frame should have started than the previous
VSYNC_TIMESTAMP. They are usually the same, but in janky times,
the INTENDED time can capture more detail about when things should
have occurred, and how long they actually took.
- current vs previous frames: The use of INTENDED_VSYNC_TIMESTAMP
means that some frames can appear to overlap (because the system
tried to start one frame while the previous was still being processed).
This can lead to errors in state captures for those frames since they
appear in multiple frames for the same point in time. To avoid that
problem, the system now caches the previous "end" time of a frame,
and avoids using a start time less than that value.

Bug: 213245198
Test: Passed all JankStats tests on multiple API versions
Change-Id: I23fcc474f48619be9e0a996d1d0ac4d8e168e9e1

M metrics/metrics-performance/src/main/java/androidx/metrics/performance/JankStatsApi31Impl.kt
M metrics/metrics-performance/src/main/java/androidx/metrics/performance/JankStatsApi24Impl.kt
M metrics/metrics-performance/src/main/java/androidx/metrics/performance/JankStatsApi26Impl.kt

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

ch...@google.com <ch...@google.com> Apr 20, 2022 06:24PM

Marked as fixed.

Issue 213245198