New issue
Advanced search Search tips

Issue 863926 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 20
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug

Blocking:
issue 760553



Sign in to add a comment

Values from TBMv2-version of frame_times does not exactly match on mobile

Project Member Reported by chiniforooshan@chromium.org, Jul 16

Issue description

Cc: bokan@chromium.org sadrul@chromium.org vmi...@chromium.org ericrk@chromium.org
I'm not sure who initially wrote the smoothness.frame_times metric and so I'm adding all smoothness/rendering benchmark owners. Sorry!

Question: I rewrote the frame_times metric in JS and now the old and new values are not identical on Android (they seem to agree on other platforms). When I compute the value of frame_times manually, according to my understanding of what it should be, from one of the traces, it matches the new (JS) version. Does anyone know why the python implementation gives a different number? Or who I should contact about this?

Comparison between the two values (Note that the frame time counts are different too, usually by 1-2 frames):
https://chromeperf.appspot.com/report?sid=3d1dac8445f8607dbf8c83bd3f54c4d9f457eb410ea13810016f13b7b864e061

The way I manually computed the metric:
1- Download a trace from one of the runs (click on the graph and select "View trace from this run").
2- Select all events under the single async event titled "SyntheticGestureController::running".
3- Click on "BenchmarkInstrumentation::DisplayRenderingStats", in the overview window at the bottom of the trace viewer.
4- It will show "CPU Self Time" statistics for "BenchmarkInstrumentation::DisplayRenderingStats". Click on "Start" to show timestamp statistics instead.
5- It will show avg, count, max, min, and std of timestamps for "BenchmarkInstrumentation::DisplayRenderingStats". Compute (max - min) / (count - 1).

Thank you!
Blocking: 760553
Does this issue repro locally? Is it possible to see what events each code is processing? Perhaps the python-version is filtering out some events?
Thanks for the suggestion; yeah I think if nothing obvious comes to mind I should find a test device and try to debug locally.

I don't think the python version is filtering out some events because the fream_times_count is higher than frame_times_tbmv2_count by 1-3 frames, not lower.
Does this happen only on android?
Yes, I've seen it on Android only.
Status: Assigned (was: Untriaged)
This bug has an owner, thus, it's been triaged. Changing status to "assigned".
Not completely. After that CL metrics match on desktop and some mobile bots (e.g. Pixel 2) but there is still a slight mismatch on Nexus 5 bots: 

https://chromeperf.appspot.com/report?sid=49d89d6125752419fbf130ca964fbfe22a7459f37903fd83ff6a866ae50ba391
Perhaps it is possible that in tbmv2, we are processing some events more than once? (see a similar issue I ran into here: https://chromium-review.googlesource.com/c/catapult/+/1161113)
Hmmm, but, the graph in #9 shows that event counts for TBMv2 version is LESS than counts for legacy version. Also, when I look at a trace from one of the telemetry runs, there is only one interaction record.

But, I'm curious to know, do you have an example in which segments contain or overlap each other, as claimed in the description of https://chromium-review.googlesource.com/c/catapult/+/1161113? My assumption was that interaction records do not overlap, unlike user model generated expectations which was used in the early version of the rendering metric.
Update: the cause is that Surface Flinger data is missing in the TBMv2 version. It was not reproducible in newer Android phones due to  crbug.com/881873 : because of that bug, both the legacy and TBMv2 versions were missing SF data and so were identical. 

The fix is under review: https://chromium-review.googlesource.com/c/catapult/+/1230554
Project Member

Comment 13 by bugdroid1@chromium.org, Sep 18

The following revision refers to this bug:
  https://chromium.googlesource.com/catapult/+/758dedc2a937ecb59455d62bdf1f941a7179d754

commit 758dedc2a937ecb59455d62bdf1f941a7179d754
Author: Ehsan Chiniforooshan <chiniforooshan@chromium.org>
Date: Tue Sep 18 19:49:50 2018

Telemetry: Fix SF events

Currently, Surface Flinger events that Telemetry collects
are turned into malformed trace events (there is no event
phase field). There is a special importer just to import
these events to the model.

In this CL, we make Telemetry generate valid trace data.
This way,

1- there is no need for an special importer,
2- the standard trace viewer can also load the result and
   show SF events, and
3- TBMv2 metrics can access SF data.  crbug.com/863926  is due
   to not having SF data in TBMv2.

Example trace after this CL:
https://drive.google.com/file/d/18Zcqw9tNBpfUUYvDVR0y1wQTmwOv3ud6

Bug:  chromium:863926 
Change-Id: I4f1e27cb07c916823528dc5bb9c2f22b13007fef
Reviewed-on: https://chromium-review.googlesource.com/1230554
Reviewed-by: Ben Hayden <benjhayden@chromium.org>
Reviewed-by: Juan Antonio Navarro Pérez <perezju@chromium.org>
Commit-Queue: Ehsan Chiniforooshan <chiniforooshan@chromium.org>

[modify] https://crrev.com/758dedc2a937ecb59455d62bdf1f941a7179d754/tracing/tracing/metrics/rendering/frame_time.html
[modify] https://crrev.com/758dedc2a937ecb59455d62bdf1f941a7179d754/telemetry/telemetry/web_perf/metrics/rendering_stats_unittest.py
[modify] https://crrev.com/758dedc2a937ecb59455d62bdf1f941a7179d754/telemetry/telemetry/web_perf/metrics/rendering_stats.py
[modify] https://crrev.com/758dedc2a937ecb59455d62bdf1f941a7179d754/tracing/tracing/trace_data/trace_data.py
[modify] https://crrev.com/758dedc2a937ecb59455d62bdf1f941a7179d754/telemetry/telemetry/timeline/model.py
[modify] https://crrev.com/758dedc2a937ecb59455d62bdf1f941a7179d754/tracing/tracing/model/helpers/chrome_model_helper.html
[modify] https://crrev.com/758dedc2a937ecb59455d62bdf1f941a7179d754/telemetry/telemetry/timeline/trace_event_importer.py
[delete] https://crrev.com/d2771a5b0b96e8f75024e07fbeba65221e261cd1/telemetry/telemetry/timeline/surface_flinger_importer.py
[modify] https://crrev.com/758dedc2a937ecb59455d62bdf1f941a7179d754/tracing/tracing/metrics/rendering/frame_time_test.html
[modify] https://crrev.com/758dedc2a937ecb59455d62bdf1f941a7179d754/telemetry/telemetry/internal/platform/android_platform_backend.py
[modify] https://crrev.com/758dedc2a937ecb59455d62bdf1f941a7179d754/telemetry/telemetry/internal/platform/tracing_agent/display_tracing_agent.py
[modify] https://crrev.com/758dedc2a937ecb59455d62bdf1f941a7179d754/telemetry/telemetry/web_perf/metrics/smoothness.py

Project Member

Comment 14 by bugdroid1@chromium.org, Sep 19

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/20136a68eadb1aaeda4252b590f0bcc047c04ebe

commit 20136a68eadb1aaeda4252b590f0bcc047c04ebe
Author: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Date: Wed Sep 19 18:59:44 2018

Roll src/third_party/catapult c968ea0b6538..92ef0386b8b6 (15 commits)

https://chromium.googlesource.com/catapult.git/+log/c968ea0b6538..92ef0386b8b6


git log c968ea0b6538..92ef0386b8b6 --date=short --no-merges --format='%ad %ae %s'
2018-09-19 dtu@chromium.org [pinpoint] Data migration basic error handling.
2018-09-19 dtu@chromium.org [pinpoint] Basic stats page.
2018-09-19 dtu@chromium.org [pinpoint] Update fields in Jobs table.
2018-09-19 cbruni@chromium.org results2.html debounce search
2018-09-19 simonhatch@chromium.org Dashboard - Fix alert and comparison_magnitude.
2018-09-19 simonhatch@chromium.org Dashboard - Replace last_alerted_revision with query
2018-09-19 slobodan@google.com Fixing frames in animations following response.
2018-09-19 pasko@chromium.org androidStartupMetric: remove the FCP
2018-09-18 dtu@chromium.org [pinpoint] Remove unused <job-chart-icon> import.
2018-09-18 achuith@chromium.org oobe: Catch WebSocketException instead.
2018-09-18 dtu@chromium.org [pinpoint] Remove list of differences and reformat arguments list.
2018-09-18 sadrul@chromium.org rendering: Remove some redundant mean_ metrics.
2018-09-18 sadrul@chromium.org rendering: Remove discrepancy metrics.
2018-09-18 chiniforooshan@chromium.org Telemetry: Fix SF events
2018-09-18 mattm@chromium.org netlog_viewer/README.md: Re-order with user-focused sections at top.


Created with:
  gclient setdep -r src/third_party/catapult@92ef0386b8b6

The AutoRoll server is located here: https://autoroll.skia.org/r/catapult-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.

CQ_INCLUDE_TRYBOTS=luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel

BUG=chromium:878390, chromium:868073 , chromium:883079 ,chromium:883715, chromium:886621 , chromium:879353 , chromium:884950 , chromium:884950 , chromium:863926 
TBR=sullivan@chromium.org

Change-Id: I9fa8e95d9aad524c032b0bf637dfc07926ce7795
Reviewed-on: https://chromium-review.googlesource.com/1234576
Reviewed-by: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Commit-Queue: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#592499}
[modify] https://crrev.com/20136a68eadb1aaeda4252b590f0bcc047c04ebe/DEPS

Sign in to add a comment