Issue metadata
Sign in to add a comment
|
1646% regression in rendering.mobile/tasks_per_frame_total_all at 603410:603426 |
||||||||||||||||||||||
Issue descriptionSee the link to graphs below.
,
Nov 1
📍 Pinpoint job started. https://pinpoint-dot-chromeperf.appspot.com/job/140cd675e40000
,
Nov 2
📍 Found a significant difference after 1 commit. https://pinpoint-dot-chromeperf.appspot.com/job/140cd675e40000 Start a field trial for Isolated code cache by mythria@chromium.org https://chromium.googlesource.com/chromium/src/+/142ae7f8b7a7426ca426524a9593a39cc3e4f9ac tasks_per_frame_total_all: 4604 → 2.036e+04 (+1.575e+04) Understanding performance regressions: http://g.co/ChromePerformanceRegressions Benchmark documentation link: https://bit.ly/rendering-benchmarks
,
Nov 5
I am not sure about what exactly what this metric is measuring. Looking at the graphs, it seems this metric was always bimodal with data points in the range of ~16000 and ~700 and this change somehow impacted the timing and is now always at the higher points? This change could have increased the number of I/O tasks and IPCs because we now fetch the code caches explicitly and not along with the resource from the Http cache. Adding metric owners to see if this is an important metric we care about. I tried looking at important metrics listed in https://bit.ly/rendering-benchmarks (mean_frame_time, mean_input_event_latency) but there is no new data for them. Are there any other metrics I should look at?
,
Nov 5
📍 Pinpoint job started. https://pinpoint-dot-chromeperf.appspot.com/job/15c8eae9e40000
,
Nov 5
I found a regression on input_event_latency (I guess a replacement for mean_input_event_latency?) metric and started a bisect job. Even this metric was noisy before my change landed.
,
Nov 5
I looked at the traces and I am familiar with this part of the code, but I suspect it is related to some kind of timing. In traces where the number of tasks and the input_event_latency is less, there are fewer but longer (~100ms) tasks that perform (LocalFrameView::performLayout) as opposed to other traces where there are more but shorter (~5ms) tasks that correspond to (RenderWidgetInputHandler::OnHandleInputEvent). Could it be because of the timing of the start of the scroll action? vmiura@, sadrul@ any ideas?
,
Nov 5
Looking at the various breakdowns (link [1]): . It doesn't look like there are many changes in tasks_per_frame metrics for renderer_main, renderer_compositor, display_compositor, IO, or GPU. . tasks_perf_frame metrics for browser does go up a little, and quite a bit for 'other'. I think this accounts for the overall regression in tasks_per_frame_total_all metric. However, looking at the pinpoint result [2], there seems to be a fair amount of regression in all of these metrics. Looking at the trace from that run [3], it looks like there's a task in the renderer main-thread starting at ~12,255ms, which runs for over 1 second (~1533.217ms). This seems to align with the touch-start event, which would explain the regression in the input_event_latency metric (before the CL, that long task seems to happen before touch-start happens). The alert didn't trigger for this metric, but it's worth keeping an eye on. +nzolghadr@ FYI. [1] https://chromeperf.appspot.com/report?sid=1122f0754f420094a75375f07d002045b3848d0b1fd67e9b08a1ed27ff31622f&start_rev=601118&end_rev=605213 [2] https://storage.cloud.google.com/results2-public/140cd675e40000.html [3] https://console.developers.google.com/m/cloudstorage/b/chrome-telemetry-output/o/google_docs_2018_2018-11-01_15-10-54_1318.html
,
Nov 5
+chiniforooshan@ The trace from pinpoint without the change is in [4]. It looks to me like both before and after the change, the tasks_per_frame_browser should be fairly similar. Looking more closely, it looks like with the change, there's just very few draws happening: pipeline:draw is reported 9 times with the change, vs. 142 times without the change. And that does affect the tasks_per_frame_ metrics. The same with pipeline:begin_frame_transport metric, which means the client is receiving only a small number of begin-frames. [4] https://console.developers.google.com/m/cloudstorage/b/chrome-telemetry-output/o/google_docs_2018_2018-11-01_17-27-56_42915.html
,
Nov 5
mythria@ can you try running the benchmark locally on clank [1] and see if you can reproduce the problem? [1] https://github.com/catapult-project/catapult/blob/master/telemetry/docs/run_benchmarks_locally.md
,
Nov 6
📍 Found a significant difference after 1 commit. https://pinpoint-dot-chromeperf.appspot.com/job/15c8eae9e40000 Start a field trial for Isolated code cache by mythria@chromium.org https://chromium.googlesource.com/chromium/src/+/142ae7f8b7a7426ca426524a9593a39cc3e4f9ac input_event_latency: 628.8 → No values Understanding performance regressions: http://g.co/ChromePerformanceRegressions Benchmark documentation link: https://bit.ly/rendering-benchmarks
,
Nov 6
📍 Pinpoint job started. https://pinpoint-dot-chromeperf.appspot.com/job/13973723e40000
,
Nov 6
📍 Found a significant difference after 1 commit. https://pinpoint-dot-chromeperf.appspot.com/job/13973723e40000 Perfetto: Fix for worker threads not finalizing proto packets by oysteine@google.com https://chromium.googlesource.com/chromium/src/+/d420ce980291dbdee3d8a68594afbbfe5b75fbb4 tasks_per_frame_other: 32.43 → 62.27 (+29.84) Understanding performance regressions: http://g.co/ChromePerformanceRegressions Benchmark documentation link: https://bit.ly/rendering-benchmarks
,
Nov 6
I also noticed a smaller regression on the desktop story in the similar range and started a bisect. I am not sure that is related to this though. If it is important may be we could create a bug and investigate that separately. I tried to reproduce this locally on a Nexus 6 device but there is no clear regression. I ran it few times, but there is no clear regression. Infact it shows a slight improvement (within the noise range) with my patch (results [1]). I don't have a nexus 5 at the moment, I will try to run it on a Nexus 5 as well. [1] https://mythria.users.x20web.corp.google.com/www/results_render_google_docs.html
,
Nov 6
A regression of tasks_per_frame_total_all is expected by the Perfetto change (it swaps out the tracing backend, so this does not regress when tracing is not enabled), though normally like 10-20%. Looking at the graph this particular metric was highly bimodal/noisy before and stabilized a lot after, which is interesting.
,
Nov 7
Thanks! Then I suspect it is some secondary effect making it stable and not directly related to my change. I also can't reproduce the regression locally. Are we interested in looking into why it is more stable now or can we just ignore this?
,
Dec 4
Marking it as won't fix. As said in comment #16, I think it's just a matter of timing. Please feel free to reopen if anyone thinks we need to look into it.
,
Dec 4
|
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by 42576172...@developer.gserviceaccount.com
, Nov 1