Issue metadata
Sign in to add a comment
|
27.1% regression in blink_perf.bindings at 398489:398501 |
||||||||||||||||||||
Issue description
,
Jun 13 2016
=== 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 : Track based on JSArrayBuffer addresses on pages instead of the attached Author : mlippautz Commit description: backing store. Details of tracking: - Scavenge: New space pages are processes in bulk on the main thread - MC: Unswept pages are processed in bulk in parallel. All other pages are processed by the sweeper concurrently. BUG= chromium:611688 LOG=N TEST=cctest/test-array-buffer-tracker/* CQ_EXTRA_TRYBOTS=tryserver.v8:v8_linux_arm64_gc_stress_dbg,v8_linux_gc_stress_dbg,v8_mac_gc_stress_dbg,v8_linux64_tsan_rel,v8_mac64_asan_rel Review-Url: https://codereview.chromium.org/2036643002 Cr-Commit-Position: refs/heads/master@{#36798} Commit : 839f3fd406426a221d74eb7a33a72794c3c7a548 Date : Tue Jun 07 17:29:35 2016 ===== TESTED REVISIONS ===== Revision Mean Std Dev N Good? chromium@398488 334.453 15.9733 5 good chromium@398492 354.044 2.78508 5 good chromium@398494 355.054 4.19312 5 good chromium@398494,v8@1f2eaa1c14 361.283 2.06264 5 good chromium@398494,v8@ba3703db61 360.449 1.99176 5 good chromium@398494,v8@839f3fd406 251.749 2.31745 5 bad <-- chromium@398494,v8@67af060318 255.025 2.40741 5 bad chromium@398495 249.427 3.20648 5 bad chromium@398501 248.483 6.95451 5 bad Bisect job ran on: android_s5_perf_bisect Bug ID: 619491 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-same-type/typed-array-construct-from-same-type Relative Change: 25.70% Score: 99.9 Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_s5_perf_bisect/builds/702 Job details: https://chromeperf.appspot.com/buildbucket_job_status/9009984205473863024 Not what you expected? We'll investigate and get back to you! https://chromeperf.appspot.com/bad_bisect?try_job_id=5787664784031744 | 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!
,
Jun 13 2016
===== BISECT JOB RESULTS ===== Status: completed ===== SUSPECTED CL(s) ===== Subject : Track based on JSArrayBuffer addresses on pages instead of the attached Author : mlippautz Commit description: backing store. Details of tracking: - Scavenge: New space pages are processes in bulk on the main thread - MC: Unswept pages are processed in bulk in parallel. All other pages are processed by the sweeper concurrently. BUG= chromium:611688 LOG=N TEST=cctest/test-array-buffer-tracker/* CQ_EXTRA_TRYBOTS=tryserver.v8:v8_linux_arm64_gc_stress_dbg,v8_linux_gc_stress_dbg,v8_mac_gc_stress_dbg,v8_linux64_tsan_rel,v8_mac64_asan_rel Review-Url: https://codereview.chromium.org/2036643002 Cr-Commit-Position: refs/heads/master@{#36798} Commit : 839f3fd406426a221d74eb7a33a72794c3c7a548 Date : Tue Jun 07 17:29:35 2016 ===== TESTED REVISIONS ===== Revision Mean Std Dev N Good? chromium@398488 316.851 4.48086 5 good chromium@398492 331.532 4.38671 5 good chromium@398494 333.214 7.75391 5 good chromium@398494,v8@1f2eaa1c14 333.502 5.68011 5 good chromium@398494,v8@ba3703db61 331.675 1.17657 5 good chromium@398494,v8@839f3fd406 234.988 3.7618 5 bad <-- chromium@398494,v8@67af060318 236.899 3.26636 5 bad chromium@398495 233.138 3.82313 5 bad chromium@398501 234.57 3.78646 5 bad Bisect job ran on: android_s5_perf_bisect Bug ID: 619491 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-same-type/typed-array-construct-from-same-type Relative Change: 25.97% Score: 99.9 Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_s5_perf_bisect/builds/708 Job details: https://chromeperf.appspot.com/buildbucket_job_status/9009958215201970416 Not what you expected? We'll investigate and get back to you! https://chromeperf.appspot.com/bad_bisect?try_job_id=5812704577585152 | 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!
,
Jun 13 2016
Alright, I could also repro on x64 locally. The benchmarks harness a fast path for array buffer registration which needs to be properly inlined. On x64 the numbers recover with better inlining so I will prepare a CL for that. In general I am not a big fan of those microbenchmarks with super-tight loops though.
,
Jun 15 2016
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/5846acc240dd5df71759fefbe9b1ad0bc17f6375 commit 5846acc240dd5df71759fefbe9b1ad0bc17f6375 Author: mlippautz <mlippautz@chromium.org> Date: Wed Jun 15 16:41:23 2016 [heap] Add inlined fast path for JSArrayBuffer (un)register in tracker BUG= chromium:619491 , chromium:611688 LOG=N R=ulan@chromium.org Review-Url: https://codereview.chromium.org/2065013002 Cr-Commit-Position: refs/heads/master@{#37010} [modify] https://crrev.com/5846acc240dd5df71759fefbe9b1ad0bc17f6375/BUILD.gn [add] https://crrev.com/5846acc240dd5df71759fefbe9b1ad0bc17f6375/src/heap/array-buffer-tracker-inl.h [modify] https://crrev.com/5846acc240dd5df71759fefbe9b1ad0bc17f6375/src/heap/array-buffer-tracker.cc [modify] https://crrev.com/5846acc240dd5df71759fefbe9b1ad0bc17f6375/src/heap/array-buffer-tracker.h [modify] https://crrev.com/5846acc240dd5df71759fefbe9b1ad0bc17f6375/src/heap/heap.cc [modify] https://crrev.com/5846acc240dd5df71759fefbe9b1ad0bc17f6375/src/v8.gyp
,
Jun 17 2016
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/849d6b45ef6358cedf2451a56a6179f380c95cf3 commit 849d6b45ef6358cedf2451a56a6179f380c95cf3 Author: mlippautz <mlippautz@chromium.org> Date: Fri Jun 17 10:26:18 2016 [heap] Preallocate ArrayBufferTracker for new space pages BUG= chromium:619491 LOG=N Review-Url: https://codereview.chromium.org/2076093002 Cr-Commit-Position: refs/heads/master@{#37063} [modify] https://crrev.com/849d6b45ef6358cedf2451a56a6179f380c95cf3/src/heap/spaces-inl.h
,
Jun 24 2016
Doesn't seem to have moved the benchmark at all. Was it supposed to recover some of the loss?
,
Jun 27 2016
Actually, yes. I checked locally on a N5 (which showed a similar regression) that it not only recovered but improved this benchmark. Will check again once I have access to HW (sometime this week).
,
Jun 28 2016
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/8d2ae27808f047ca8b8f90e63a9c8735321d2ad0 commit 8d2ae27808f047ca8b8f90e63a9c8735321d2ad0 Author: mlippautz <mlippautz@chromium.org> Date: Tue Jun 28 08:36:33 2016 [heap] Optimize ArrayBuffer tracking With the current approach we only need to track using an unordered set as we can still access the backing store pointer and length by the time we free the backing store. BUG= chromium:619491 , chromium:611688 LOG=N R=ulan@chromium.org Review-Url: https://codereview.chromium.org/2107443002 Cr-Commit-Position: refs/heads/master@{#37318} [modify] https://crrev.com/8d2ae27808f047ca8b8f90e63a9c8735321d2ad0/src/heap/array-buffer-tracker-inl.h [modify] https://crrev.com/8d2ae27808f047ca8b8f90e63a9c8735321d2ad0/src/heap/array-buffer-tracker.cc [modify] https://crrev.com/8d2ae27808f047ca8b8f90e63a9c8735321d2ad0/src/heap/array-buffer-tracker.h
,
Jun 29 2016
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/0e1eaec71d5c6d05169308a71eaa51571e7c4f70 commit 0e1eaec71d5c6d05169308a71eaa51571e7c4f70 Author: mlippautz <mlippautz@chromium.org> Date: Wed Jun 29 05:29:24 2016 Revert of [heap] Optimize ArrayBuffer tracking (patchset #5 id:80001 of https://codereview.chromium.org/2107443002/ ) Reason for revert: Seems to break GPU bots Original issue's description: > [heap] Optimize ArrayBuffer tracking > > With the current approach we only need to track using an unordered set as we can > still access the backing store pointer and length by the time we free the > backing store. > > BUG= chromium:619491 , chromium:611688 > LOG=N > R=ulan@chromium.org > > Committed: https://crrev.com/8d2ae27808f047ca8b8f90e63a9c8735321d2ad0 > Cr-Commit-Position: refs/heads/master@{#37318} TBR=ulan@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= chromium:619491 , chromium:611688 Review-Url: https://codereview.chromium.org/2105273002 Cr-Commit-Position: refs/heads/master@{#37358} [modify] https://crrev.com/0e1eaec71d5c6d05169308a71eaa51571e7c4f70/src/heap/array-buffer-tracker-inl.h [modify] https://crrev.com/0e1eaec71d5c6d05169308a71eaa51571e7c4f70/src/heap/array-buffer-tracker.cc [modify] https://crrev.com/0e1eaec71d5c6d05169308a71eaa51571e7c4f70/src/heap/array-buffer-tracker.h
,
Jun 29 2016
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/f58dd088f0886f838f35a38613cd7b1a66f0cd63 commit f58dd088f0886f838f35a38613cd7b1a66f0cd63 Author: mlippautz <mlippautz@chromium.org> Date: Wed Jun 29 14:53:34 2016 Reland "[heap] Optimize ArrayBuffer tracking" With the current approach we only need to track using an unordered set as we can still access the backing store pointer and length by the time we free the backing store. Reland: The issue was fixed in 67b5a501db179412479e827e51c43515e4196b27. BUG= chromium:619491 , chromium:611688 LOG=N R=ulan@chromium.org This reverts commit 0e1eaec71d5c6d05169308a71eaa51571e7c4f70. Review-Url: https://codereview.chromium.org/2109913003 Cr-Commit-Position: refs/heads/master@{#37399} [modify] https://crrev.com/f58dd088f0886f838f35a38613cd7b1a66f0cd63/src/heap/array-buffer-tracker-inl.h [modify] https://crrev.com/f58dd088f0886f838f35a38613cd7b1a66f0cd63/src/heap/array-buffer-tracker.cc [modify] https://crrev.com/f58dd088f0886f838f35a38613cd7b1a66f0cd63/src/heap/array-buffer-tracker.h
,
Jul 5 2016
Moving this nonessential bug to the next milestone. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jul 11 2016
This doesn't seem to have recovered. mlippautz@, any thoughts?
,
Jul 22 2016
Agreed, not seeing any recovery here. Any other ideas?
,
Aug 1 2016
Friendly perf-sheriff ping, any update on this?
,
Aug 3 2016
I don't see a short-term fix for this regression. However, the concept of the new tracker is needed to make progress in other directions that enable optimizations on real-world websites. Medium-term we'd like to not rely on full GCs in this case, but have eager scavenges kick in (based on amount of external memory allocated). I'll keep this bug open and update it once this lands.
,
Aug 17 2016
Perf sheriff ping: mlippautz@, any updates on eager scavenges? Is there another bug that this bug can be blocked on?
,
Aug 18 2016
Will happen at some point, not immediately though. The regression in this micro benchmark is expected since we just plainly copy more memory in new space. Marking as WontFix for now. |
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by oth@chromium.org
, Jun 13 2016