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

Issue 610250 link

Starred by 0 users

Issue metadata

Status: Fixed
Owner:
NOT IN USE
Closed: May 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

11.5% regression in blink_perf.events at 392086:392119

Project Member Reported by alexclarke@chromium.org, May 9 2016

Issue description

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

Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?keys=agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgtInRuAoM


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

linux-release
Cc: msten...@opera.com
Owner: msten...@opera.com

=== Auto-CCing suspected CL author mstensho@opera.com ===

Hi mstensho@opera.com, the bisect results pointed to your CL below as possibly
causing a regression. Please have a look at this info and see whether
your CL be related.


===== BISECT JOB RESULTS =====
Status: completed


===== SUSPECTED CL(s) =====
Subject : Less duplicated code between nodeAtPoint() in LayoutBox and LayoutBlock.
Author  : mstensho
Commit description:
  
This is a preparatory patch for moving line/float-specific parts of
LayoutBlock::hitTestChildren() into LayoutBlockFlow.

BUG= 302024 

Review-Url: https://codereview.chromium.org/1957673002
Cr-Commit-Position: refs/heads/master@{#392116}
Commit  : bdc54b5ca7a098e251ff63d9ae1b38dcd5b467f1
Date    : Fri May 06 19:19:54 2016


===== TESTED REVISIONS =====
Revision         Mean      Std Dev      N  Good?
chromium@392085  0.961866  0.000351725  5  good
chromium@392102  0.971017  0.0143632    5  good
chromium@392111  0.96541   0.00138519   5  good
chromium@392115  0.956936  0.0054548    5  good
chromium@392116  0.858881  0.00120583   5  bad    <--
chromium@392117  0.858088  0.00116819   5  bad
chromium@392119  0.853746  0.00890178   5  bad

Bisect job ran on: linux_perf_bisect
Bug ID: 610250

Test Command: src/tools/perf/run_benchmark -v --browser=release --output-format=chartjson --upload-results --also-run-disabled-tests blink_perf.events
Test Metric: hit-test-lots-of-layers/hit-test-lots-of-layers
Relative Change: 11.24%
Score: 99.9

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/linux_perf_bisect/builds/6483
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9013151191285499936


Not what you expected? We'll investigate and get back to you!
  https://chromeperf.appspot.com/bad_bisect?try_job_id=5904390435635200

| O O | Visit http://www.chromium.org/developers/speed-infra/perf-bug-faq
|  X  | for more information addressing perf regression bugs. For feedback,
| / \ | file a bug with component Tests>AutoBisect.  Thank you!

Comment 3 by msten...@opera.com, May 9 2016

I observe the same.

0.6803137538892423 runs/s without the patch.
0.6063330673944943 runs/s with the patch.
Project Member

Comment 4 by bugdroid1@chromium.org, May 10 2016

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

commit 4f8e71034d840043dcf49bb51e8f911ec284f709
Author: mstensho <mstensho@opera.com>
Date: Tue May 10 16:44:16 2016

nodeAtPoint(): Perform the early-check EARLY.

Hit-testing overflow controls before checking if we need to do anything at all
affected performance, so just remove the LayoutBlock override for nodeAtPoint()
and hit test overflow controls there, but do so AFTER we have made sure that
the point is within bounds.

Fixes 15% performance regression for
PerformanceTests/Events/hit-test-lots-of-layers.html

BUG= 610250 

Review-Url: https://codereview.chromium.org/1960373002
Cr-Commit-Position: refs/heads/master@{#392626}

[modify] https://crrev.com/4f8e71034d840043dcf49bb51e8f911ec284f709/third_party/WebKit/Source/core/layout/LayoutBlock.cpp
[modify] https://crrev.com/4f8e71034d840043dcf49bb51e8f911ec284f709/third_party/WebKit/Source/core/layout/LayoutBlock.h
[modify] https://crrev.com/4f8e71034d840043dcf49bb51e8f911ec284f709/third_party/WebKit/Source/core/layout/LayoutBox.cpp
[modify] https://crrev.com/4f8e71034d840043dcf49bb51e8f911ec284f709/third_party/WebKit/Source/core/layout/LayoutBox.h

Comment 5 by msten...@opera.com, May 10 2016

Status: Fixed (was: Assigned)

Sign in to add a comment