56128 2011-03-10 11:52:32 -0800 REGRESSION (r80052): ~25% regression on v8-splay in-browser 2011-03-29 14:50:56 -0700 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) PC OS X 10.5 RESOLVED FIXED InRadar P2 Normal --- 1 ggaren ggaren slewis oldest_to_newest 365382 0 ggaren 2011-03-10 11:52:32 -0800 Stephanie measured a ~25% regression on v8-splay, caused by http://trac.webkit.org/changeset/80052. 365498 1 ggaren 2011-03-10 13:47:47 -0800 This regression is measurable in the SunSpider harness but not the v8 harness. 365543 2 ggaren 2011-03-10 14:34:43 -0800 The extra time is in a kernel call: 0.1% 18.3% mach_kernel thread_setuserstack 365736 3 ggaren 2011-03-10 18:56:24 -0800 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. 365737 4 ggaren 2011-03-10 18:57:43 -0800 I meant to say the block size is now 16KB. 365785 5 ggaren 2011-03-10 20:11:55 -0800 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. 372525 6 slewis 2011-03-23 16:23:46 -0700 Reopening because the regression also occurred testing in browser 372527 7 slewis 2011-03-23 16:25:10 -0700 <rdar://problem/9099476> 375730 8 87406 ggaren 2011-03-29 13:59:47 -0700 Created attachment 87406 Patch 375736 9 87406 darin 2011-03-29 14:04:00 -0700 Comment on attachment 87406 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. 375779 10 ggaren 2011-03-29 14:50:56 -0700 Committed r82327: <http://trac.webkit.org/changeset/82327> 87406 2011-03-29 13:59:47 -0700 2011-03-29 14:04:00 -0700 Patch bug-56128-20110329135946.patch text/plain 1679 ggaren SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gODIzMTIpCisrKyBTb3VyY2Uv SmF2YVNjcmlwdENvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMjEgQEAK KzIwMTEtMDMtMjkgIEdlb2ZmcmV5IEdhcmVuICA8Z2dhcmVuQGFwcGxlLmNvbT4KKworICAgICAg ICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICB+MjUlIHJlZ3Jlc3Npb24g b24gdjgtc3BsYXkgaW4gdGhlIFN1blNwaWRlciBoYXJuZXNzCisgICAgICAgIGh0dHBzOi8vYnVn cy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD01NjEyOAorICAgICAgICAKKyAgICAgICAgSSdt IG5vdCBzdXJlIGlmIHRoaXMgaXMgdGhlIHJvb3QgY2F1c2Ugb2YgdGhlIHJlZ3Jlc3Npb24gU3Rl cGhhbmllCisgICAgICAgIG1lYXN1cmVkLCBidXQgaXQgc2VlbXMgdG8gZ2V0IHVzIGJhY2sgdG8g cHJldmlvdXMgdjgtc3BsYXkgdGltZXMuCisgICAgICAgIAorICAgICAgICBTdW5TcGlkZXIgcmVw b3J0cyBubyBjaGFuZ2UuIHY4LXNwbGF5IHNheXMgNDElIGZhc3Rlci4KKworICAgICAgICAqIHJ1 bnRpbWUvSGVhcC5jcHA6CisgICAgICAgIChKU0M6OkhlYXA6OnJlc2V0KTogTWFrZSBtYXJraW5n IHByb3BvcnRpb25hbCB0byAxWCB0aGUgc2l6ZSBvZiB0aGUgaGVhcCwKKyAgICAgICAgbm90IC41 WCB0aGUgc2l6ZSBvZiB0aGUgaGVhcC4gV2hlbiB0aGUgaGVhcCBpcyBsYXJnZSwgdGhpcyBtYWtl cyBhIGJpZworICAgICAgICBkaWZmZXJlbmNlLiAoT3VyIG9sZCBoZWFwIGdyb3d0aCBwb2xpY3kg bWF0Y2hlZCB0aGlzLiBZb3UgY2FuIHNlZSBieQorICAgICAgICBsb29raW5nIGF0IHJlc2l6ZUJs b2NrcyBpbiByZXZpc2lvbnMgcHJpb3IgdG8gcjc3Njk5LikKKwogMjAxMS0wMy0yOSAgU3RldmUg RmFsa2VuYnVyZyAgPHNmYWxrZW5AYXBwbGUuY29tPgogCiAgICAgICAgIFJldmlld2VkIGJ5IERh cmluIEFkbGVyLgpJbmRleDogU291cmNlL0phdmFTY3JpcHRDb3JlL3J1bnRpbWUvSGVhcC5jcHAK PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PQotLS0gU291cmNlL0phdmFTY3JpcHRDb3JlL3J1bnRpbWUvSGVhcC5jcHAJKHJl dmlzaW9uIDgyMzExKQorKysgU291cmNlL0phdmFTY3JpcHRDb3JlL3J1bnRpbWUvSGVhcC5jcHAJ KHdvcmtpbmcgY29weSkKQEAgLTM4Myw3ICszODMsNyBAQCB2b2lkIEhlYXA6OnJlc2V0KFN3ZWVw VG9nZ2xlIHN3ZWVwVG9nZ2xlCiAgICAgICAgIG1fbWFya2VkU3BhY2Uuc2hyaW5rKCk7CiAgICAg fQogCi0gICAgc2l6ZV90IHByb3BvcnRpb25hbEJ5dGVzID0gc3RhdGljX2Nhc3Q8c2l6ZV90Pigx LjUgKiBtX21hcmtlZFNwYWNlLnNpemUoKSk7CisgICAgc2l6ZV90IHByb3BvcnRpb25hbEJ5dGVz ID0gMiAqIG1fbWFya2VkU3BhY2Uuc2l6ZSgpOwogICAgIG1fbWFya2VkU3BhY2Uuc2V0SGlnaFdh dGVyTWFyayhtYXgocHJvcG9ydGlvbmFsQnl0ZXMsIG1pbkJ5dGVzUGVyQ3ljbGUpKTsKIAogICAg IEpBVkFTQ1JJUFRDT1JFX0dDX0VORCgpOwo=