170690 2017-04-10 13:42:42 -0700 WebAssembly: report Memory usage to GC 2017-05-16 00:10:02 -0700 1 1 1 Unclassified WebKit JavaScriptCore WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 170628 159775 1 jfbastien jfbastien commit-queue fpizlo jfbastien keith_miller mark.lam msaboff saam webkit-bug-importer oldest_to_newest 1296136 0 jfbastien 2017-04-10 13:42:42 -0700 As part of bug #170628 I noticed that we never report the memory allocated by WebAssembly.Memory to the GC. This change would do it: Wasm::PageCount JSWebAssemblyMemory::grow(VM& vm, ExecState* exec, uint32_t delt ... vm.heap.reportExtraMemoryAllocated(Wasm::PageCount(delta).bytes()); void JSWebAssemblyMemory::finishCreation(VM& vm) ... heap()->reportExtraMemoryAllocated(m_memory->size()); JSWebAssemblyMemory::visitChildren(JSCell* cell, SlotVisitor& visitor) ... visitor.reportExtraMemoryVisited(thisObject->memory().size()); I'm not sure it's the right thing to do though! The "initial" size seems like a good thing to report, but if we haven't actually backed it up with physical pages then it may severely over-estimate allocation and cause the GC to run too much. This change is significant enough that I'll punt it to its own patch, after bug #170628. 1304072 1 webkit-bug-importer 2017-05-03 09:54:31 -0700 <rdar://problem/31965310> 1307203 2 309699 jfbastien 2017-05-11 00:32:32 -0700 Created attachment 309699 patch I'm still not sure this is the right thing. We definitely should tell the GC about this memory but: 1. It's more memory than other allocations. Maybe that's the same as ArrayBuffer and doesn't matter? 2. We're not sure all pages are dirty, so we could be over-reporting if the developer over-allocates. We just don't know if they do. I think these don't matter, and we can adjust if they do, so this patch is probably right. I'd nonetheless would rather ask and get feedback on these points. 1309013 3 309699 keith_miller 2017-05-15 23:56:19 -0700 Comment on attachment 309699 patch r=me. 1309020 4 309699 commit-queue 2017-05-16 00:09:59 -0700 Comment on attachment 309699 patch Clearing flags on attachment: 309699 Committed r216911: <http://trac.webkit.org/changeset/216911> 1309021 5 commit-queue 2017-05-16 00:10:02 -0700 All reviewed patches have been landed. Closing bug. 309699 2017-05-11 00:32:32 -0700 2017-05-16 00:09:59 -0700 patch 0001-Report-WebAssembly-memory-usage-to-GC.patch text/plain 2864 jfbastien RnJvbSBmMmJlNDMwZTAzMmQyZDZlM2JkZTQ3OWRiNjViMGY4ZjFjN2IyYTY1IE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBKRiBCYXN0aWVuIDxqZmJhc3RpZW5AYXBwbGUuY29tPgpEYXRl OiBUaHUsIDExIE1heSAyMDE3IDAwOjI5OjQ4IC0wNzAwClN1YmplY3Q6IFtQQVRDSF0gUmVwb3J0 IFdlYkFzc2VtYmx5IG1lbW9yeSB1c2FnZSB0byBHQwoKLS0tCiBTb3VyY2UvSmF2YVNjcmlwdENv cmUvQ2hhbmdlTG9nICAgICAgICAgICAgICAgICAgICAgICB8IDEzICsrKysrKysrKysrKysKIFNv dXJjZS9KYXZhU2NyaXB0Q29yZS93YXNtL2pzL0pTV2ViQXNzZW1ibHlNZW1vcnkuY3BwIHwgIDkg KysrKystLS0tCiAyIGZpbGVzIGNoYW5nZWQsIDE4IGluc2VydGlvbnMoKyksIDQgZGVsZXRpb25z KC0pCgpkaWZmIC0tZ2l0IGEvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZyBiL1NvdXJj ZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKaW5kZXggMWIxNGJkZS4uMjU5OTI4NyAxMDA2NDQK LS0tIGEvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZworKysgYi9Tb3VyY2UvSmF2YVNj cmlwdENvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTYgQEAKKzIwMTctMDUtMTEgIEpGIEJhc3Rp ZW4gIDxqZmJhc3RpZW5AYXBwbGUuY29tPgorCisgICAgICAgIFdlYkFzc2VtYmx5OiByZXBvcnQg TWVtb3J5IHVzYWdlIHRvIEdDCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3df YnVnLmNnaT9pZD0xNzA2OTAKKyAgICAgICAgPHJkYXI6Ly9wcm9ibGVtLzMxOTY1MzEwPgorCisg ICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgICogd2FzbS9qcy9K U1dlYkFzc2VtYmx5TWVtb3J5LmNwcDoKKyAgICAgICAgKEpTQzo6SlNXZWJBc3NlbWJseU1lbW9y eTo6Z3Jvdyk6CisgICAgICAgIChKU0M6OkpTV2ViQXNzZW1ibHlNZW1vcnk6OmZpbmlzaENyZWF0 aW9uKToKKyAgICAgICAgKEpTQzo6SlNXZWJBc3NlbWJseU1lbW9yeTo6dmlzaXRDaGlsZHJlbik6 CisKIDIwMTctMDUtMDkgIFNhbSBXZWluaWcgIDxzYW1Ad2Via2l0Lm9yZz4KIAogICAgICAgICBS ZW1vdmUgc3VwcG9ydCBmb3IgbGVnYWN5IE5vdGlmaWNhdGlvbnMKZGlmZiAtLWdpdCBhL1NvdXJj ZS9KYXZhU2NyaXB0Q29yZS93YXNtL2pzL0pTV2ViQXNzZW1ibHlNZW1vcnkuY3BwIGIvU291cmNl L0phdmFTY3JpcHRDb3JlL3dhc20vanMvSlNXZWJBc3NlbWJseU1lbW9yeS5jcHAKaW5kZXggYzZk NzhjZi4uZDYwYjNiNyAxMDA2NDQKLS0tIGEvU291cmNlL0phdmFTY3JpcHRDb3JlL3dhc20vanMv SlNXZWJBc3NlbWJseU1lbW9yeS5jcHAKKysrIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL3dhc20v anMvSlNXZWJBc3NlbWJseU1lbW9yeS5jcHAKQEAgLTEsNSArMSw1IEBACiAvKgotICogQ29weXJp Z2h0IChDKSAyMDE2IEFwcGxlIEluYy4gQWxsIHJpZ2h0cyByZXNlcnZlZC4KKyAqIENvcHlyaWdo dCAoQykgMjAxNi0yMDE3IEFwcGxlIEluYy4gQWxsIHJpZ2h0cyByZXNlcnZlZC4KICAqCiAgKiBS ZWRpc3RyaWJ1dGlvbiBhbmQgdXNlIGluIHNvdXJjZSBhbmQgYmluYXJ5IGZvcm1zLCB3aXRoIG9y IHdpdGhvdXQKICAqIG1vZGlmaWNhdGlvbiwgYXJlIHBlcm1pdHRlZCBwcm92aWRlZCB0aGF0IHRo ZSBmb2xsb3dpbmcgY29uZGl0aW9ucwpAQCAtMTE3LDcgKzExNyw4IEBAIFdhc206OlBhZ2VDb3Vu dCBKU1dlYkFzc2VtYmx5TWVtb3J5Ojpncm93KFZNJiB2bSwgRXhlY1N0YXRlKiBleGVjLCB1aW50 MzJfdCBkZWx0CiAgICAgfQogCiAgICAgbWVtb3J5KCkuY2hlY2soKTsKLSAgICAvLyBGSVhNRSBT aG91bGQgd2UgcmVwb3J0IGV4dHJhIG1lbW9yeSB0byB0aGUgR0Mgb24gYWxsb2NhdGlvbiAvIGdy b3cgLyB2aXNpdD8gaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE3MDY5 MAorCisgICAgdm0uaGVhcC5yZXBvcnRFeHRyYU1lbW9yeUFsbG9jYXRlZChXYXNtOjpQYWdlQ291 bnQoZGVsdGEpLmJ5dGVzKCkpOwogICAgIHJldHVybiBvbGRQYWdlQ291bnQ7CiB9CiAKQEAgLTEy NSw3ICsxMjYsNyBAQCB2b2lkIEpTV2ViQXNzZW1ibHlNZW1vcnk6OmZpbmlzaENyZWF0aW9uKFZN JiB2bSkKIHsKICAgICBCYXNlOjpmaW5pc2hDcmVhdGlvbih2bSk7CiAgICAgQVNTRVJUKGluaGVy aXRzKHZtLCBpbmZvKCkpKTsKLSAgICAvLyBGSVhNRSBTaG91bGQgd2UgcmVwb3J0IGV4dHJhIG1l bW9yeSB0byB0aGUgR0Mgb24gYWxsb2NhdGlvbiAvIGdyb3cgLyB2aXNpdD8gaHR0cHM6Ly9idWdz LndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE3MDY5MAorICAgIGhlYXAoKS0+cmVwb3J0RXh0 cmFNZW1vcnlBbGxvY2F0ZWQobWVtb3J5KCkuc2l6ZSgpKTsKICAgICB2bS5oZWFwLnJlcG9ydFdl YkFzc2VtYmx5RmFzdE1lbW9yaWVzQWxsb2NhdGVkKDEpOwogfQogCkBAIC0xNDQsNyArMTQ1LDcg QEAgdm9pZCBKU1dlYkFzc2VtYmx5TWVtb3J5Ojp2aXNpdENoaWxkcmVuKEpTQ2VsbCogY2VsbCwg U2xvdFZpc2l0b3ImIHZpc2l0b3IpCiAKICAgICBCYXNlOjp2aXNpdENoaWxkcmVuKHRoaXNPYmpl Y3QsIHZpc2l0b3IpOwogICAgIHZpc2l0b3IuYXBwZW5kKHRoaXNPYmplY3QtPm1fYnVmZmVyV3Jh cHBlcik7Ci0gICAgLy8gRklYTUUgU2hvdWxkIHdlIHJlcG9ydCBleHRyYSBtZW1vcnkgdG8gdGhl IEdDIG9uIGFsbG9jYXRpb24gLyBncm93IC8gdmlzaXQ/IGh0dHBzOi8vYnVncy53ZWJraXQub3Jn L3Nob3dfYnVnLmNnaT9pZD0xNzA2OTAKKyAgICB2aXNpdG9yLnJlcG9ydEV4dHJhTWVtb3J5Vmlz aXRlZCh0aGlzT2JqZWN0LT5tZW1vcnkoKS5zaXplKCkpOwogfQogCiB9IC8vIG5hbWVzcGFjZSBK U0MKLS0gCjIuOS4zCgo=