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

Issue 724099 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Buried. Ping if important.
Closed: Jun 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 2
Type: Bug-Regression



Sign in to add a comment

6.3% regression in dromaeo.domcoremodify at 472157:472250

Project Member Reported by tdres...@chromium.org, May 18 2017

Issue description

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

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


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

chromium-rel-win7-gpu-intel
Project Member

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

Cc: mkwst@chromium.org
Owner: mkwst@chromium.org

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

Hi mkwst@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 : mkwst
  Commit : 23cd806334edf349b4371e63f231bc1361fe0a08
  Date   : Tue May 16 21:00:16 2017
  Subject: Move `<script nonce>` hiding to `Element`.

Bisect Details
  Configuration: winx64intel_perf_bisect
  Benchmark    : dromaeo.domcoremodify
  Metric       : dom/dom
  Change       : 4.33% | 746.399358589 -> 714.091552617

Revision             Result                  N
chromium@472156      746.399 +- 13.747       6      good
chromium@472203      748.573 +- 11.2577      6      good
chromium@472209      746.447 +- 3.68373      6      good
chromium@472212      744.072 +- 15.0806      6      good
chromium@472214      748.642 +- 11.3499      6      good
chromium@472215      700.969 +- 10.0985      6      bad       <--
chromium@472227      710.342 +- 22.9156      6      bad
chromium@472250      714.092 +- 15.6424      6      bad

To Run This Test
  src/tools/perf/run_benchmark -v --browser=release_x64 --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests dromaeo.domcoremodify

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

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


| 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 mkwst@chromium.org, May 19 2017

Status: Assigned (was: Untriaged)
Looks legit. I'll work on it.
Project Member

Comment 5 by bugdroid1@chromium.org, May 26 2017

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

commit afc52f4ebf737fb545461fa2a5041d8ce08f5386
Author: Mike West <mkwst@chromium.org>
Date: Fri May 26 15:58:00 2017

Update internal '[[CryptographicNonce]]' slot when parsing attributes.

More discussion on https://github.com/whatwg/html/pull/2373 has lead to
a shift in the implementation from doing everything at insertion time to
updating the internal slot's value at attribute-parse time. This patch
updates both the tests and implementation.

It should also bring `dromaeo.domcoremodify` back up a bit from the drop
we experienced after landing the previous pass at this functionality.

Bug: 680419, 724099 
Change-Id: I93e7880c94889fb8cd04dec5c639fe52105b091a
Reviewed-on: https://chromium-review.googlesource.com/517064
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Commit-Queue: Mike West <mkwst@chromium.org>
Cr-Commit-Position: refs/heads/master@{#475014}
[modify] https://crrev.com/afc52f4ebf737fb545461fa2a5041d8ce08f5386/third_party/WebKit/LayoutTests/external/wpt/content-security-policy/nonce-hiding/script-nonces-hidden-meta.tentative.html
[modify] https://crrev.com/afc52f4ebf737fb545461fa2a5041d8ce08f5386/third_party/WebKit/LayoutTests/external/wpt/content-security-policy/nonce-hiding/script-nonces-hidden.tentative.html
[modify] https://crrev.com/afc52f4ebf737fb545461fa2a5041d8ce08f5386/third_party/WebKit/LayoutTests/external/wpt/content-security-policy/nonce-hiding/svgscript-nonces-hidden-meta.tentative.html
[modify] https://crrev.com/afc52f4ebf737fb545461fa2a5041d8ce08f5386/third_party/WebKit/LayoutTests/external/wpt/content-security-policy/nonce-hiding/svgscript-nonces-hidden.tentative.html
[modify] https://crrev.com/afc52f4ebf737fb545461fa2a5041d8ce08f5386/third_party/WebKit/LayoutTests/external/wpt/html/dom/reflection-metadata-expected.txt
[modify] https://crrev.com/afc52f4ebf737fb545461fa2a5041d8ce08f5386/third_party/WebKit/LayoutTests/external/wpt/html/dom/reflection-misc-expected.txt
[modify] https://crrev.com/afc52f4ebf737fb545461fa2a5041d8ce08f5386/third_party/WebKit/Source/core/html/HTMLElement.cpp

Comment 6 by mkwst@chromium.org, Jun 12 2017

Labels: Merge-Request-60
Status: Fixed (was: Assigned)
Recovered.
Project Member

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

Labels: -Merge-Request-60 Hotlist-Merge-Review Merge-Review-60
This bug requires manual review: M60 has already been promoted to the beta branch, so this requires manual review
Please contact the 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
Please apply appropriate OSs.  Thanks.

Comment 9 by mkwst@chromium.org, Jun 13 2017

Labels: OS-Android OS-Chrome OS-Linux OS-Mac OS-Windows
All Blink OSs. Setting flags accordingly. :)
Labels: -Merge-Review-60 Merge-Rejected-60
Is this related to shipping the new nonce behavior tracked in  issue 731752  which I recently rejected?  I'm going to assume so, and since I've requested we wait for 61 for that behavior, I'll request we wait with this patch for 61 as well.

If this perf regression is present even without the new nonce behavior shipping, please re-apply the merge request label and we can discuss.

Sign in to add a comment