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

Issue 670205 link

Starred by 1 user
Status: Duplicate
Owner:
jgruber@chromium.org
Closed: Dec 2016
Cc:
jgruber@chromium.org
█ epertoso@chromium.org
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

2%-16.7% regression in memory.top_10_mobile at 434943:435065

Project Member Reported by epertoso@chromium.org, Dec 1 2016 
See the link to graphs below.
Project Member

Comment 3 by 42576172...@developer.gserviceaccount.com, Dec 1 2016

Cc: jgruber@chromium.org
Owner: jgruber@chromium.org

=== PERF REGRESSION ===


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

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


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


===== SUSPECTED CL(s) =====
Subject : [regexp] Migrate @@match to TurboFan
Author  : jgruber
Commit description:
  
Microbenchmarks show a 4x improvement on the fast path and 2.5x improvement on
the slow path when compared to the CPP builtin implementation.

Compared to the old JS implementation, the fast path is 20% faster and the slow
path 35% slower.

BUG= v8:5339 , v8:5562 

Review-Url: https://codereview.chromium.org/2527963002
Cr-Commit-Position: refs/heads/master@{#41338}
Commit  : 4e7571a5a9bb8563ae93e0aff48c08bead765b14
Date    : Tue Nov 29 09:03:18 2016


===== TESTED REVISIONS =====
Revision                       Mean     Std Dev   N    Good?
chromium@434976                7101817  34575405  100  good
chromium@434978                7111147  34649740  100  good
chromium@434978,v8@18eda7024b  7110449  34525955  100  good
chromium@434978,v8@9c0e2a6723  7123048  34570785  100  good
chromium@434978,v8@4e7571a5a9  8204244  34414524  100  bad    <--
chromium@434979                8239214  34552206  100  bad
chromium@434982                8234146  34625943  100  bad
chromium@434987                8222428  34487231  100  bad

Bisect job ran on: android_nexus9_perf_bisect
Bug ID: 670205

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests memory.top_10_mobile
Test Metric: memory:chrome:renderer_processes:reported_by_chrome:v8:heap:effective_size_avg/memory:chrome:renderer_processes:reported_by_chrome:v8:heap:effective_size_avg
Relative Change: 15.78%

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


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

| 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 5 by 42576172...@developer.gserviceaccount.com, Dec 2 2016 


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


===== SUSPECTED CL(s) =====
Subject : [regexp] Migrate @@match to TurboFan
Author  : jgruber
Commit description:
  
Microbenchmarks show a 4x improvement on the fast path and 2.5x improvement on
the slow path when compared to the CPP builtin implementation.

Compared to the old JS implementation, the fast path is 20% faster and the slow
path 35% slower.

BUG= v8:5339 , v8:5562 

Review-Url: https://codereview.chromium.org/2527963002
Cr-Commit-Position: refs/heads/master@{#41338}
Commit  : 4e7571a5a9bb8563ae93e0aff48c08bead765b14
Date    : Tue Nov 29 09:03:18 2016


===== TESTED REVISIONS =====
Revision                       Mean     Std Dev  N  Good?
chromium@434976                4571657  572405   8  good
chromium@434978                4502936  561033   8  good
chromium@434978,v8@18eda7024b  4564382  153854   8  good
chromium@434978,v8@9c0e2a6723  4626149  749117   8  good
chromium@434978,v8@4e7571a5a9  5831021  338581   5  bad    <--
chromium@434979                5569357  298385   5  bad
chromium@434981                5569762  296431   5  bad
chromium@434985                5731874  283926   5  bad
chromium@434992                5601707  305329   5  bad
chromium@435007                5667723  464179   5  bad

Bisect job ran on: android_nexus9_perf_bisect
Bug ID: 670205

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests --story-filter=http.yandex.ru.touchsearch.text.science memory.top_10_mobile
Test Metric: memory:chrome:renderer_processes:reported_by_chrome:v8:effective_size_avg/background/after_http_yandex_ru_touchsearch_text_science
Relative Change: 23.98%

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


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

| 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 6 by jgruber@chromium.org, Dec 2 2016 

An initial guess what could be going on:

re[@@match](str) creates an array of all matches found in a string if re is a global regexp.

The result array is initially allocated with a capacity of 8 and then grows dynamically. The original @@match implementation shrunk the array to its actual size at the end, while the new implementation does not.

Will take a look.
Project Member

Comment 7 by bugdroid1@chromium.org, Dec 7 2016 

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

commit 40e176056d9640635d3bbc9f4e63b50ccc9057bc
Author: jgruber <jgruber@chromium.org>
Date: Wed Dec 07 13:05:30 2016

[regexp] Shrink results array in @@match and @@split

Both @@match and @@split internally use dynamically growing fixed
arrays. Shrink to fit when wrapping these in a JSArray to avoid
excessive memory usage.

BUG= chromium:670205 ,chromium:670708

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

[modify] https://crrev.com/40e176056d9640635d3bbc9f4e63b50ccc9057bc/src/builtins/builtins-regexp.cc
Project Member

Comment 9 by 42576172...@developer.gserviceaccount.com, Dec 8 2016

Mergedinto: 670718
Status: Duplicate (was: Assigned)

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


===== SUSPECTED CL(s) =====
Subject : [stubs] Port builtin for Array.push fast-case from Crankshaft to TF
Author  : danno
Commit description:
  
Improves performance in simple, single element case by 5% and in multiple
elements cases by 2%.

BUG= chromium:608675 
LOG=N

Review-Url: https://codereview.chromium.org/2497243002
Cr-Commit-Position: refs/heads/master@{#41368}
Commit  : df2578d2ec630ccd08619e1f733da3f61e7ee380
Date    : Tue Nov 29 16:58:30 2016


===== TESTED REVISIONS =====
Revision                       Mean    Std Dev  N   Good?
chromium@435138                154829  1668928  50  good
chromium@435210                154829  1668928  50  good
chromium@435228                105813  1510078  48  good
chromium@435228,v8@957f3f10e5  118784  1672703  50  good
chromium@435228,v8@f8b8983962  118784  1672703  50  good
chromium@435228,v8@df2578d2ec  190874  1668043  50  bad    <--
chromium@435228,v8@d385ed069b  190874  1668043  50  bad
chromium@435229                190874  1668043  50  bad
chromium@435230                190874  1491943  40  bad
chromium@435231                190874  1668043  50  bad
chromium@435233                190874  1668043  50  bad
chromium@435237                190874  1668043  50  bad
chromium@435246                190874  1668043  50  bad
chromium@435282                190874  1668043  50  bad
chromium@435425                190874  1668043  50  bad

Bisect job ran on: android_nexus6_perf_bisect
Bug ID: 670205

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests memory.blink_memory_mobile
Test Metric: memory:chrome:renderer_processes:reported_by_chrome:v8:heap:large_object_space:effective_size_avg/memory:chrome:renderer_processes:reported_by_chrome:v8:heap:large_object_space:effective_size_avg
Relative Change: 23.28%

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus6_perf_bisect/builds/2810
Job details: https://chromeperf.appspot.com/buildbucket_job_status/8993854502628090912


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

| 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 10 by jgruber@chromium.org, Dec 9 2016

Status: Assigned (was: Duplicate)
One of the associated alerts is unrelated (see #9), but this issue itself is still valid.
Project Member

Comment 12 by 42576172...@developer.gserviceaccount.com, Dec 9 2016 


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


===== TESTED REVISIONS =====
Revision         Mean     Std Dev  N   Good?
chromium@434955  6536516  848599   27  good
chromium@434971  6534202  531165   18  good
chromium@434975  6362971  219642   5   good
chromium@434977  6491913  418855   8   good
chromium@434978  6577058  762901   27  unknown
chromium@434979  6702787  1007652  27  unknown
chromium@434980  6742648  1210795  27  bad
chromium@434986  6797748  1316020  18  bad
chromium@435016  6904430  890123   8   bad
chromium@435076  6753468  1501795  18  bad

Bisect job ran on: android_nexus6_perf_bisect
Bug ID: 670205

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests --story-filter=load.search.amazon system_health.memory_mobile
Test Metric: memory:chrome:all_processes:reported_by_chrome:v8:effective_size_avg/load_search/load_search_amazon
Relative Change: 3.32%

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus6_perf_bisect/builds/2815
Job details: https://chromeperf.appspot.com/buildbucket_job_status/8993754011516642624


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

| 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 13 by jgruber@chromium.org, Dec 13 2016

Status: Duplicate (was: Assigned)
►

Sign in to add a comment