New issue
Advanced search Search tips

Issue 646139 link

Starred by 1 user
Status: Fixed
Owner:
u...@chromium.org
Closed: Sep 2016
Cc:
u...@chromium.org
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

1.6% regression in jetstream at 416696:416735

Project Member Reported by rsch...@chromium.org, Sep 12 2016 
See the link to graphs below.

Comment 1 by rsch...@chromium.org, Sep 12 2016 

All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=646139

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


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

linux-release
Project Member

Comment 3 by 42576172...@developer.gserviceaccount.com, Sep 13 2016

Cc: juncai@chromium.org
Owner: juncai@chromium.org

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

Hi juncai@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 : Check if BubbleReference weak pointer is valid before using it in the chooser
Author  : juncai
Commit description:
  
Since BubbleReference is a base::WeakPtr<BubbleController>, before
using it, needs to check if it is valid. This CL added code to do it.

BUG= 642748 

Review-Url: https://codereview.chromium.org/2309563002
Cr-Commit-Position: refs/heads/master@{#416698}
Commit  : 76f6b1648624d9fe6ce48b7d5996e3c2d41ec710
Date    : Tue Sep 06 20:26:27 2016


===== TESTED REVISIONS =====
Revision         Mean     Std Dev   N   Good?
chromium@416695  221.389  0.978528  18  good
chromium@416697  221.056  0.802366  18  good
chromium@416698  220.417  0.668558  12  bad    <--
chromium@416700  219.6    1.34164   5   bad
chromium@416705  220.222  1.21537   18  bad
chromium@416715  221.0    0.953463  12  bad
chromium@416735  219.6    0.894427  5   bad

Bisect job ran on: linux_perf_bisect
Bug ID: 646139

Test Command: src/tools/perf/run_benchmark -v --browser=release --output-format=chartjson --upload-results --also-run-disabled-tests jetstream
Test Metric: Score/Score
Relative Change: 1.17%
Score: 99.5

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


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

| 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 4 by rsch...@chromium.org, Sep 13 2016

Cc: -juncai@chromium.org
Owner: rsch...@chromium.org
Nah, I don't agree with the bisect. Let's try that again.
Project Member

Comment 6 by 42576172...@developer.gserviceaccount.com, Sep 13 2016

Cc: u...@chromium.org
Owner: u...@chromium.org

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

Hi ulan@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 : [heap] Refactor incremental marking step.
Author  : ulan
Commit description:
  
This patch
- extracts the logic of keeping track of allocated bytes
  from the actual incremental marking step.
- replaces OldSpaceStep with a check for incremental marking start.
- removes the force_marking parameter of AdvanceIncrementalMarking.

BUG= chromium:616434 
LOG=NO

Review-Url: https://codereview.chromium.org/2304123003
Cr-Commit-Position: refs/heads/master@{#39213}
Commit  : eca8a5ebbdab6b3276fca3abead893fd721336a0
Date    : Tue Sep 06 15:29:23 2016


===== TESTED REVISIONS =====
Revision                       Mean     Std Dev   N   Good?
chromium@416695                221.667  1.02899   18  good
chromium@416715                221.056  1.10997   18  good
chromium@416716                221.875  1.12599   8   good
chromium@416716,v8@eca8a5ebbd  219.75   1.38873   8   bad    <--
chromium@416716,v8@5dd940082b  219.75   1.16496   8   bad
chromium@416716,v8@b28b7e1328  219.625  1.18773   8   bad
chromium@416717                219.417  0.996205  12  bad
chromium@416718                219.417  0.996205  12  bad
chromium@416720                219.0    0.92582   8   bad
chromium@416725                218.75   0.707107  8   bad
chromium@416735                219.833  1.2673    12  bad

Bisect job ran on: linux_perf_bisect
Bug ID: 646139

Test Command: src/tools/perf/run_benchmark -v --browser=release --output-format=chartjson --upload-results --also-run-disabled-tests jetstream
Test Metric: Score/Score
Relative Change: 1.08%
Score: 99.5

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


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

| 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 u...@chromium.org, Sep 13 2016

Status: Started (was: Assigned)
My CL regresses Jetstream/splay due to GC timing.

Comment 8 by sullivan@chromium.org, Sep 23 2016 

Ulan, are you still working on this?

Comment 9 by u...@chromium.org, Sep 26 2016 

Yes, I have a patch in review, should land this week.
Project Member

Comment 10 by bugdroid1@chromium.org, Sep 28 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/1beb89f24cb01663843a7519921e7caf0910c760

commit 1beb89f24cb01663843a7519921e7caf0910c760
Author: ulan <ulan@chromium.org>
Date: Wed Sep 28 13:27:44 2016

[heap] New heuristics for incremental marking step size.

This patch simplifies code for speeding up marking and
removes write barrier counter.

The step size is now computed based in two parts:
- bytes to mark in order to keep up with allocation,
- bytes to mark in order to make progress.

BUG= chromium:616434 ,  chromium:646139 , chromium:644819
LOG=NO

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

[modify] https://crrev.com/1beb89f24cb01663843a7519921e7caf0910c760/src/arm/code-stubs-arm.cc
[modify] https://crrev.com/1beb89f24cb01663843a7519921e7caf0910c760/src/arm64/code-stubs-arm64.cc
[modify] https://crrev.com/1beb89f24cb01663843a7519921e7caf0910c760/src/heap/heap.cc
[modify] https://crrev.com/1beb89f24cb01663843a7519921e7caf0910c760/src/heap/heap.h
[modify] https://crrev.com/1beb89f24cb01663843a7519921e7caf0910c760/src/heap/incremental-marking.cc
[modify] https://crrev.com/1beb89f24cb01663843a7519921e7caf0910c760/src/heap/incremental-marking.h
[modify] https://crrev.com/1beb89f24cb01663843a7519921e7caf0910c760/src/ia32/code-stubs-ia32.cc
[modify] https://crrev.com/1beb89f24cb01663843a7519921e7caf0910c760/src/x64/code-stubs-x64.cc
[modify] https://crrev.com/1beb89f24cb01663843a7519921e7caf0910c760/test/cctest/heap/test-heap.cc
Project Member

Comment 11 by bugdroid1@chromium.org, Sep 28 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/1beb89f24cb01663843a7519921e7caf0910c760

commit 1beb89f24cb01663843a7519921e7caf0910c760
Author: ulan <ulan@chromium.org>
Date: Wed Sep 28 13:27:44 2016

[heap] New heuristics for incremental marking step size.

This patch simplifies code for speeding up marking and
removes write barrier counter.

The step size is now computed based in two parts:
- bytes to mark in order to keep up with allocation,
- bytes to mark in order to make progress.

BUG= chromium:616434 ,  chromium:646139 , chromium:644819
LOG=NO

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

[modify] https://crrev.com/1beb89f24cb01663843a7519921e7caf0910c760/src/arm/code-stubs-arm.cc
[modify] https://crrev.com/1beb89f24cb01663843a7519921e7caf0910c760/src/arm64/code-stubs-arm64.cc
[modify] https://crrev.com/1beb89f24cb01663843a7519921e7caf0910c760/src/heap/heap.cc
[modify] https://crrev.com/1beb89f24cb01663843a7519921e7caf0910c760/src/heap/heap.h
[modify] https://crrev.com/1beb89f24cb01663843a7519921e7caf0910c760/src/heap/incremental-marking.cc
[modify] https://crrev.com/1beb89f24cb01663843a7519921e7caf0910c760/src/heap/incremental-marking.h
[modify] https://crrev.com/1beb89f24cb01663843a7519921e7caf0910c760/src/ia32/code-stubs-ia32.cc
[modify] https://crrev.com/1beb89f24cb01663843a7519921e7caf0910c760/src/x64/code-stubs-x64.cc
[modify] https://crrev.com/1beb89f24cb01663843a7519921e7caf0910c760/test/cctest/heap/test-heap.cc

Comment 12 by u...@chromium.org, Sep 29 2016

Status: Fixed (was: Started)
#11 improved jetstream by about 1%, which is the difference reported by the bisect bot in #6:

chromium@416716                221.875  1.12599   8   good
chromium@416716,v8@eca8a5ebbd  219.75   1.38873   8   bad    <--
►

Sign in to add a comment