New issue
Advanced search Search tips

Issue 872599 link

Starred by 2 users

Issue metadata

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



Sign in to add a comment

17.9%-22.5% regression in blink_perf.layout at 581370:581414

Project Member Reported by toyoshim@chromium.org, Aug 9

Issue description

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

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


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

Android Nexus6 WebView Perf
Cc: pdr@chromium.org ericwilligers@chromium.org
Owner: pdr@chromium.org
Status: Assigned (was: Untriaged)
📍 Found significant differences after each of 2 commits.
https://pinpoint-dot-chromeperf.appspot.com/job/1196d402640000

Retire PaymentDetailsModifierData flag by ericwilligers@chromium.org
https://chromium.googlesource.com/chromium/src/+/af1cfb65aa121b157dfffd1575a36b7a1076e7c5
0.6151 → 0.6218 (+0.006763)

Remove lifecycle updates for preferred content size changes by pdr@chromium.org
https://chromium.googlesource.com/chromium/src/+/601fd07dc26269d097d6ef6bd90f1203d361a72c
0.62 → 0.5206 (-0.09939)

Understanding performance regressions:
  http://g.co/ChromePerformanceRegressions
Components: Blink>Paint
This is a real regression. Previously, we'd recompute preferred widths once-per-frame, but we now calculate it synchronously after every layout which can occur many times per frame. I thought clean layout implied clean preferred widths but this is not true because preferred widths are computed lazily and are typically not needed.
Cc: lanwei@chromium.org
 Issue 873149  has been merged into this issue.
📍 Job complete. See results below.
https://pinpoint-dot-chromeperf.appspot.com/job/168a9482640000
📍 Job complete. See results below.
https://pinpoint-dot-chromeperf.appspot.com/job/12d4786c640000
📍 Job complete. See results below.
https://pinpoint-dot-chromeperf.appspot.com/job/14df9088640000
📍 Job complete. See results below.
https://pinpoint-dot-chromeperf.appspot.com/job/14895764640000
📍 Job complete. See results below.
https://pinpoint-dot-chromeperf.appspot.com/job/15d4786c640000
Project Member

Comment 16 by bugdroid1@chromium.org, Aug 13

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

commit 33cd2bccd7dbd33dfddbe3fb4f4c8a6b1a7e4617
Author: Philip Rogers <pdr@chromium.org>
Date: Mon Aug 13 21:29:29 2018

Defer preferred size updates to after commit

https://crrev.com/581383 removed a preferred size timer and started
synchronously updating preferred sizes in DidUpdateMainFrameLayout.
This caused a performance regression because, even though layout is
up-to-date, the preferred contents size may need to be re-computed.
Script can cause many layouts per frame which was a performance
regression because many multiple preferred contents size updates are now
occurring per frame.

This patch adds a bool for if a layout occurred which may have changed
the preferred content size, and defers the update until after the
commit.

Bug:  872599 
Change-Id: Idfe1ba523b1ecfdf8f959bafc2e20ef6a52487bb
Reviewed-on: https://chromium-review.googlesource.com/1172059
Commit-Queue: Philip Rogers <pdr@chromium.org>
Reviewed-by: vmpstr <vmpstr@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582708}
[modify] https://crrev.com/33cd2bccd7dbd33dfddbe3fb4f4c8a6b1a7e4617/android_webview/renderer/aw_render_view_ext.cc
[modify] https://crrev.com/33cd2bccd7dbd33dfddbe3fb4f4c8a6b1a7e4617/android_webview/renderer/aw_render_view_ext.h
[modify] https://crrev.com/33cd2bccd7dbd33dfddbe3fb4f4c8a6b1a7e4617/content/renderer/render_view_impl.cc
[modify] https://crrev.com/33cd2bccd7dbd33dfddbe3fb4f4c8a6b1a7e4617/content/renderer/render_view_impl.h

Status: Fixed (was: Assigned)

Sign in to add a comment