New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 903155 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 12
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

14.1%-31.8% regression in rendering.desktop/thread_total_all_cpu_time_per_frame at 604536:604558

Project Member Reported by alexclarke@chromium.org, Nov 8

Issue description

See the link to graphs below.
 
All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=903155

(For debugging:) Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?sid=5c943c1e19bbe1bc844e5baf2e095bc9cf29234d89e563967282d4357612a30c


Bot(s) for this bug's original alert(s):

mac-10_12_laptop_low_end-perf
mac-10_13_laptop_high_end-perf

rendering.desktop - Benchmark documentation link:
  https://bit.ly/rendering-benchmarks
📍 Couldn't reproduce a difference.
https://pinpoint-dot-chromeperf.appspot.com/job/1735f7cbe40000
Cc: sahel@chromium.org oysteine@google.com
Owner: sahel@chromium.org
Status: Assigned (was: Untriaged)
📍 Found significant differences after each of 2 commits.
https://pinpoint-dot-chromeperf.appspot.com/job/14c517a0140000

Perfetto: Fix for worker threads not finalizing proto packets by oysteine@google.com
https://chromium.googlesource.com/chromium/src/+/d420ce980291dbdee3d8a68594afbbfe5b75fbb4
tasks_per_frame_total_all: 74.57 → 88.29 (+13.72)

Register fling scheduler on GSB instead of GFS. by sahel@chromium.org
https://chromium.googlesource.com/chromium/src/+/0e58f2caab6a230fdb0f2b1993a44c2f4546c2f5
tasks_per_frame_total_all: 90.78 → 102.2 (+11.39)

Understanding performance regressions:
  http://g.co/ChromePerformanceRegressions

Benchmark documentation link:
  https://bit.ly/rendering-benchmarks
Project Member

Comment 6 by bugdroid1@chromium.org, Dec 7

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

commit 0996ec8f771b7c0cc28aa56b201abf3ace2d4176
Author: Sahel Sharify <sahel@chromium.org>
Date: Fri Dec 07 17:19:41 2018

Revert "Register fling scheduler on GSB instead of GFS."

Revert "Register fling scheduler on GSB instead of GFS." and
Revert "Fixed fling regression from registering observer on GSB instead of GFS."

This reverts the following two commits:
0b48b62c4392e7cd594d9fbe8684e93abc72aa1b
0e58f2caab6a230fdb0f2b1993a44c2f4546c2f5

The reason for reverting is that registering fling scheduler observer on GSB
instead of GFS does not fix the one frame jank at the beginning of the
fling. This change also caused some performance regressions and touchpad
scrolling regression on eve.

Bug: 882907, 911975 , 911334 ,903161,903156, 903155 
Change-Id: I6aba2c0314eeab616fcb1c18c448c0b3c075d4e6
Reviewed-on: https://chromium-review.googlesource.com/c/1363893
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Commit-Queue: Sahel Sharify <sahel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#614730}
[modify] https://crrev.com/0996ec8f771b7c0cc28aa56b201abf3ace2d4176/content/browser/renderer_host/input/fling_controller.cc
[modify] https://crrev.com/0996ec8f771b7c0cc28aa56b201abf3ace2d4176/content/browser/renderer_host/input/fling_controller.h
[modify] https://crrev.com/0996ec8f771b7c0cc28aa56b201abf3ace2d4176/content/browser/renderer_host/input/fling_controller_unittest.cc
[modify] https://crrev.com/0996ec8f771b7c0cc28aa56b201abf3ace2d4176/content/browser/renderer_host/input/fling_scheduler.cc
[modify] https://crrev.com/0996ec8f771b7c0cc28aa56b201abf3ace2d4176/content/browser/renderer_host/input/fling_scheduler.h
[modify] https://crrev.com/0996ec8f771b7c0cc28aa56b201abf3ace2d4176/content/browser/renderer_host/input/fling_scheduler_android.cc
[modify] https://crrev.com/0996ec8f771b7c0cc28aa56b201abf3ace2d4176/content/browser/renderer_host/input/fling_scheduler_android.h
[modify] https://crrev.com/0996ec8f771b7c0cc28aa56b201abf3ace2d4176/content/browser/renderer_host/input/fling_scheduler_unittest.cc
[modify] https://crrev.com/0996ec8f771b7c0cc28aa56b201abf3ace2d4176/content/browser/renderer_host/input/gesture_event_queue.cc
[modify] https://crrev.com/0996ec8f771b7c0cc28aa56b201abf3ace2d4176/content/browser/renderer_host/input/gesture_event_queue_unittest.cc
[modify] https://crrev.com/0996ec8f771b7c0cc28aa56b201abf3ace2d4176/content/browser/renderer_host/input/mock_input_router_client.h

Project Member

Comment 7 by bugdroid1@chromium.org, Dec 11

Labels: merge-merged-3626
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3cc06e54913c18ff3a45e9dda912bc7b5e5d5e20

commit 3cc06e54913c18ff3a45e9dda912bc7b5e5d5e20
Author: Sahel Sharify <sahel@chromium.org>
Date: Tue Dec 11 21:28:04 2018

Revert "Register fling scheduler on GSB instead of GFS."

Revert "Register fling scheduler on GSB instead of GFS." and
Revert "Fixed fling regression from registering observer on GSB instead of GFS."

This reverts the following two commits:
0b48b62c4392e7cd594d9fbe8684e93abc72aa1b
0e58f2caab6a230fdb0f2b1993a44c2f4546c2f5

The reason for reverting is that registering fling scheduler observer on GSB
instead of GFS does not fix the one frame jank at the beginning of the
fling. This change also caused some performance regressions and touchpad
scrolling regression on eve.

Bug: 882907, 911975 , 911334 ,903161,903156, 903155 
Change-Id: I6aba2c0314eeab616fcb1c18c448c0b3c075d4e6
Reviewed-on: https://chromium-review.googlesource.com/c/1363893
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Commit-Queue: Sahel Sharify <sahel@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#614730}(cherry picked from commit 0996ec8f771b7c0cc28aa56b201abf3ace2d4176)
Reviewed-on: https://chromium-review.googlesource.com/c/1372714
Reviewed-by: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#262}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
[modify] https://crrev.com/3cc06e54913c18ff3a45e9dda912bc7b5e5d5e20/content/browser/renderer_host/input/fling_controller.cc
[modify] https://crrev.com/3cc06e54913c18ff3a45e9dda912bc7b5e5d5e20/content/browser/renderer_host/input/fling_controller.h
[modify] https://crrev.com/3cc06e54913c18ff3a45e9dda912bc7b5e5d5e20/content/browser/renderer_host/input/fling_controller_unittest.cc
[modify] https://crrev.com/3cc06e54913c18ff3a45e9dda912bc7b5e5d5e20/content/browser/renderer_host/input/fling_scheduler.cc
[modify] https://crrev.com/3cc06e54913c18ff3a45e9dda912bc7b5e5d5e20/content/browser/renderer_host/input/fling_scheduler.h
[modify] https://crrev.com/3cc06e54913c18ff3a45e9dda912bc7b5e5d5e20/content/browser/renderer_host/input/fling_scheduler_android.cc
[modify] https://crrev.com/3cc06e54913c18ff3a45e9dda912bc7b5e5d5e20/content/browser/renderer_host/input/fling_scheduler_android.h
[modify] https://crrev.com/3cc06e54913c18ff3a45e9dda912bc7b5e5d5e20/content/browser/renderer_host/input/fling_scheduler_unittest.cc
[modify] https://crrev.com/3cc06e54913c18ff3a45e9dda912bc7b5e5d5e20/content/browser/renderer_host/input/gesture_event_queue.cc
[modify] https://crrev.com/3cc06e54913c18ff3a45e9dda912bc7b5e5d5e20/content/browser/renderer_host/input/gesture_event_queue_unittest.cc
[modify] https://crrev.com/3cc06e54913c18ff3a45e9dda912bc7b5e5d5e20/content/browser/renderer_host/input/mock_input_router_client.h

Owner: oysteine@google.com
I reverted "Register fling scheduler on GSB instead of GFS" and the graph does not show recovery. Assigning it to the owner of the second pin pointed cl:
oysteine@ PTAL
Status: Fixed (was: Assigned)
Ah yep that looks like another alert from the perfetto switch. Recovered with http://crrev.com/614540
Labels: CommitLog-Audit-Violation Merge-Without-Approval
Here's a summary of the rules that were executed: 
 - OnlyMergeApprovedChange: Rule Failed -- Revision 3cc06e54913c18ff3a45e9dda912bc7b5e5d5e20 was merged to refs/branch-heads/3626 branch with no merge approval from a TPM! 
Please explain why this change was merged to the branch!
 - AcknowledgeMerge: Notification Required -- 
Labels: Merge-Merged-72-3626
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/3cc06e54913c18ff3a45e9dda912bc7b5e5d5e20

Commit: 3cc06e54913c18ff3a45e9dda912bc7b5e5d5e20
Author: sahel@chromium.org
Commiter: bokan@chromium.org
Date: 2018-12-11 21:28:04 +0000 UTC

Revert "Register fling scheduler on GSB instead of GFS."

Revert "Register fling scheduler on GSB instead of GFS." and
Revert "Fixed fling regression from registering observer on GSB instead of GFS."

This reverts the following two commits:
0b48b62c4392e7cd594d9fbe8684e93abc72aa1b
0e58f2caab6a230fdb0f2b1993a44c2f4546c2f5

The reason for reverting is that registering fling scheduler observer on GSB
instead of GFS does not fix the one frame jank at the beginning of the
fling. This change also caused some performance regressions and touchpad
scrolling regression on eve.

Bug: 882907, 911975 , 911334 ,903161,903156, 903155 
Change-Id: I6aba2c0314eeab616fcb1c18c448c0b3c075d4e6
Reviewed-on: https://chromium-review.googlesource.com/c/1363893
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Commit-Queue: Sahel Sharify <sahel@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#614730}(cherry picked from commit 0996ec8f771b7c0cc28aa56b201abf3ace2d4176)
Reviewed-on: https://chromium-review.googlesource.com/c/1372714
Reviewed-by: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#262}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}

Sign in to add a comment