New issue
Advanced search Search tips

Issue 605524 link

Starred by 0 users

Issue metadata

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



Sign in to add a comment

15.3% regression in blink_perf.bindings at 387900:387917

Project Member Reported by rmcilroy@chromium.org, Apr 21 2016

Issue description

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

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


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

chromium-rel-mac-hdd
Project Member

Comment 2 by 42576172...@developer.gserviceaccount.com, Apr 23 2016

Cc: mlippautz@chromium.org
Owner: mlippautz@chromium.org

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

Hi mlippautz@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] Optimize NewSpace::AllocatedSinceLastGC
Author  : mlippautz
Commit description:
  
Replace page link walking with arithmetic computation.

BUG=chromium:603460
LOG=N

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

Cr-Commit-Position: refs/heads/master@{#35529}
Commit  : 723e120bd04aeb4da5b6b5e348cb5eaf28de5c48
Date    : Fri Apr 15 13:03:51 2016


===== TESTED REVISIONS =====
Revision                Mean Value  Std. Dev.   Num Values  Good?
chromium@387899         431.664155  14.805306   5           good
chromium@387903         432.374221  12.962835   5           good
chromium@387905         435.626682  7.141057    5           good
chromium@387905,v8@139617b0d5429.377506  12.964119   5           good
chromium@387905,v8@5df9406a07433.113553  14.166608   5           good
chromium@387905,v8@00a589d9ff423.986103  18.098045   5           good
chromium@387905,v8@723e120bd0369.309429  16.564507   5           bad         <-
chromium@387905,v8@1c81ad3f66365.804581  32.587512   5           bad
chromium@387905,v8@09db5406d4368.122611  19.317146   5           bad
chromium@387906         378.352881  15.592467   5           bad
chromium@387908         365.708134  11.106344   5           bad
chromium@387917         375.939252  19.567222   5           bad

Bisect job ran on: mac_hdd_perf_bisect
Bug ID: 605524

Test Command: src/tools/perf/run_benchmark -v --browser=release --output-format=chartjson --upload-results --also-run-disabled-tests blink_perf.bindings
Test Metric: undefined-first-child/undefined-first-child
Relative Change: 12.91%
Score: 99.9

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/mac_hdd_perf_bisect/builds/490
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9014665414790048144


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

| 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!
Michael, could you have a look into why your CL is causing this regression or revert the CL while investigating?
Already on my radar. 

If it's not too critical I would rather investigate and fix than revert. The change should only have positive impact on performance.
Project Member

Comment 5 by bugdroid1@chromium.org, Apr 28 2016

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

commit 08dbdd4037aa612dc1afe40dbb073b3384b8a497
Author: mlippautz <mlippautz@chromium.org>
Date: Thu Apr 28 13:51:34 2016

[heap] Force inlining of AllocatedSinceLastGC

Speculatively forcining inlining as not inlining potentially regresses
performance.

BUG= chromium:605524 
LOG=N

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

[modify] https://crrev.com/08dbdd4037aa612dc1afe40dbb073b3384b8a497/src/heap/heap.h
[modify] https://crrev.com/08dbdd4037aa612dc1afe40dbb073b3384b8a497/src/heap/spaces.h

Project Member

Comment 6 by bugdroid1@chromium.org, Apr 28 2016

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

commit 5dc254f63b3c592c8e6c37b8012a5788fc45e5db
Author: mlippautz <mlippautz@chromium.org>
Date: Thu Apr 28 21:08:35 2016

Revert "[heap] Optimize NewSpace::AllocatedSinceLastGC"

Also revert "[heap] Force inlining of AllocatedSinceLastGC"

This is a speculative revert to see if it actually impacts the benchmarks in
question.

This reverts commit 723e120bd04aeb4da5b6b5e348cb5eaf28de5c48.
This reverts commit 08dbdd4037aa612dc1afe40dbb073b3384b8a497.

BUG= chromium:605524 
LOG=N
TBR=ulan@chromium.org

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

[modify] https://crrev.com/5dc254f63b3c592c8e6c37b8012a5788fc45e5db/src/heap/heap-inl.h
[modify] https://crrev.com/5dc254f63b3c592c8e6c37b8012a5788fc45e5db/src/heap/heap.h
[modify] https://crrev.com/5dc254f63b3c592c8e6c37b8012a5788fc45e5db/src/heap/spaces-inl.h
[modify] https://crrev.com/5dc254f63b3c592c8e6c37b8012a5788fc45e5db/src/heap/spaces.cc
[modify] https://crrev.com/5dc254f63b3c592c8e6c37b8012a5788fc45e5db/src/heap/spaces.h

Status: Fixed (was: Assigned)
The regression seems fixed (by reverting). Will investigate before re-landing.

Sign in to add a comment