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

Issue 866981 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Jul 28
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug-Regression



Sign in to add a comment

1.7%-66.7% regression in system_health.common_desktop at 576859:576986

Project Member Reported by sullivan@chromium.org, Jul 24

Issue description

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

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


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

Win 7 Nvidia GPU Perf
Win 7 Perf
linux-perf
mac-10_12_laptop_low_end-perf
mac-10_13_laptop_high_end-perf
Cc: uwyiming@google.com
Status: Assigned (was: Untriaged)
📍 Found a significant difference after 1 commit.
https://pinpoint-dot-chromeperf.appspot.com/job/11e3ab4da40000

Add a universal wait-for-page-ready heuristic to the Autofill Captured Sites Interactive Uitest. by uwyiming@google.com
https://chromium.googlesource.com/chromium/src/+/dd60b0b2896699f093771630d10c184e50279b94
0.06925 → 0.1003 (+0.03102)

Understanding performance regressions:
  http://g.co/ChromePerformanceRegressions
Cc: vmp...@chromium.org enne@chromium.org
Labels: -Pri-2 Pri-1
Owner: uwyiming@chromium.org
This turned out to be a bad bug because it causes two blink updates per frame. The issue is that DidCommitAndDrawFrame is causing SendCommitRequestToImplThreadIfNeeded to be called:
cc::ProxyMain::SendCommitRequestToImplThreadIfNeeded(cc::ProxyMain::CommitPipelineStage) + 64
cc::ProxyMain::SetNeedsAnimate() + 336
cc::LayerTreeHost::SetNeedsAnimate() + 63
cc::LayerTreeHost::QueueSwapPromise(std::__1::unique_ptr<cc::SwapPromise, std::__1::default_delete<
content::LayerTreeView::QueueSwapPromise(std::__1::unique_ptr<cc::SwapPromise, std::__1::default_delete
content::RenderWidget::QueueMessage(IPC::Message*, content::MessageDeliveryPolicy) + 851
content::RenderWidget::DidCommitAndDrawCompositorFrame() + 499
content::LayerTreeView::DidCommitAndDrawFrame() + 26
cc::LayerTreeHost::DidCommitAndDrawFrame() + 26
cc::ProxyMain::DidCommitAndDrawFrame() + 203

Yiming is going to remove the queue'd message for now and investigate a different approach.
@sullivan, can I remove Restrict-View-Google?
Labels: -Restrict-View-Google
Removed. It was on there because one of the new bots was accidentally marked internal-only (all this data should be public)
Project Member

Comment 7 by bugdroid1@chromium.org, Jul 27

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

commit f0d7951f253c73c2678c83af541481bb34276a40
Author: Yiming Zhou <uwyiming@google.com>
Date: Fri Jul 27 21:18:55 2018

Remove queuing the DidCommitAndDrawCompositorFrame message.

This change removes queuing the DidCommitAndDrawCompositorFrame message
when Chrome commits a paint update. Queuing the message causes an
additional blink update, resulting in a serious performance regression.

The original intent of queuing the DidCommitAndDrawCompositorFrame
message is to notify the WebContents when Chrome performs a paint, which
in turn helps Chrome's experimental capture sites automation test framework.
Since this performance regression is particularly large, and the absence
of this event only impacts the test framework, we decide to remove
queuing the message for now.

Bug:  866981 
Change-Id: I20e6fa6dbe1fd30f1529d6dda20e6208ae719bd1
Reviewed-on: https://chromium-review.googlesource.com/1153594
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578800}
[modify] https://crrev.com/f0d7951f253c73c2678c83af541481bb34276a40/content/renderer/render_widget.cc

Status: Fixed (was: Assigned)

Sign in to add a comment