RESOLVED FIXED 56128
REGRESSION (r80052): ~25% regression on v8-splay in-browser
https://bugs.webkit.org/show_bug.cgi?id=56128
Summary REGRESSION (r80052): ~25% regression on v8-splay in-browser
Geoffrey Garen
Reported 2011-03-10 11:52:32 PST
Stephanie measured a ~25% regression on v8-splay, caused by http://trac.webkit.org/changeset/80052.
Attachments
Patch (1.64 KB, patch)
2011-03-29 13:59 PDT, Geoffrey Garen
darin: review+
Geoffrey Garen
Comment 1 2011-03-10 13:47:47 PST
This regression is measurable in the SunSpider harness but not the v8 harness.
Geoffrey Garen
Comment 2 2011-03-10 14:34:43 PST
The extra time is in a kernel call: 0.1% 18.3% mach_kernel thread_setuserstack
Geoffrey Garen
Comment 3 2011-03-10 18:56:24 PST
thread_setuserstack seems to be a mis-symbolication by Shark. Really, this is the vm_map kernel trap. The extra calls to vm_map occur because the block size is now 8KB instead of 256KB, so we're making more kernel calls to map in a large heap.
Geoffrey Garen
Comment 4 2011-03-10 18:57:43 PST
I meant to say the block size is now 16KB.
Geoffrey Garen
Comment 5 2011-03-10 20:11:55 PST
Since this regression only shows up in the SunSpider harness, I don't think it's worth fixing. Running the v8 tests through the SunSpider harness is a Frankensteinian configuration. In this case, what you see is the cost of constructing an 8000 node data structure and then performing only 80 modifications of it. Here, initial heap growth dominates everything else in the benchmark. In the real v8 benchmark, the orders of magnitude are reversed, and there are orders of magnitude more modification operations than initial setup operations.
Stephanie Lewis
Comment 6 2011-03-23 16:23:46 PDT
Reopening because the regression also occurred testing in browser
Stephanie Lewis
Comment 7 2011-03-23 16:25:10 PDT
Geoffrey Garen
Comment 8 2011-03-29 13:59:47 PDT
Darin Adler
Comment 9 2011-03-29 14:04:00 PDT
Comment on attachment 87406 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=87406&action=review > Source/JavaScriptCore/runtime/Heap.cpp:386 > - size_t proportionalBytes = static_cast<size_t>(1.5 * m_markedSpace.size()); > + size_t proportionalBytes = 2 * m_markedSpace.size(); It’s unclear where the constant 2 comes from. Your comments in change log talk about 1X vs. .5X so there is obviously something going on here that I just can’t spot. A comment explaining what we are accomplishing by multiplying by 2 and why 2 is the right value could be quite helpful.
Geoffrey Garen
Comment 10 2011-03-29 14:50:56 PDT
Note You need to log in before you can comment on or make changes to this bug.