New issue
Advanced search Search tips

Issue 727467 link

Starred by 1 user
Status: Fixed
Owner:
tkent@chromium.org
Closed: Jun 2017
Cc:
tkent@chromium.org
toyoshim@chromium.org
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

18%-34.7% regression in blink_perf.dom at 474443:474617

Project Member Reported by toyoshim@chromium.org, May 30 2017 
See the link to graphs below.
Project Member

Comment 3 by 42576172...@developer.gserviceaccount.com, May 30 2017

Cc: tkent@chromium.org
Owner: tkent@chromium.org

=== Auto-CCing suspected CL author tkent@chromium.org ===

Hi tkent@chromium.org, the bisect results pointed to your CL, please take a look at the
results.


=== BISECT JOB RESULTS ===
Perf regression found with culprit

Suspected Commit
  Author : tkent
  Commit : 9d5b3e6d930dd8052d1001b52348be1fb0098188
  Date   : Thu May 25 04:20:04 2017
  Subject: DOMTokenList should unify duplicated tokens.

Bisect Details
  Configuration: android_webview_arm64_aosp_perf_bisect
  Benchmark    : blink_perf.dom
  Metric       : modify-element-classname/modify-element-classname
  Change       : 29.97% | 920.292166667 -> 1196.0825

Revision             Result                  N
chromium@474543      920.292 +- 13.4952      6      good
chromium@474546      920.484 +- 38.4221      6      good
chromium@474547      1137.84 +- 80.6568      6      bad       <--
chromium@474548      1190.99 +- 35.1008      6      bad
chromium@474549      1201.95 +- 39.4385      6      bad
chromium@474555      1196.08 +- 39.311       6      bad

To Run This Test
  src/tools/perf/run_benchmark -v --browser=android-webview --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests blink_perf.dom

Debug Info
  https://chromeperf.appspot.com/buildbucket_job_status/8978200688702698912

Is this bisect wrong?
  https://chromeperf.appspot.com/bad_bisect?try_job_id=4973688241258496


| 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 Speed>Bisection.  Thank you!

Comment 4 by tkent@chromium.org, Jun 1 2017

Labels: -M-61 M-60
Status: Started (was: Untriaged)
Project Member

Comment 5 by bugdroid1@chromium.org, Jun 1 2017 

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

commit 0b9abe6d9be48f1f92f708d46afdac2811134102
Author: Kent Tamura <tkent@chromium.org>
Date: Thu Jun 01 03:25:08 2017

Optimize single class name parsing.

r474547 regressed PerformanceTests/DOM/modify-element-classname.html. This CL
fixes it by skipping hash lookup.

Before r474547: Average 148.5ms
After r474547:  Average 158.8ms
This CL:        Average 133.3ms

Bug:  727467 
Change-Id: I1f382514b06dd3a7209df67781f20e9415556372
Reviewed-on: https://chromium-review.googlesource.com/517616
Reviewed-by: Takayoshi Kochi <kochi@chromium.org>
Commit-Queue: Kent Tamura <tkent@chromium.org>
Cr-Commit-Position: refs/heads/master@{#476167}
[modify] https://crrev.com/0b9abe6d9be48f1f92f708d46afdac2811134102/third_party/WebKit/Source/core/BUILD.gn
[modify] https://crrev.com/0b9abe6d9be48f1f92f708d46afdac2811134102/third_party/WebKit/Source/core/dom/SpaceSplitString.cpp
[modify] https://crrev.com/0b9abe6d9be48f1f92f708d46afdac2811134102/third_party/WebKit/Source/core/dom/SpaceSplitString.h
[add] https://crrev.com/0b9abe6d9be48f1f92f708d46afdac2811134102/third_party/WebKit/Source/core/dom/SpaceSplitStringTest.cpp

Comment 6 by tkent@chromium.org, Jun 2 2017

Labels: Merge-Request-60
Status: Fixed (was: Started)
Project Member

Comment 7 by sheriffbot@chromium.org, Jun 2 2017

Labels: -Merge-Request-60 Hotlist-Merge-Approved Merge-Approved-60
Your change meets the bar and is auto-approved for M60. Please go ahead and merge the CL to branch 3112 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), josafat@(ChromeOS), bustamante@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 8 by bugdroid1@chromium.org, Jun 5 2017

Labels: -merge-approved-60 merge-merged-3112
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9ac19570ce602b0233b59029d7deb11c231032fe

commit 9ac19570ce602b0233b59029d7deb11c231032fe
Author: Kent Tamura <tkent@chromium.org>
Date: Mon Jun 05 00:38:24 2017

Merge "Optimize single class name parsing." to M60

r474547 regressed PerformanceTests/DOM/modify-element-classname.html. This CL
fixes it by skipping hash lookup.

Before r474547: Average 148.5ms
After r474547:  Average 158.8ms
This CL:        Average 133.3ms

TBR=tkent@chromium.org

(cherry picked from commit 0b9abe6d9be48f1f92f708d46afdac2811134102)

Bug:  727467 
Change-Id: I1f382514b06dd3a7209df67781f20e9415556372
Reviewed-on: https://chromium-review.googlesource.com/517616
Reviewed-by: Takayoshi Kochi <kochi@chromium.org>
Commit-Queue: Kent Tamura <tkent@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#476167}
Reviewed-on: https://chromium-review.googlesource.com/523463
Reviewed-by: Kent Tamura <tkent@chromium.org>
Cr-Commit-Position: refs/branch-heads/3112@{#135}
Cr-Branched-From: b6460e24cf59f429d69de255538d0fc7a425ccf9-refs/heads/master@{#474897}
[modify] https://crrev.com/9ac19570ce602b0233b59029d7deb11c231032fe/third_party/WebKit/Source/core/BUILD.gn
[modify] https://crrev.com/9ac19570ce602b0233b59029d7deb11c231032fe/third_party/WebKit/Source/core/dom/SpaceSplitString.cpp
[modify] https://crrev.com/9ac19570ce602b0233b59029d7deb11c231032fe/third_party/WebKit/Source/core/dom/SpaceSplitString.h
[add] https://crrev.com/9ac19570ce602b0233b59029d7deb11c231032fe/third_party/WebKit/Source/core/dom/SpaceSplitStringTest.cpp
►

Sign in to add a comment