New issue
Advanced search Search tips

Issue 729033 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 747504



Sign in to add a comment

Filter Expected Queueing Time of background tabs

Project Member Reported by maxlg@chromium.org, Jun 2 2017

Issue description

Currently, we record and report expected queueing time of both foreground and background tabs. However, as users do not interact with background tabs, the queueing time of the background renderers doesn't impact input latency. Therefore, when estimating queueing time of a renderer, we should filter out those of the background tabs.
 
Cc: rbyers@chromium.org
Components: Speed>Metrics
Labels: Performance-Responsiveness
Owner: maxlg@chromium.org

Comment 2 by maxlg@chromium.org, Jun 2 2017

Description: Show this description
On Android I think there are (at least) two separate cases to worry about:
 - Background tabs (w/ Chrome running in foreground)
 - Chrome running in background

This is even more complicated on desktop given that there can be multiple partially overlapping windows.  Perhaps we just want to build on the same signal as page visibility?  I.e. if we're telling the webpage that they're currently not visible, we shouldn't be tracking responsiveness metrics.
Cc: skyos...@chromium.org
Sami - any thoughts on what signal we should use here?
We currently have two separate concepts of backgrounding in the scheduler (which we are unifying):

1. RendererSchedulerImpl::OnRendererBackgrounded[1] -- called when *all* tabs in a given renderer are hidden.

2. WebFrameSchedulerImpl::SetFrameVisible[2] -- called when a given tab/frame becomes hidden.

I think for EQT you want #1 because all tabs are sharing the main thread and therefore responsiveness is important for the user.

[1] https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.cc?l=554&gs=cpp%253Ablink%253A%253Ascheduler%253A%253Aclass-RendererSchedulerImpl%253A%253AOnRendererBackgrounded()%2540chromium%252F..%252F..%252Fthird_party%252FWebKit%252FSource%252Fplatform%252Fscheduler%252Frenderer%252Frenderer_scheduler_impl.cc%257Cdef&gsn=OnRendererBackgrounded&ct=xref_usages

[2] https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/scheduler/renderer/web_frame_scheduler_impl.cc?rcl=ce64e673564b8a2b0912c07567e96c97aa4dca06&l=109
Thanks Sami - yeah, that sounds correct. Once we can attribute tasks to frames, we'll want to re-evaluate.

Comment 7 by maxlg@chromium.org, Jun 5 2017

Great, I will do that. Thank y'all!
Cc: altimin@chromium.org
Re #5: we're _trying_ to make them more coherent, but there are challenges associated with merging them completely [1]. For the short-to-middle term we'll have to stick with having two concepts.

Exposing frame attribution for a task queue should be relatively straightforward, I'll try to get to it next week.

[1] 
Process priority logic lives in browser/ and it might be a bad idea to expose more information to a renderer https://cs.chromium.org/chromium/src/content/browser/renderer_host/render_process_host_impl.cc?type=cs&q=ChildProcessMsg_SetProcessBackgrounded+package:%5Echromium$&l=3303
Owner: npm@chromium.org
Status: Assigned (was: Available)
Blocking: 747504
Project Member

Comment 11 by bugdroid1@chromium.org, Aug 3 2017

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

commit 6be7ecdcae29068ea6ac0f60e9ab880d67c13156
Author: Nicolas Pena <npm@chromium.org>
Date: Thu Aug 03 17:15:54 2017

EQT: Skip steps when renderer is backgrounded

In this CL, the QueueingTimeEstimator only uses visible steps for
calculating Expected Queueing Time. A step is considered visible if
the renderer was never backgrounded throughout the step (excluding
possible changes at exact start or end of step). A window may thus be
made up of steps that are not contiguous in time. The objective of this
CL is to start reducing the massive concentration of the UMA stats for
Expected Queueing Time on the 0-1 ms bucket, because that is not very
useful and not a good indicator of latency.

Bug:  chromium:729033 
Change-Id: I3cfd4d3268d0f2858fc77de25c08d35a7a1cfc36
Reviewed-on: https://chromium-review.googlesource.com/587588
Commit-Queue: Nicolás Peña <npm@chromium.org>
Reviewed-by: Timothy Dresser <tdresser@chromium.org>
Reviewed-by: Alexander Timin <altimin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491773}
[modify] https://crrev.com/6be7ecdcae29068ea6ac0f60e9ab880d67c13156/third_party/WebKit/Source/platform/scheduler/base/queueing_time_estimator.cc
[modify] https://crrev.com/6be7ecdcae29068ea6ac0f60e9ab880d67c13156/third_party/WebKit/Source/platform/scheduler/base/queueing_time_estimator.h
[modify] https://crrev.com/6be7ecdcae29068ea6ac0f60e9ab880d67c13156/third_party/WebKit/Source/platform/scheduler/base/queueing_time_estimator_unittest.cc
[modify] https://crrev.com/6be7ecdcae29068ea6ac0f60e9ab880d67c13156/third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.cc
[modify] https://crrev.com/6be7ecdcae29068ea6ac0f60e9ab880d67c13156/third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.h

Comment 12 by npm@chromium.org, Aug 4 2017

Status: Fixed (was: Assigned)

Sign in to add a comment