182226 2018-01-28 15:52:58 -0800 LargeAllocation should do the same distancing as MarkedBlock 2018-01-28 21:49:35 -0800 1 1 1 Unclassified WebKit JavaScriptCore WebKit Nightly Build All All RESOLVED FIXED InRadar P2 Normal --- 181636 1 fpizlo fpizlo ggaren jfbastien keith_miller mark.lam msaboff rmorisset saam webkit-bug-importer ysuzuki oldest_to_newest 1393943 0 fpizlo 2018-01-28 15:52:58 -0800 Patch forthcoming. 1393944 1 webkit-bug-importer 2018-01-28 15:53:19 -0800 <rdar://problem/36968095> 1393945 2 332498 fpizlo 2018-01-28 16:10:57 -0800 Created attachment 332498 the patch 1393965 3 332498 saam 2018-01-28 20:58:10 -0800 Comment on attachment 332498 the patch View in context: https://bugs.webkit.org/attachment.cgi?id=332498&action=review > Source/JavaScriptCore/heap/LargeAllocation.cpp:41 > + size_t sizeIncludingDistancing = sizeBeforeDistancing + distancing; We don’t have any JIT code that needs to be updated to also do this? Do we not inline large allocations in the JIT? 1393967 4 fpizlo 2018-01-28 21:07:05 -0800 (In reply to Saam Barati from comment #3) > Comment on attachment 332498 [details] > the patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=332498&action=review > > > Source/JavaScriptCore/heap/LargeAllocation.cpp:41 > > + size_t sizeIncludingDistancing = sizeBeforeDistancing + distancing; > > We don’t have any JIT code that needs to be updated to also do this? Do we > not inline large allocations in the JIT? Only small allocations are inlined. Large allocations are very complicated to do. Here's a GC thinking trick: the cost of an allocation "hides behind" the cost of initializing every element. Because any not-totally-dumb allocation would be followed by at least an initialization of every element. Large allocations mean allocating at least 8000 bytes. Whether or not you inline a function call has infinitesimal cost compared to the cost of initializing 8000 bytes. Therefore, we don't inline large allocations. 1393968 5 fpizlo 2018-01-28 21:08:47 -0800 Landed in Landed in https://trac.webkit.org/changeset/227721/webkit 1393971 6 saam 2018-01-28 21:49:35 -0800 (In reply to Filip Pizlo from comment #4) > (In reply to Saam Barati from comment #3) > > Comment on attachment 332498 [details] > > the patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=332498&action=review > > > > > Source/JavaScriptCore/heap/LargeAllocation.cpp:41 > > > + size_t sizeIncludingDistancing = sizeBeforeDistancing + distancing; > > > > We don’t have any JIT code that needs to be updated to also do this? Do we > > not inline large allocations in the JIT? > > Only small allocations are inlined. Large allocations are very complicated > to do. > > Here's a GC thinking trick: the cost of an allocation "hides behind" the > cost of initializing every element. Because any not-totally-dumb allocation > would be followed by at least an initialization of every element. > > Large allocations mean allocating at least 8000 bytes. > > Whether or not you inline a function call has infinitesimal cost compared to > the cost of initializing 8000 bytes. > > Therefore, we don't inline large allocations. Makes sense. 332498 2018-01-28 16:10:57 -0800 2018-01-28 20:58:10 -0800 the patch blah.patch text/plain 6705 fpizlo SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMjI3NzIwKQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDM4IEBA CisyMDE4LTAxLTI4ICBGaWxpcCBQaXpsbyAgPGZwaXpsb0BhcHBsZS5jb20+CisKKyAgICAgICAg TGFyZ2VBbGxvY2F0aW9uIHNob3VsZCBkbyB0aGUgc2FtZSBkaXN0YW5jaW5nIGFzIE1hcmtlZEJs b2NrCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xODIy MjYKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBUaGlz IG1ha2VzIExhcmdlQWxsb2NhdGlvbiBkbyB0aGUgc2FtZSBleGFjdCBkaXN0YW5jaW5nIHRoYXQg TWFya2VkQmxvY2sgcHJvbWlzZXMgdG8gZG8uCisgICAgICAgIAorICAgICAgICBUbyBtYWtlIHRo YXQgcG9zc2libGUsIHRoaXMgcGF0Y2ggZmlyc3QgbWFrZXMgTWFya2VkQmxvY2sga25vdyBleGFj dGx5IGhvdyBtdWNoIGRpc3RhbmNpbmcgaXQKKyAgICAgICAgaXMgZG9pbmc6CisgICAgICAgIAor ICAgICAgICAtIEkndmUgcmF0aW9uYWxpemVkIHRoZSBwYXlsb2FkU2l6ZSBjYWxjdWxhdGlvbi4g SW4gcGFydGljdWxhciwgSSBtYWRlIE1hcmtlZFNwYWNlIHVzZSB0aGUKKyAgICAgICAgICBjYWxj dWxhdGlvbiBkb25lIGluIE1hcmtlZEJsb2NrLiBNYXJrZWRTcGFjZSB1c2VkIHRvIGRvIHRoZSBt YXRoIGEgZGlmZmVyZW50IHdheS4gVGhpcworICAgICAgICAgIGtlZXBzIHRoZSBvbGQgd2F5IGp1 c3QgZm9yIGEgc3RhdGljX2Fzc2VydC4KKyAgICAgICAgCisgICAgICAgIC0gVGhlIHByb21pc2Vk IGFtb3VudCBvZiBkaXN0YW5jaW5nIGlzIG5vdyBjb2RpZmllZCBpbiBIZWFwQ2VsbC5oIGFzCisg ICAgICAgICAgbWluaW11bURpc3RhbmNlQmV0d2VlbkNlbGxzRnJvbURpZmZlcmVudE9yaWdpbnMu IFdlIGFzc2VydCB0aGF0IHRoZSBmb290ZXIgc2l6ZSBpcyBhdCBsZWFzdAorICAgICAgICAgIGFz IGJpZyBhcyB0aGlzLiBJIGRpZG4ndCB3YW50IHRvIGp1c3QgdXNlIGZvb3RlciBzaXplIGZvciB0 aGlzIGNvbnN0YW50IGJlY2F1c2UgdGhlbiwgaWYKKyAgICAgICAgICB5b3UgaW5jcmVhc2VkIHRo ZSBzaXplIG9mIHRoZSBmb290ZXIsIHlvdSdkIGFsc28gYWRkIHBhZGRpbmcgdG8gZXZlcnkgbGFy Z2UgYWxsb2NhdGlvbi4KKyAgICAgICAgCisgICAgICAgIFRoZW4gdGhpcyBwYXRjaCBqdXN0IGFk ZHMgbWluaW11bURpc3RhbmNlQmV0d2VlbkNlbGxzRnJvbURpZmZlcmVudE9yaWdpbnMgdG8gZWFj aCBsYXJnZQorICAgICAgICBhbGxvY2F0aW9uLiBJdCBhbHNvIHplcm9lcyB0aGF0IHNsaWNlIG9m IG1lbW9yeSB0byBwcmV2ZW50IGFueSBpbmZvcm1hdGlvbiBsZWFrcyB0aGF0IHdheS4KKyAgICAg ICAgCisgICAgICAgIFRoaXMgaXMgcGVyZiBuZXV0cmFsLiBMYXJnZSBhbGxvY2F0aW9ucyBzdGFy dCBvdXQgYXQgfjgwMDAgYnl0ZXMuIFRoZSBhbW91bnQgb2YgcGFkZGluZyBpcworICAgICAgICB+ MzAwIGJ5dGVzLiBUaGF0J3MgMy43NSUgc3BhY2Ugb3ZlcmhlYWQgZm9yIG9iamVjdHMgdGhhdCBh cmUgfjgwMDAgYnl0ZXMsIHplcm8gb3ZlcmhlYWQgZm9yCisgICAgICAgIHNtYWxsZXIgb2JqZWN0 cywgYW5kIGRpbWluaXNoaW5nIG92ZXJoZWFkIGZvciBsYXJnZXIgb2JqZWN0cy4gV2UgYWxsb2Nh dGUgdmVyeSBmZXcgbGFyZ2UKKyAgICAgICAgb2JqZWN0cywgc28gd2Ugc2hvdWxkbid0IGhhdmUg YW55IHJlYWwgc3BhY2Ugb3ZlcmhlYWQgZnJvbSB0aGlzLgorCisgICAgICAgICogaGVhcC9IZWFw Q2VsbC5oOgorICAgICAgICAqIGhlYXAvTGFyZ2VBbGxvY2F0aW9uLmNwcDoKKyAgICAgICAgKEpT Qzo6TGFyZ2VBbGxvY2F0aW9uOjp0cnlDcmVhdGUpOgorICAgICAgICAqIGhlYXAvTWFya2VkQmxv Y2suaDoKKyAgICAgICAgKiBoZWFwL01hcmtlZFNwYWNlLmg6CisKIDIwMTgtMDEtMjcgIEZpbGlw IFBpemxvICA8ZnBpemxvQGFwcGxlLmNvbT4KIAogICAgICAgICBNYWtlIE1hcmtlZEJsb2NrOjpG b290ZXIgYmlnZ2VyCkluZGV4OiBTb3VyY2UvSmF2YVNjcmlwdENvcmUvaGVhcC9IZWFwQ2VsbC5o Cj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT0KLS0tIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9oZWFwL0hlYXBDZWxsLmgJKHJl dmlzaW9uIDIyNzcyMCkKKysrIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9oZWFwL0hlYXBDZWxsLmgJ KHdvcmtpbmcgY29weSkKQEAgLTEsNSArMSw1IEBACiAvKgotICogQ29weXJpZ2h0IChDKSAyMDE2 LTIwMTcgQXBwbGUgSW5jLiBBbGwgcmlnaHRzIHJlc2VydmVkLgorICogQ29weXJpZ2h0IChDKSAy MDE2LTIwMTggQXBwbGUgSW5jLiBBbGwgcmlnaHRzIHJlc2VydmVkLgogICoKICAqIFJlZGlzdHJp YnV0aW9uIGFuZCB1c2UgaW4gc291cmNlIGFuZCBiaW5hcnkgZm9ybXMsIHdpdGggb3Igd2l0aG91 dAogICogbW9kaWZpY2F0aW9uLCBhcmUgcGVybWl0dGVkIHByb3ZpZGVkIHRoYXQgdGhlIGZvbGxv d2luZyBjb25kaXRpb25zCkBAIC0zNyw2ICszNyw4IEBAIGNsYXNzIFN1YnNwYWNlOwogY2xhc3Mg Vk07CiBzdHJ1Y3QgQ2VsbEF0dHJpYnV0ZXM7CiAKK3N0YXRpYyBjb25zdGV4cHIgdW5zaWduZWQg bWluaW11bURpc3RhbmNlQmV0d2VlbkNlbGxzRnJvbURpZmZlcmVudE9yaWdpbnMgPSBzaXplb2Yo dm9pZCopID09IDggPyAzMDQgOiAyODg7CisKIGNsYXNzIEhlYXBDZWxsIHsKIHB1YmxpYzoKICAg ICBlbnVtIEtpbmQgOiBpbnQ4X3QgewpJbmRleDogU291cmNlL0phdmFTY3JpcHRDb3JlL2hlYXAv TGFyZ2VBbGxvY2F0aW9uLmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvSmF2YVNjcmlwdENvcmUv aGVhcC9MYXJnZUFsbG9jYXRpb24uY3BwCShyZXZpc2lvbiAyMjc3MjApCisrKyBTb3VyY2UvSmF2 YVNjcmlwdENvcmUvaGVhcC9MYXJnZUFsbG9jYXRpb24uY3BwCSh3b3JraW5nIGNvcHkpCkBAIC0x LDUgKzEsNSBAQAogLyoKLSAqIENvcHlyaWdodCAoQykgMjAxNi0yMDE3IEFwcGxlIEluYy4gQWxs IHJpZ2h0cyByZXNlcnZlZC4KKyAqIENvcHlyaWdodCAoQykgMjAxNi0yMDE4IEFwcGxlIEluYy4g QWxsIHJpZ2h0cyByZXNlcnZlZC4KICAqCiAgKiBSZWRpc3RyaWJ1dGlvbiBhbmQgdXNlIGluIHNv dXJjZSBhbmQgYmluYXJ5IGZvcm1zLCB3aXRoIG9yIHdpdGhvdXQKICAqIG1vZGlmaWNhdGlvbiwg YXJlIHBlcm1pdHRlZCBwcm92aWRlZCB0aGF0IHRoZSBmb2xsb3dpbmcgY29uZGl0aW9ucwpAQCAt MzUsOSArMzUsMTggQEAgbmFtZXNwYWNlIEpTQyB7CiAKIExhcmdlQWxsb2NhdGlvbiogTGFyZ2VB bGxvY2F0aW9uOjp0cnlDcmVhdGUoSGVhcCYgaGVhcCwgc2l6ZV90IHNpemUsIFN1YnNwYWNlKiBz dWJzcGFjZSkKIHsKLSAgICB2b2lkKiBzcGFjZSA9IHN1YnNwYWNlLT5hbGlnbmVkTWVtb3J5QWxs b2NhdG9yKCktPnRyeUFsbG9jYXRlQWxpZ25lZE1lbW9yeShhbGlnbm1lbnQsIGhlYWRlclNpemUo KSArIHNpemUpOworICAgIC8vIFRoaXMgaW5jbHVkZXMgcGFkZGluZyBhdCB0aGUgZW5kIG9mIHRo ZSBhbGxvY2F0aW9uIHRvIG1haW50YWluIHRoZSBkaXN0YW5jaW5nIGNvbnN0cmFpbnQuCisgICAg Y29uc3RleHByIHNpemVfdCBkaXN0YW5jaW5nID0gbWluaW11bURpc3RhbmNlQmV0d2VlbkNlbGxz RnJvbURpZmZlcmVudE9yaWdpbnM7CisgICAgc2l6ZV90IHNpemVCZWZvcmVEaXN0YW5jaW5nID0g aGVhZGVyU2l6ZSgpICsgc2l6ZTsKKyAgICBzaXplX3Qgc2l6ZUluY2x1ZGluZ0Rpc3RhbmNpbmcg PSBzaXplQmVmb3JlRGlzdGFuY2luZyArIGRpc3RhbmNpbmc7CisgICAgCisgICAgdm9pZCogc3Bh Y2UgPSBzdWJzcGFjZS0+YWxpZ25lZE1lbW9yeUFsbG9jYXRvcigpLT50cnlBbGxvY2F0ZUFsaWdu ZWRNZW1vcnkoYWxpZ25tZW50LCBzaXplSW5jbHVkaW5nRGlzdGFuY2luZyk7CiAgICAgaWYgKCFz cGFjZSkKICAgICAgICAgcmV0dXJuIG51bGxwdHI7CisgICAgCisgICAgLy8gTWFrZSBzdXJlIHRo YXQgdGhlIHBhZGRpbmcgZG9lcyBub3QgY29udGFpbiB1c2VmdWwgdGhpbmdzLgorICAgIG1lbXNl dChzdGF0aWNfY2FzdDxjaGFyKj4oc3BhY2UpICsgc2l6ZUJlZm9yZURpc3RhbmNpbmcsIDAsIGRp c3RhbmNpbmcpOworICAgIAogICAgIGlmIChzY3JpYmJsZUZyZWVDZWxscygpKQogICAgICAgICBz Y3JpYmJsZShzcGFjZSwgc2l6ZSk7CiAgICAgcmV0dXJuIG5ldyAoTm90TnVsbCwgc3BhY2UpIExh cmdlQWxsb2NhdGlvbihoZWFwLCBzaXplLCBzdWJzcGFjZSk7CkluZGV4OiBTb3VyY2UvSmF2YVNj cmlwdENvcmUvaGVhcC9NYXJrZWRCbG9jay5oCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9KYXZhU2Ny aXB0Q29yZS9oZWFwL01hcmtlZEJsb2NrLmgJKHJldmlzaW9uIDIyNzcyMCkKKysrIFNvdXJjZS9K YXZhU2NyaXB0Q29yZS9oZWFwL01hcmtlZEJsb2NrLmgJKHdvcmtpbmcgY29weSkKQEAgLTI4NSwx NCArMjg1LDE5IEBAIHB1YmxpYzoKICAgICAgICAgQml0bWFwPGF0b21zUGVyQmxvY2s+IG1fbWFy a3M7CiAgICAgICAgIEJpdG1hcDxhdG9tc1BlckJsb2NrPiBtX25ld2x5QWxsb2NhdGVkOwogICAg IH07Ci0gICAgICAgIAorICAgIAogcHJpdmF0ZTogICAgCiAgICAgRm9vdGVyJiBmb290ZXIoKTsK ICAgICBjb25zdCBGb290ZXImIGZvb3RlcigpIGNvbnN0OwogCiBwdWJsaWM6CiAgICAgc3RhdGlj IGNvbnN0ZXhwciBzaXplX3QgZW5kQXRvbSA9IChibG9ja1NpemUgLSBzaXplb2YoRm9vdGVyKSkg LyBhdG9tU2l6ZTsKKyAgICBzdGF0aWMgY29uc3RleHByIHNpemVfdCBwYXlsb2FkU2l6ZSA9IGVu ZEF0b20gKiBhdG9tU2l6ZTsKKyAgICBzdGF0aWMgY29uc3RleHByIHNpemVfdCBmb290ZXJTaXpl ID0gYmxvY2tTaXplIC0gcGF5bG9hZFNpemU7CiAKKyAgICBzdGF0aWNfYXNzZXJ0KHBheWxvYWRT aXplID09ICgoYmxvY2tTaXplIC0gc2l6ZW9mKE1hcmtlZEJsb2NrOjpGb290ZXIpKSAmIH4oYXRv bVNpemUgLSAxKSksICJQYXlsb2FkIHNpemUgY29tcHV0ZWQgdGhlIGFsdGVybmF0ZSB3YXkgc2hv dWxkIGdpdmUgdGhlIHNhbWUgcmVzdWx0Iik7CisgICAgc3RhdGljX2Fzc2VydChmb290ZXJTaXpl ID49IG1pbmltdW1EaXN0YW5jZUJldHdlZW5DZWxsc0Zyb21EaWZmZXJlbnRPcmlnaW5zLCAiRm9v dGVyIGlzIG5vdCBiaWcgZW5vdWdoIHRvIGNyZWF0ZSB0aGUgbmVjZXNzYXJ5IGRpc3RhbmNlIGJl dHdlZW4gb2JqZWN0cyBmcm9tIGRpZmZlcmVudCBvcmlnaW5zIik7CisgICAgCiAgICAgc3RhdGlj IE1hcmtlZEJsb2NrOjpIYW5kbGUqIHRyeUNyZWF0ZShIZWFwJiwgQWxpZ25lZE1lbW9yeUFsbG9j YXRvciopOwogICAgICAgICAKICAgICBIYW5kbGUmIGhhbmRsZSgpOwpJbmRleDogU291cmNlL0ph dmFTY3JpcHRDb3JlL2hlYXAvTWFya2VkU3BhY2UuaAo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvSmF2 YVNjcmlwdENvcmUvaGVhcC9NYXJrZWRTcGFjZS5oCShyZXZpc2lvbiAyMjc3MjApCisrKyBTb3Vy Y2UvSmF2YVNjcmlwdENvcmUvaGVhcC9NYXJrZWRTcGFjZS5oCSh3b3JraW5nIGNvcHkpCkBAIC01 NSw5ICs1NSw4IEBAIHB1YmxpYzoKICAgICAvLyBTaXplcyB1cCB0byB0aGlzIGFtb3VudCBnZXQg YSBzaXplIGNsYXNzIGZvciBlYWNoIHNpemUgc3RlcC4KICAgICBzdGF0aWMgY29uc3RleHByIHNp emVfdCBwcmVjaXNlQ3V0b2ZmID0gODA7CiAgICAgCi0gICAgLy8gVGhlIGFtb3VudCBvZiBhdmFp bGFibGUgcGF5bG9hZCBpbiBhIGJsb2NrIGlzIHRoZSBibG9jaydzIHNpemUgbWludXMgdGhlIGZv b3Rlci4gQnV0IHRoZQotICAgIC8vIGhlYWRlciBzaXplIG1pZ2h0IG5vdCBiZSBhdG9tIHNpemUg YWxpZ25lZCwgc28gd2Ugcm91bmQgZG93biB0aGUgcmVzdWx0IGFjY29yZGluZ2x5LgotICAgIHN0 YXRpYyBjb25zdGV4cHIgc2l6ZV90IGJsb2NrUGF5bG9hZCA9IChNYXJrZWRCbG9jazo6YmxvY2tT aXplIC0gc2l6ZW9mKE1hcmtlZEJsb2NrOjpGb290ZXIpKSAmIH4oTWFya2VkQmxvY2s6OmF0b21T aXplIC0gMSk7CisgICAgLy8gVGhlIGFtb3VudCBvZiBhdmFpbGFibGUgcGF5bG9hZCBpbiBhIGJs b2NrIGlzIHRoZSBibG9jaydzIHNpemUgbWludXMgdGhlIGZvb3Rlci4KKyAgICBzdGF0aWMgY29u c3RleHByIHNpemVfdCBibG9ja1BheWxvYWQgPSBNYXJrZWRCbG9jazo6cGF5bG9hZFNpemU7CiAg ICAgCiAgICAgLy8gVGhlIGxhcmdlc3QgY2VsbCB3ZSdyZSB3aWxsaW5nIHRvIGFsbG9jYXRlIGlu IGEgTWFya2VkQmxvY2sgdGhlICJub3JtYWwgd2F5IiAoaS5lLiB1c2luZyBzaXplCiAgICAgLy8g Y2xhc3NlcywgcmF0aGVyIHRoYW4gYSBsYXJnZSBhbGxvY2F0aW9uKSBpcyBoYWxmIHRoZSBzaXpl IG9mIHRoZSBwYXlsb2FkLCByb3VuZGVkIGRvd24uIFRoaXMK