New issue
Advanced search Search tips

Issue 615831 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Closed: Oct 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

28.3% regression in memory.long_running_idle_gmail_tbmv2 at 396410:396424

Project Member Reported by primiano@chromium.org, May 30 2016

Issue description

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

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


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

chromium-rel-win7-gpu-nvidia
Project Member

Comment 2 by 42576172...@developer.gserviceaccount.com, May 30 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     Std Dev   N   Good?
chromium@396409  3.71522  0.40717   18  good
chromium@396424  3.65861  0.346147  18  bad

Bisect job ran on: winx64nvidia_perf_bisect
Bug ID: 615831

Test Command: src/tools/perf/run_benchmark -v --browser=release_x64 --output-format=chartjson --upload-results --also-run-disabled-tests memory.long_running_idle_gmail_tbmv2
Test Metric: Idle-v8-gc-incremental-finalize_max/Idle-v8-gc-incremental-finalize_max
Relative Change: 8.80%
Score: 0

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/winx64nvidia_perf_bisect/builds/1645
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9011239159427578096


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

| 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!
Project Member

Comment 3 by 42576172...@developer.gserviceaccount.com, May 30 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     Std Dev   N   Good?
chromium@396409  3.83067  0.656718  18  good
chromium@396424  3.65367  0.491618  18  bad

Bisect job ran on: winx64nvidia_perf_bisect
Bug ID: 615831

Test Command: src/tools/perf/run_benchmark -v --browser=release_x64 --output-format=chartjson --upload-results --also-run-disabled-tests memory.long_running_idle_gmail_tbmv2
Test Metric: Idle-v8-gc-incremental-finalize_max/Idle-v8-gc-incremental-finalize_max
Relative Change: 2.87%
Score: 0

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/winx64nvidia_perf_bisect/builds/1644
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9011239163433183600


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

| 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!
Cc: bmeu...@chromium.org mvstan...@chromium.org peria@chromium.org
There seems to be a definite regression, I can see it on different bots on all OS
https://chromeperf.appspot.com/report?sid=ec6c03ba490591eee9123ed9f27fe51fe9460668405eb437c161878bcee11cdc

Trying bisecting on other bots. In the meantime cc-ing plausible candidates by eyeballing at the CL range.
If anybody has any idea please speak up.
mvstanton https://codereview.chromium.org/1906823002
bmeurer https://codereview.chromium.org/2016993003
peria https://codereview.chromium.org/2017893002
Cc: primiano@chromium.org
 Issue 616079  has been merged into this issue.
This is showing up on more and more bots, so i kicked off an additional bisect where the regression is clear.
Owner: mvstan...@chromium.org

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

Hi mvstanton@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 : Move of the type feedback vector to the closure.
Author  : mvstanton
Commit description:
  
We get less "pollution" of type feedback if we have one vector per native
context, rather than one for the whole system. This CL moves the vector
appropriately.

BUG=

Review-Url: https://codereview.chromium.org/1906823002
Cr-Commit-Position: refs/heads/master@{#36539}
Commit  : 91c88644dcf396285f88977080ea85f4614bbc02
Date    : Fri May 27 08:10:51 2016


===== TESTED REVISIONS =====
Revision                       Mean  Std Dev    N  Good?
chromium@396437                2.21  0.0547723  5  good
chromium@396439                2.15  0.1        5  good
chromium@396439,v8@1332be4ab4  2.21  0.114018   5  good
chromium@396439,v8@91c88644dc  3.17  0.327109   5  bad    <--
chromium@396440                3.19  0.230217   5  bad
chromium@396442                3.09  0.207364   5  bad
chromium@396447                3.15  0.2        5  bad

Bisect job ran on: win_8_perf_bisect
Bug ID: 615831

Test Command: src/tools/perf/run_benchmark -v --browser=release --output-format=chartjson --upload-results --also-run-disabled-tests memory.long_running_idle_gmail_tbmv2
Test Metric: Idle-v8-gc-incremental-finalize_pct_090/Idle-v8-gc-incremental-finalize_pct_090
Relative Change: 42.53%
Score: 99.5

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/win_8_perf_bisect/builds/1951
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9011036140158498256


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

| 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!
Status: Started (was: Assigned)
We've increased the number of weak cells considerably, and the atomic processing of these is expensive.

milliseconds in incremental finalization:
  ProcessWeakCells master   9
  ProcessWeakCells mine     15
  %change                   -66.67%

Continuing...

I've got a fix for this...
Project Member

Comment 11 by bugdroid1@chromium.org, Jun 7 2016

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

commit 3cfcc7e111a32f231a483b8feda19ef2d8c45253
Author: mvstanton <mvstanton@chromium.org>
Date: Tue Jun 07 12:01:51 2016

Avoid creating weak cells for literal arrays that are empty of literals.

It may be that we have a feedback vector, but no literals. In this case
we can store into the OptimizedCodeMap directly instead of using a WeakCell,
because all data in the feedback vector is already held weakly.

The use of a WeakCell in the OptimizedCodeMap is only required when
there are literals which may hold maps strongly.

This is to address a performance regression caused by the creation of
a large number of WeakCells.

BUG= chromium:615831 

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

[modify] https://crrev.com/3cfcc7e111a32f231a483b8feda19ef2d8c45253/src/arm/builtins-arm.cc
[modify] https://crrev.com/3cfcc7e111a32f231a483b8feda19ef2d8c45253/src/arm64/builtins-arm64.cc
[modify] https://crrev.com/3cfcc7e111a32f231a483b8feda19ef2d8c45253/src/ia32/builtins-ia32.cc
[modify] https://crrev.com/3cfcc7e111a32f231a483b8feda19ef2d8c45253/src/mips/builtins-mips.cc
[modify] https://crrev.com/3cfcc7e111a32f231a483b8feda19ef2d8c45253/src/mips64/builtins-mips64.cc
[modify] https://crrev.com/3cfcc7e111a32f231a483b8feda19ef2d8c45253/src/objects.cc
[modify] https://crrev.com/3cfcc7e111a32f231a483b8feda19ef2d8c45253/src/type-feedback-vector.cc
[modify] https://crrev.com/3cfcc7e111a32f231a483b8feda19ef2d8c45253/src/x64/builtins-x64.cc

It looks like this didn't actually help. I'll keep looking this week...
Labels: -performance-sheriff Performance-Sheriff
Looks like this hasn't recovered yet. Any update, mvstanton?

Comment 14 by peria@chromium.org, Jun 28 2016

Cc: -peria@chromium.org
Any update?  The regression has not yet recovered.
Project Member

Comment 16 by sheriffbot@chromium.org, Jul 8 2016

Labels: -M-53 M-54 MovedFrom-53
Moving this nonessential bug to the next milestone.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
I'll be investigating again in the next days, sorry for the delay!
Regular perf sheriff ping :)   Any updates?
mvstanton, weekly check in on this.
Friendly perf-sheriff ping, any update on this?
Perf sheriff ping
No update yet, under investigation.
Weekly check in from your friendly triage team :) don't forget to update this.
Ping  mvstanton@, please let us know your investgation's updates.
mvstanton@, we haven't seen an update since July. Has there been any movement here?
Status: WontFix (was: Started)
Creating one feedback vector per native context helps us out a lot in the V8 runtime. It provides higher quality feedback because each vector only contains maps from one native context, thus increasing the chances that we can compile monomorphic or polymorphic code instead of going generic for property loads and stores.

Follow on work on top of this over the summer made all global loads more efficient. This improved the speedometer benchmark and "real world" (top 25 web sites) performance.

We simply use more WeakCells in the system now. I'll close this bug because I don't see a handle on a way to fix it, and other areas of V8 improved.


Sign in to add a comment