201631 2019-09-09 22:17:06 -0700 [JSC] Manually adding 256 size class to make the current JSC allocation behavior stable 2019-09-10 02:08:22 -0700 1 1 1 Unclassified WebKit JavaScriptCore WebKit Nightly Build Unspecified Unspecified NEW P2 Normal --- 1 ysuzuki ysuzuki ews-watchlist keith_miller mark.lam msaboff saam tzagallo oldest_to_newest 1569451 0 ysuzuki 2019-09-09 22:17:06 -0700 We have old and hacky code like this. add(sizeof(UnlinkedFunctionCodeBlock)); This manually adds sizeof(UnlinkedFunctionCodeBlock) to size class sequence. This is really fragile since size-class sequence depends on UnlinkedFunctionCodeBlock size. The size-class sequence is very fundamental thing for JSC's allocation pattern. And changing this completely changes how JSC allocates. bug 201613 changed sizeof(UnlinkedFunctionCodeBlock) and dramatically changes the allocation behavior of JSC. We should make the previous allocation pattern stable. Instead of adding `add(sizeof(UnlinkedFunctionCodeBlock));`, adding `add(256)` directly to make the previous behavior as baseline. 1569455 1 378443 ysuzuki 2019-09-09 22:42:51 -0700 Created attachment 378443 Patch 1569456 2 ysuzuki 2019-09-09 22:42:59 -0700 The previous size-class sequence JSC Heap MarkedSpace size class dump: 16, 32, 48, 64, 80, 112, 160, 224, 256, 320, 432, 608, 880, 1232, 1776, 2672, 4016, 5360, 8032 The current size-class sequence JSC Heap MarkedSpace size class dump: 16, 32, 48, 64, 80, 112, 160, 224, 240, 320, 432, 608, 880, 1232, 1776, 2672, 4016, 5360, 8032 This patch will explicitly select the previous sequence. 224 <-> 240 is only 16, one cell size. We know 256 works well so we will pick 256 here. 256's characteristics is nice. 256*2 will becomes 608 size class, and 608*2 will becomes 1232. We know the previous size-class sequence tends to work well with the code like, var array = []; while (...) array.push(value); 1569460 3 378443 mark.lam 2019-09-09 23:16:42 -0700 Comment on attachment 378443 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=378443&action=review r=me > Source/JavaScriptCore/ChangeLog:38 > + This patch explicitly adds 256 into the size-class to make the size-class sequence the thing we know it works well. I suggest /know it works well/know works well/. > Source/JavaScriptCore/heap/MarkedSpace.cpp:139 > + // Manually adding 256 case, empiricailly know this produces good sequence of size classes. I suggest rephrasing slightly: "Manually adding 256 case. We know empirically that this produces efficient memory usage of size classes." 1569462 4 378443 mark.lam 2019-09-09 23:19:14 -0700 Comment on attachment 378443 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=378443&action=review >> Source/JavaScriptCore/heap/MarkedSpace.cpp:139 >> + // Manually adding 256 case, empiricailly know this produces good sequence of size classes. > > I suggest rephrasing slightly: "Manually adding 256 case. We know empirically that this produces efficient memory usage of size classes." Better yet: "Manually adding 256 case. We know empirically that having this size class results in more efficient memory usage." or something like that. 1569474 5 ysuzuki 2019-09-10 02:06:43 -0700 Hmm, after checking the regression, it seems that this does not affect much. I'll look into further. macOS regression seems due to scavenger's change. 1569475 6 378443 ysuzuki 2019-09-10 02:08:22 -0700 Comment on attachment 378443 Patch I'll look into the problem further. After I'm confident this is better, I'll upload the revised version of this. 378443 2019-09-09 22:42:51 -0700 2019-09-10 02:08:22 -0700 Patch bug-201631-20190909224250.patch text/plain 3908 ysuzuki U3VidmVyc2lvbiBSZXZpc2lvbjogMjQ5NzA2CmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlw dENvcmUvQ2hhbmdlTG9nIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwppbmRleCBk YjRhM2EwMDNmMDY2YWY1MGE4N2RhMjdlMWFjMmVmM2I2M2Y5NDgxLi5lNGNhNzRjNzRkNWYyNTU2 ZGRjOTVjZDU0NTE1YzRhNmQxOGQ0MDFlIDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKQEAgLTEs MyArMSw0OSBAQAorMjAxOS0wOS0wOSAgWXVzdWtlIFN1enVraSAgPHlzdXp1a2lAYXBwbGUuY29t PgorCisgICAgICAgIFtKU0NdIE1hbnVhbGx5IGFkZGluZyAyNTYgc2l6ZSBjbGFzcyB0byBtYWtl IHRoZSBjdXJyZW50IEpTQyBhbGxvY2F0aW9uIGJlaGF2aW9yIHN0YWJsZQorICAgICAgICBodHRw czovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MjAxNjMxCisKKyAgICAgICAgUmV2 aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgV2UgaGF2ZSBvbGQgYW5kIGhhY2t5 IGNvZGUgbGlrZSB0aGlzLgorCisgICAgICAgICAgICBhZGQoc2l6ZW9mKFVubGlua2VkRnVuY3Rp b25Db2RlQmxvY2spKTsKKworICAgICAgICBUaGlzIG1hbnVhbGx5IGFkZHMgYHNpemVvZihVbmxp bmtlZEZ1bmN0aW9uQ29kZUJsb2NrKWAgdG8gc2l6ZS1jbGFzcyBzZXF1ZW5jZS4KKyAgICAgICAg VGhpcyBpcyAqKnJlYWxseSoqIGZyYWdpbGUgc2luY2Ugc2l6ZS1jbGFzcyBzZXF1ZW5jZSBkZXBl bmRzIG9uIFVubGlua2VkRnVuY3Rpb25Db2RlQmxvY2sgc2l6ZS4KKyAgICAgICAgVGhlIHNpemUt Y2xhc3Mgc2VxdWVuY2UgaXMgdmVyeSBmdW5kYW1lbnRhbCB0aGluZyBmb3IgSlNDJ3MgYWxsb2Nh dGlvbiBwYXR0ZXJuLgorICAgICAgICBBbmQgY2hhbmdpbmcgdGhpcyBjb21wbGV0ZWx5IGNoYW5n ZXMgaG93IEpTQyBhbGxvY2F0ZXMuCisKKyAgICAgICAgcjI0OTY2OCBmaXhlZCBhIGJ1ZyB0aGF0 IGlzIGNvbXBsZXRlbHkgdW5yZWxhdGVkIHRvIGFsbG9jYXRpb25zLiBCdXQgdGhhdCBwYXRjaCBj aGFuZ2VkIHNpemVvZihVbmxpbmtlZEZ1bmN0aW9uQ29kZUJsb2NrKQorICAgICAgICBhbmQgdGhp cyBkcmFtYXRpY2FsbHkgY2hhbmdlZCB0aGUgYWxsb2NhdGlvbiBiZWhhdmlvciBvZiBKU0MuCisK KyAgICAgICAgV2Uga25vdyB0aGF0IHRoZSBwcmV2aW91cyBzaXplLWNsYXNzIHNlcXVlbmNlIGVt cGlyaWNhbGx5IHdvcmtzIHdlbGwgaW4gUkFNaWZpY2F0aW9uLiBXaGlsZSBpdCB3b3JrZWQgd2Vs bCBpbiBMdWFKU0ZpZ2h0LAorICAgICAgICB0aGF0IHNlcXVlbmNlIHdvcmtlZCB3ZWxsIGV2ZW4g aW4gb3RoZXIgUkFNaWZpY2F0aW9uIHRlc3RzLCBiYXNlZCBvbiBKZXRTdHJlYW0yLgorCisgICAg ICAgICAgICBUaGUgcHJldmlvdXMgc2l6ZS1jbGFzcyBzZXF1ZW5jZQorICAgICAgICAgICAgSlND IEhlYXAgTWFya2VkU3BhY2Ugc2l6ZSBjbGFzcyBkdW1wOiAxNiwgMzIsIDQ4LCA2NCwgODAsIDEx MiwgMTYwLCAyMjQsIDI1NiwgMzIwLCA0MzIsIDYwOCwgODgwLCAxMjMyLCAxNzc2LCAyNjcyLCA0 MDE2LCA1MzYwLCA4MDMyCisKKyAgICAgICAgICAgIFRoZSBjdXJyZW50IHNpemUtY2xhc3Mgc2Vx dWVuY2UKKyAgICAgICAgICAgIEpTQyBIZWFwIE1hcmtlZFNwYWNlIHNpemUgY2xhc3MgZHVtcDog MTYsIDMyLCA0OCwgNjQsIDgwLCAxMTIsIDE2MCwgMjI0LCAyNDAsIDMyMCwgNDMyLCA2MDgsIDg4 MCwgMTIzMiwgMTc3NiwgMjY3MiwgNDAxNiwgNTM2MCwgODAzMgorCisgICAgICAgIFdlIGVtcGly aWNhbGx5IGtub3cgdGhhdCB0aGUgcHJldmlvdXMgc2l6ZS1jbGFzcyBzZXF1ZW5jZSB3b3JrZWQg d2VsbCBmb3IgYXJyYXktZXhwYW5zaW9uIGNhc2UuCisKKyAgICAgICAgICAgIHZhciBhcnJheSA9 IFtdOworICAgICAgICAgICAgd2hpbGUgKC4uLikKKyAgICAgICAgICAgICAgICBhcnJheS5wdXNo KHZhbHVlKTsKKworICAgICAgICBXZSBkbyBub3Qgd2FudCB0byBjaGFuZ2UgdGhlIHNpemUtY2xh c3Mgc2VxdWVuY2Ugd2l0aCBhbiB1bnJlbGF0ZWQgYnVnIGZpeC4gV2Ugc2hvdWxkIHN0YWJpbGl6 ZSB0aGUgc2l6ZS1jbGFzcyBzZXF1ZW5jZSBhbmQgc2VsZWN0IHRoZSBvbmUKKyAgICAgICAgd2hp Y2ggZW1waXJpY2FsbHkgd29ya3Mgd2VsbC4gQW5kIHdlIHNob3VsZCBub3QgYm90aGVyIGFib3V0 IHRoZSBzaXplb2YoVW5saW5rZWRGdW5jdGlvbkNvZGVCbG9jaykgZXRjLgorCisgICAgICAgIFRo aXMgcGF0Y2ggZXhwbGljaXRseSBhZGRzIDI1NiBpbnRvIHRoZSBzaXplLWNsYXNzIHRvIG1ha2Ug dGhlIHNpemUtY2xhc3Mgc2VxdWVuY2UgdGhlIHRoaW5nIHdlIGtub3cgaXQgd29ya3Mgd2VsbC4K KworICAgICAgICBidWcgMjAxNjEzIGNoYW5nZWQgc2l6ZW9mKFVubGlua2VkRnVuY3Rpb25Db2Rl QmxvY2spIGFuZCBkcmFtYXRpY2FsbHkgY2hhbmdlcyB0aGUgYWxsb2NhdGlvbiBiZWhhdmlvciBv ZiBKU0MuCisgICAgICAgIFdlIHNob3VsZCBtYWtlIHRoZSBwcmV2aW91cyBhbGxvY2F0aW9uIHBh dHRlcm4gc3RhYmxlLgorCisgICAgICAgIEluc3RlYWQgb2YgYWRkaW5nIGBhZGQoc2l6ZW9mKFVu bGlua2VkRnVuY3Rpb25Db2RlQmxvY2spKTtgLCBhZGRpbmcgYGFkZCgyNTYpYCBleHBsaWNpdGx5 IHRvIG1ha2UgdGhlIHByZXZpb3VzIGJlaGF2aW9yIGFzIGEgYmFzZWxpbmUuCisKKyAgICAgICAg KiBoZWFwL01hcmtlZFNwYWNlLmNwcDoKKwogMjAxOS0wOS0wOSAgWXVzdWtlIFN1enVraSAgPHlz dXp1a2lAYXBwbGUuY29tPgogCiAgICAgICAgIFtKU0NdIENvZGVCbG9jazo6bV9jb25zdGFudFJl Z2lzdGVycyBzaG91bGQgYmUgZ3VhcmRlZCBieSBDb25jdXJyZW50SlNMb2NrIHdoZW4gVmVjdG9y IHJlYWxsb2NhdGUgbWVtb3J5CmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvaGVh cC9NYXJrZWRTcGFjZS5jcHAgYi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvaGVhcC9NYXJrZWRTcGFj ZS5jcHAKaW5kZXggY2EwZjEyNjcwOThkM2ExYmFmMWQyYmFkN2IxOWRhMmQ0NWEzM2QwMi4uMDQy OWQ3NTAzY2EwNTA0ZWU0YWEzZWE3ZmMxNDc4NWRlMmY5MWVjYiAxMDA2NDQKLS0tIGEvU291cmNl L0phdmFTY3JpcHRDb3JlL2hlYXAvTWFya2VkU3BhY2UuY3BwCisrKyBiL1NvdXJjZS9KYXZhU2Ny aXB0Q29yZS9oZWFwL01hcmtlZFNwYWNlLmNwcApAQCAtMTM2LDExICsxMzYsOSBAQCBjb25zdCBW ZWN0b3I8c2l6ZV90PiYgc2l6ZUNsYXNzZXMoKQogICAgICAgICAgICAgICAgIGFkZChiZXR0ZXJT aXplQ2xhc3MpOwogICAgICAgICAgICAgfQogCi0gICAgICAgICAgICAvLyBNYW51YWxseSBpbmpl Y3Qgc2l6ZSBjbGFzc2VzIGZvciBvYmplY3RzIHdlIGtub3cgd2lsbCBiZSBhbGxvY2F0ZWQgaW4g aGlnaCB2b2x1bWUuCi0gICAgICAgICAgICAvLyBGSVhNRTogQWxsIG9mIHRoZXNlIHRoaW5ncyBz aG91bGQgaGF2ZSBJc29TdWJzcGFjZXMuCi0gICAgICAgICAgICAvLyBodHRwczovL2J1Z3Mud2Vi a2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTc5ODc2Ci0gICAgICAgICAgICBhZGQoc2l6ZW9mKFVu bGlua2VkRnVuY3Rpb25Db2RlQmxvY2spKTsKLSAgICAgICAgICAgIGFkZChzaXplb2YoSlNTdHJp bmcpKTsKKyAgICAgICAgICAgIC8vIE1hbnVhbGx5IGFkZGluZyAyNTYgY2FzZSwgZW1waXJpY2Fp bGx5IGtub3cgdGhpcyBwcm9kdWNlcyBnb29kIHNlcXVlbmNlIG9mIHNpemUgY2xhc3Nlcy4KKyAg ICAgICAgICAgIC8vIEZJWE1FOiBSZXZpc2l0IHNpemUtY2xhc3Mgc2VxdWVuY2Ugd2l0aCByYXRp b25hbGUuCisgICAgICAgICAgICBhZGQoMjU2KTsKIAogICAgICAgICAgICAgewogICAgICAgICAg ICAgICAgIC8vIFNvcnQgYW5kIGRlZHVwbGljYXRlLgo=