New issue
Advanced search Search tips

Issue 601832 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

13% regression in blink_perf.bindings at 385685:385697

Project Member Reported by rsch...@chromium.org, Apr 8 2016

Issue description

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

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


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

android-nexus5
Submitted some more bisects.
Project Member

Comment 3 by 42576172...@developer.gserviceaccount.com, Apr 11 2016


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


=== Bisection aborted ===
The bisect was aborted because The metric values for the initial "good" and "bad" revisions do not represent a clear regression.
Please contact the the team (see below) if you believe this is in error.

=== Warnings ===
The following warnings were raised by the bisect job:

 * Bisect failed to reproduce the regression with enough confidence.

===== TESTED REVISIONS =====
Revision                Mean Value  Std. Dev.   Num Values  Good?
chromium@385696         1418.375    22.437636   12          good
chromium@385709         1424.925    28.700709   8           bad

Bisect job ran on: android_nexus9_perf_bisect
Bug ID: 601832

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --also-run-disabled-tests blink_perf.pywebsocket
Test Metric: XHR-receive-blob-worker-sync-verify/XHR-receive-blob-worker-sync-verify
Relative Change: 0.80%
Score: 0

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus9_perf_bisect/builds/1731
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9015659608006726352


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

| 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!
The regression on blink_perf.pywebsocket will be handled at  bug 602322 .
Kicked off another bisect with a wider range.
Project Member

Comment 6 by 42576172...@developer.gserviceaccount.com, Apr 20 2016

Cc: peria@chromium.org
Owner: peria@chromium.org

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

Hi peria@chromium.org, 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 : Add a new performance benchmark test for postMessage
Author  : peria
Commit description:
  
BUG= 148757 
TEST=tools/perf/run_benchmark run blink_perf.bindings

Review URL: https://codereview.chromium.org/1871463002

Cr-Commit-Position: refs/heads/master@{#385719}
Commit  : f4ff9e2d8d11f1c4d2225c37dddc4c755dbd2c32
Date    : Thu Apr 07 12:16:06 2016


===== TESTED REVISIONS =====
Revision                Mean Value  Std. Dev.   Num Values  Good?
chromium@385679         148.928949  0.576653    5           good
chromium@385710         154.567437  1.001259    5           good
chromium@385718         155.125466  0.811563    5           good
chromium@385719         118.45793   17.283549   5           bad         <-
chromium@385720         120.320844  16.650326   5           bad
chromium@385722         114.840866  14.047914   5           bad
chromium@385725         119.647145  17.747206   5           bad
chromium@385740         114.63802   17.659341   5           bad

Bisect job ran on: android_s5_perf_bisect
Bug ID: 601832

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --also-run-disabled-tests blink_perf.bindings
Test Metric: typed-array-construct-from-array/typed-array-construct-from-array
Relative Change: 23.03%
Score: 99.0

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_s5_perf_bisect/builds/613
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9014850022958913952


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

| 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 7 by peria@chromium.org, Apr 21 2016

Cc: -peria@chromium.org
Owner: ----
Status: Available (was: Assigned)
My change did not touch any C++/Python files, so it could not change performance.
Added file is out of the regressed test.
Cc: haraken@chromium.org peria@chromium.org bashi@chromium.org yukishiino@chromium.org
Owner: yukishiino@chromium.org
Summary: 13% regression in blink_perf.bindings at 385685:385697 (was: 1.3% regression in page_cycler.typical_25 at 385685:385697)
+peria, and blink_perf.bindings owners yukishiino, bashi, haraken

Apologies, peria, this bug was super confusing as it had some unrelated alerts grouped in:

* There was a memory regression in a page cycler that didn't show clearly in any individual pages, I removed it from the bug.
* There was a less than 10% regression in blink_perf.pywebsocket, I removed it as the description of that benchmark says to ignore regressions under 10%.

That leaves two regressions on blink_perf.bindings on two different devices. The bisect identified your CL pretty clearly as regressing the runtime of this test. But as you said, your CL doesn't affect chromium. It adds a new test to blink_perf.bindings.

The way the test is run is that all the pages in blink_perf.bindings are run together, both on the perf waterfall and the bisect. The fact that your addition of a new page changed the value of a different page makes me a little worried that there is a problem with the independence of the pages in the benchmarks. I'm going to kick off a second bisect to see if this reproduces, but since the numbers are pretth clear on the bisect in #6, assigning to yukishiino to investigate.
Project Member

Comment 10 by 42576172...@developer.gserviceaccount.com, Apr 21 2016

Owner: peria@chromium.org

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

Hi peria@chromium.org, 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 : Add performance tests for serialization in BlinkBinding
Author  : peria
Commit description:
  
This CL add performance tests to measure time to serialize following items.
In this CL, I added four type specific tests to factor out
what can be the bottleneck of serializations.
- a long string : serialize a large size native instance
- a long array : serialize an array with many small elements
- a nested array : serialize many arrays in chain
- a map : serialize a map whose keys are not ignorable

BUG= 148757 
TEST=tools/perf/run_benchmark run blink_perf.bindings

Review URL: https://codereview.chromium.org/1866083003

Cr-Commit-Position: refs/heads/master@{#385703}
Commit  : ae459e084f702f116959a296e30cc6550ea84d24
Date    : Thu Apr 07 10:01:47 2016


===== TESTED REVISIONS =====
Revision                Mean Value  Std. Dev.   Num Values  Good?
chromium@385679         147.580211  7.033334    12          good
chromium@385691         149.960192  1.212338    5           good
chromium@385697         150.636476  1.364363    5           good
chromium@385700         150.971242  2.008089    5           good
chromium@385702         151.937196  2.141277    5           good
chromium@385703         146.193368  1.634417    8           bad         <-

Bisect job ran on: android_s5_perf_bisect
Bug ID: 601832

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --also-run-disabled-tests blink_perf.bindings
Test Metric: typed-array-construct-from-array/typed-array-construct-from-array
Relative Change: 2.06%
Score: 99.5

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_s5_perf_bisect/builds/614
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9014761186103449664


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

| 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!
Second bisect confirms what I said in #8: Adding a new test changes the performance of an existing test.

Comment 12 by peria@chromium.org, Apr 22 2016

Status: Assigned (was: Available)
Hmm, it maybe related to GC handling.  I'll look into this.
Thank you for the clarification.
The new test is queing 100,000 events in the message queue, right?
I'm not good at this area, but if the message queue were implemented with the OS's event system, it may affect all the system, including other tests running on the same machine.

If we had no other ideas, i'd be worth trying to reduce the number of messages.  Do we really need such huge messages in the queue?  Another option is to queue a message when it arrives.
    A: B.postMessage(...);
    B: onmessage() { A.postMessage(...); }
    A: onmessage() { B.postMessage(...); }
    B: onmessage() { A.postMessage(...); }
    ...
In this way, we don't need to queue such huge number of messages at a time.

Project Member

Comment 14 by bugdroid1@chromium.org, Apr 25 2016

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

commit e3fe1b6186a589a8f5d020b2e4d8eaee8fe48ddd
Author: peria <peria@chromium.org>
Date: Mon Apr 25 08:17:13 2016

Fix the behavior of post-message.html

Before this CL, the test sent 10k messages and finished.
In this style, we had two issues;
1. it did not measure performance of postMessage
2. it remained many message tasks in pool, and regressed following tests.

This CL makes it to wait all messages' responses arrives,
and fixes the issues.

BUG= 601832 

Review URL: https://codereview.chromium.org/1921503002

Cr-Commit-Position: refs/heads/master@{#389430}

[modify] https://crrev.com/e3fe1b6186a589a8f5d020b2e4d8eaee8fe48ddd/third_party/WebKit/PerformanceTests/Bindings/post-message.html

Project Member

Comment 15 by bugdroid1@chromium.org, Apr 25 2016

Labels: merge-merged-2716
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e3fe1b6186a589a8f5d020b2e4d8eaee8fe48ddd

commit e3fe1b6186a589a8f5d020b2e4d8eaee8fe48ddd
Author: peria <peria@chromium.org>
Date: Mon Apr 25 08:17:13 2016

Fix the behavior of post-message.html

Before this CL, the test sent 10k messages and finished.
In this style, we had two issues;
1. it did not measure performance of postMessage
2. it remained many message tasks in pool, and regressed following tests.

This CL makes it to wait all messages' responses arrives,
and fixes the issues.

BUG= 601832 

Review URL: https://codereview.chromium.org/1921503002

Cr-Commit-Position: refs/heads/master@{#389430}

[modify] https://crrev.com/e3fe1b6186a589a8f5d020b2e4d8eaee8fe48ddd/third_party/WebKit/PerformanceTests/Bindings/post-message.html

Comment 16 by peria@chromium.org, Apr 30 2016

Status: Fixed (was: Assigned)

Sign in to add a comment