70162 2011-10-14 17:51:56 -0700 Add deleteProperty to the MethodTable 2011-10-23 21:22:11 -0700 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 67690 1 mhahnenberg mhahnenberg ggaren sam webkit.review.bot oldest_to_newest 484441 0 mhahnenberg 2011-10-14 17:51:56 -0700 We now need to add both versions of deleteProperty to the MethodTable in ClassInfo in preparation from removing the virtual version of deleteProperty. 487814 1 111834 mhahnenberg 2011-10-20 13:15:30 -0700 Created attachment 111834 Patch 489046 2 sam 2011-10-23 12:04:35 -0700 Have you done any back of the napkin calculations on what the memory difference between C++ vtables and the custom method tables are? 489051 3 mhahnenberg 2011-10-23 12:18:24 -0700 (In reply to comment #2) > Have you done any back of the napkin calculations on what the memory difference between C++ vtables and the custom method tables are? I haven't, but my sense is that almost all objects share very few ClassInfos (and therefore MethodTables). However, memory savings aren't the only reason we're trying to get rid of C++ virtual methods; mostly we just want more freedom in how we lay out our objects. 489054 4 sam 2011-10-23 12:21:56 -0700 (In reply to comment #3) > (In reply to comment #2) > > Have you done any back of the napkin calculations on what the memory difference between C++ vtables and the custom method tables are? > > I haven't, but my sense is that almost all objects share very few ClassInfos (and therefore MethodTables). However, memory savings aren't the only reason we're trying to get rid of C++ virtual methods; mostly we just want more freedom in how we lay out our objects. I didn't think memory savings was the impetuous, I am just a bit worried about the potential for bloat, so if the new mechanism uses more memory than the old one, we should understand how much. 489055 5 111834 webkit.review.bot 2011-10-23 12:23:43 -0700 Comment on attachment 111834 Patch Rejecting attachment 111834 from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: Source/JavaScriptCore/runtime/ClassInfo.h Hunk #1 FAILED at 40. 1 out of 1 hunk FAILED -- saving rejects to file Source/JavaScriptCore/runtime/ClassInfo.h.rej patching file Source/JavaScriptCore/runtime/JSFunction.h Hunk #1 succeeded at 138 with fuzz 2 (offset 5 lines). Hunk #2 succeeded at 148 with fuzz 2 (offset 2 lines). patching file Source/WebCore/WebCore.exp.in Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--reviewer', u'Sam Weinig', u'--force']" exit_code: 1 Full output: http://queues.webkit.org/results/10199577 489066 6 ggaren 2011-10-23 14:14:45 -0700 > I didn't think memory savings was the impetuous, I am just a bit worried about the potential for bloat, so if the new mechanism uses more memory than the old one, we should understand how much. We used to have one vtable per JS-derived class. When we're done, we'll lose the vtables, but gain one MethodTable per JS-derived class. I believe this will be a pure wash. On Windows, one MethodTable per class will be a slight win, since Windows sometimes creates more than one vtable for the same class (e.g., when the class is used in two different DLLs). Using plain function pointers instead of the member function pointers found in a vtable may also be a slight win, since C++ compilers sometimes encode member function pointers as more than just one pointer in size, and sometimes produce more than one copy of a given virtual function for a given class (e.g., a non-virtual thunk that inlines its target). But I don't think many JSCell subclasses trigger those mechanisms. > my sense is that almost all objects share very few ClassInfos (and therefore MethodTables) This is true, but also true of vtables. I think Sam's real question was about the total size of all MethodTables for all classes, not just the runtime footprint of commonly used MethodTables. 489099 7 mhahnenberg 2011-10-23 18:45:25 -0700 Committed r98205: <http://trac.webkit.org/changeset/98205> 111834 2011-10-20 13:15:30 -0700 2011-10-23 12:23:43 -0700 Patch bug-70162-20111020131529.patch text/plain 6075 mhahnenberg U3VidmVyc2lvbiBSZXZpc2lvbjogOTgwMDIKZGlmZiAtLWdpdCBhL1NvdXJjZS9KYXZhU2NyaXB0 Q29yZS9DaGFuZ2VMb2cgYi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvQ2hhbmdlTG9nCmluZGV4IDYy YjA1NWM2MjQ1MjQ2YzIzYmRmZTFkNjY3NjBiZGNkNmEzMjJlNmYuLjFhZWY3YWIxNjgyYjcxMjg1 YTIxNzdiYTJkNWE0NDNhOGIwMzU0NGEgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9KYXZhU2NyaXB0Q29y ZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwpAQCAtMSw1 ICsxLDE4IEBACiAyMDExLTEwLTIwICBNYXJrIEhhaG5lbmJlcmcgIDxtaGFobmVuYmVyZ0BhcHBs ZS5jb20+CiAKKyAgICAgICAgQWRkIGRlbGV0ZVByb3BlcnR5IHRvIHRoZSBNZXRob2RUYWJsZQor ICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NzAxNjIKKwor ICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIEphdmFTY3Jp cHRDb3JlLmV4cDoKKyAgICAgICAgKiBydW50aW1lL0NsYXNzSW5mby5oOiBBZGRlZCBib3RoIHZl cnNpb25zIG9mIGRlbGV0ZVByb3BlcnR5IHRvIHRoZSBNZXRob2RUYWJsZS4KKyAgICAgICAgKiBy dW50aW1lL0pTRnVuY3Rpb24uaDogQ2hhbmdlZCBKU0Z1bmN0aW9uOjpkZWxldGVQcm9wZXJ0eSB0 byAKKyAgICAgICAgYmUgcHJvdGVjdGVkIHJhdGhlciB0aGFuIHByaXZhdGUgZm9yIHN1YmNsYXNz ZXMgd2hvIGRvbid0IHByb3ZpZGUgdGhlaXIgb3duCisgICAgICAgIGltcGxlbWVudGF0aW9uLgor CisyMDExLTEwLTIwICBNYXJrIEhhaG5lbmJlcmcgIDxtaGFobmVuYmVyZ0BhcHBsZS5jb20+CisK ICAgICAgICAgUmVuYW1lIHN0YXRpYyBkZWxldGVQcm9wZXJ0eSB0byBkZWxldGVQcm9wZXJ0eUJ5 SW5kZXgKICAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTcw MjU3CiAKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJD b3JlL0NoYW5nZUxvZwppbmRleCBjYjlmNTkzZjQwYjlhZDQ1ZmNhY2RjMTYwNjcwYjU1MzY5NjM5 MWZlLi41MWEwZjY3ZTc1NzQxNGE2NzgxNWM1NTJhNWYzYjFkMTc5YWJiZjQ5IDEwMDY0NAotLS0g YS9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9n CkBAIC0xLDUgKzEsMTYgQEAKIDIwMTEtMTAtMjAgIE1hcmsgSGFobmVuYmVyZyAgPG1oYWhuZW5i ZXJnQGFwcGxlLmNvbT4KIAorICAgICAgICBBZGQgZGVsZXRlUHJvcGVydHkgdG8gdGhlIE1ldGhv ZFRhYmxlCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD03 MDE2MgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIE5v IG5ldyB0ZXN0cy4KKworICAgICAgICAqIFdlYkNvcmUuZXhwLmluOgorCisyMDExLTEwLTIwICBN YXJrIEhhaG5lbmJlcmcgIDxtaGFobmVuYmVyZ0BhcHBsZS5jb20+CisKICAgICAgICAgUmVuYW1l IHN0YXRpYyBkZWxldGVQcm9wZXJ0eSB0byBkZWxldGVQcm9wZXJ0eUJ5SW5kZXgKICAgICAgICAg aHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTcwMjU3CiAKZGlmZiAtLWdp dCBhL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9KYXZhU2NyaXB0Q29yZS5leHAgYi9Tb3VyY2UvSmF2 YVNjcmlwdENvcmUvSmF2YVNjcmlwdENvcmUuZXhwCmluZGV4IDllNmI0ZmJhY2I3MDNlMjNhZjM5 MWU0NzUxMDg2NGExZjdmNmNkNDQuLjQ1ZWEzYTlhY2YxMDRkYjQxYjk3M2EyNDMyMzZhY2RmOGE5 NDZjZjcgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9KYXZhU2NyaXB0Q29yZS5l eHAKKysrIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0phdmFTY3JpcHRDb3JlLmV4cApAQCAtMzI2 LDYgKzMyNiw3IEBAIF9fWk4zSlNDOEpTT2JqZWN0MTdwdXRXaXRoQXR0cmlidXRlc0VQTlNfOUV4 ZWNTdGF0ZUVSS05TXzEwSWRlbnRpZmllckVOU183SlNWYWx1CiBfX1pOM0pTQzhKU09iamVjdDE3 cHV0V2l0aEF0dHJpYnV0ZXNFUE5TXzlFeGVjU3RhdGVFak5TXzdKU1ZhbHVlRWoKIF9fWk4zSlND OEpTT2JqZWN0MThnZXRPd25Qcm9wZXJ0eVNsb3RFUE5TXzZKU0NlbGxFUE5TXzlFeGVjU3RhdGVF alJOU18xMlByb3BlcnR5U2xvdEUKIF9fWk4zSlNDOEpTT2JqZWN0MTlnZXRPd25Qcm9wZXJ0eU5h bWVzRVBOU185RXhlY1N0YXRlRVJOU18xN1Byb3BlcnR5TmFtZUFycmF5RU5TXzE1RW51bWVyYXRp b25Nb2RlRQorX19aTjNKU0M4SlNPYmplY3QyMWRlbGV0ZVByb3BlcnR5QnlJbmRleEVQTlNfNkpT Q2VsbEVQTlNfOUV4ZWNTdGF0ZUVqCiBfX1pOM0pTQzhKU09iamVjdDIxZGVsZXRlUHJvcGVydHlW aXJ0dWFsRVBOU185RXhlY1N0YXRlRVJLTlNfMTBJZGVudGlmaWVyRQogX19aTjNKU0M4SlNPYmpl Y3QyMWRlbGV0ZVByb3BlcnR5VmlydHVhbEVQTlNfOUV4ZWNTdGF0ZUVqCiBfX1pOM0pTQzhKU09i amVjdDIxZ2V0UHJvcGVydHlEZXNjcmlwdG9yRVBOU185RXhlY1N0YXRlRVJLTlNfMTBJZGVudGlm aWVyRVJOU18xOFByb3BlcnR5RGVzY3JpcHRvckUKZGlmZiAtLWdpdCBhL1NvdXJjZS9KYXZhU2Ny aXB0Q29yZS9ydW50aW1lL0NsYXNzSW5mby5oIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL3J1bnRp bWUvQ2xhc3NJbmZvLmgKaW5kZXggODJlMTg4NmVjZDNhNzMyMDVhZDc1NzUyMzIyNzRlMjkzYTNl YmJkOS4uNmM2NTRkMGI5M2E1MGM5YzU5MzllMWYxNGQ1M2QwZWE3OTZkY2NmOCAxMDA2NDQKLS0t IGEvU291cmNlL0phdmFTY3JpcHRDb3JlL3J1bnRpbWUvQ2xhc3NJbmZvLmgKKysrIGIvU291cmNl L0phdmFTY3JpcHRDb3JlL3J1bnRpbWUvQ2xhc3NJbmZvLmgKQEAgLTQwLDEyICs0MCwyMCBAQCBu YW1lc3BhY2UgSlNDIHsKIAogICAgICAgICB0eXBlZGVmIENvbnN0cnVjdFR5cGUgKCpHZXRDb25z dHJ1Y3REYXRhRnVuY3Rpb25QdHIpKEpTQ2VsbCosIENvbnN0cnVjdERhdGEmKTsKICAgICAgICAg R2V0Q29uc3RydWN0RGF0YUZ1bmN0aW9uUHRyIGdldENvbnN0cnVjdERhdGE7CisKKyAgICAgICAg dHlwZWRlZiBib29sICgqRGVsZXRlUHJvcGVydHlGdW5jdGlvblB0cikoSlNDZWxsKiwgRXhlY1N0 YXRlKiwgY29uc3QgSWRlbnRpZmllciYpOworICAgICAgICBEZWxldGVQcm9wZXJ0eUZ1bmN0aW9u UHRyIGRlbGV0ZVByb3BlcnR5OworCisgICAgICAgIHR5cGVkZWYgYm9vbCAoKkRlbGV0ZVByb3Bl cnR5QnlJbmRleEZ1bmN0aW9uUHRyKShKU0NlbGwqLCBFeGVjU3RhdGUqLCB1bnNpZ25lZCk7Cisg ICAgICAgIERlbGV0ZVByb3BlcnR5QnlJbmRleEZ1bmN0aW9uUHRyIGRlbGV0ZVByb3BlcnR5QnlJ bmRleDsKICAgICB9OwogCiAjZGVmaW5lIENSRUFURV9NRVRIT0RfVEFCTEUoQ2xhc3NOYW1lKSB7 IFwKICAgICAgICAgJkNsYXNzTmFtZTo6dmlzaXRDaGlsZHJlbiwgXAogICAgICAgICAmQ2xhc3NO YW1lOjpnZXRDYWxsRGF0YSwgXAogICAgICAgICAmQ2xhc3NOYW1lOjpnZXRDb25zdHJ1Y3REYXRh LCBcCisgICAgICAgICZDbGFzc05hbWU6OmRlbGV0ZVByb3BlcnR5LCBcCisgICAgICAgICZDbGFz c05hbWU6OmRlbGV0ZVByb3BlcnR5QnlJbmRleCwgXAogICAgIH0sIFwKICAgICBzaXplb2YoQ2xh c3NOYW1lKQogCmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvcnVudGltZS9KU0Z1 bmN0aW9uLmggYi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvcnVudGltZS9KU0Z1bmN0aW9uLmgKaW5k ZXggMjdmYTZmNWY2NDc0ZTA0MDJkN2I4YmJhMGU4NTI3ODllYWU0YjE5Yi4uMTA5Njk1MmQwZjA2 NWY2ZTRjNTY5MjIzZjEzNDkzZDZkOGYwNjYzNCAxMDA2NDQKLS0tIGEvU291cmNlL0phdmFTY3Jp cHRDb3JlL3J1bnRpbWUvSlNGdW5jdGlvbi5oCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9y dW50aW1lL0pTRnVuY3Rpb24uaApAQCAtMTMzLDYgKzEzMyw5IEBAIG5hbWVzcGFjZSBKU0Mgewog ICAgICAgICB2aXJ0dWFsIGJvb2wgZ2V0T3duUHJvcGVydHlEZXNjcmlwdG9yKEV4ZWNTdGF0ZSos IGNvbnN0IElkZW50aWZpZXImLCBQcm9wZXJ0eURlc2NyaXB0b3ImKTsKICAgICAgICAgdmlydHVh bCB2b2lkIGdldE93blByb3BlcnR5TmFtZXMoRXhlY1N0YXRlKiwgUHJvcGVydHlOYW1lQXJyYXkm LCBFbnVtZXJhdGlvbk1vZGUgPSBFeGNsdWRlRG9udEVudW1Qcm9wZXJ0aWVzKTsKIAorICAgICAg ICB2aXJ0dWFsIGJvb2wgZGVsZXRlUHJvcGVydHlWaXJ0dWFsKEV4ZWNTdGF0ZSosIGNvbnN0IElk ZW50aWZpZXImIHByb3BlcnR5TmFtZSk7CisgICAgICAgIHN0YXRpYyBib29sIGRlbGV0ZVByb3Bl cnR5KEpTQ2VsbCosIEV4ZWNTdGF0ZSosIGNvbnN0IElkZW50aWZpZXImIHByb3BlcnR5TmFtZSk7 CisKICAgICAgICAgc3RhdGljIHZvaWQgdmlzaXRDaGlsZHJlbihKU0NlbGwqLCBTbG90VmlzaXRv ciYpOwogCiAgICAgcHJpdmF0ZToKQEAgLTE0Myw5ICsxNDYsNiBAQCBuYW1lc3BhY2UgSlNDIHsK ICAgICAgICAgdmlydHVhbCB2b2lkIHB1dFZpcnR1YWwoRXhlY1N0YXRlKiwgY29uc3QgSWRlbnRp ZmllciYgcHJvcGVydHlOYW1lLCBKU1ZhbHVlLCBQdXRQcm9wZXJ0eVNsb3QmKTsKICAgICAgICAg c3RhdGljIHZvaWQgcHV0KEpTQ2VsbCosIEV4ZWNTdGF0ZSosIGNvbnN0IElkZW50aWZpZXImIHBy b3BlcnR5TmFtZSwgSlNWYWx1ZSwgUHV0UHJvcGVydHlTbG90Jik7CiAKLSAgICAgICAgdmlydHVh bCBib29sIGRlbGV0ZVByb3BlcnR5VmlydHVhbChFeGVjU3RhdGUqLCBjb25zdCBJZGVudGlmaWVy JiBwcm9wZXJ0eU5hbWUpOwotICAgICAgICBzdGF0aWMgYm9vbCBkZWxldGVQcm9wZXJ0eShKU0Nl bGwqLCBFeGVjU3RhdGUqLCBjb25zdCBJZGVudGlmaWVyJiBwcm9wZXJ0eU5hbWUpOwotCiAgICAg ICAgIHN0YXRpYyBKU1ZhbHVlIGFyZ3VtZW50c0dldHRlcihFeGVjU3RhdGUqLCBKU1ZhbHVlLCBj b25zdCBJZGVudGlmaWVyJik7CiAgICAgICAgIHN0YXRpYyBKU1ZhbHVlIGNhbGxlckdldHRlcihF eGVjU3RhdGUqLCBKU1ZhbHVlLCBjb25zdCBJZGVudGlmaWVyJik7CiAgICAgICAgIHN0YXRpYyBK U1ZhbHVlIGxlbmd0aEdldHRlcihFeGVjU3RhdGUqLCBKU1ZhbHVlLCBjb25zdCBJZGVudGlmaWVy Jik7CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9XZWJDb3JlLmV4cC5pbiBiL1NvdXJjZS9X ZWJDb3JlL1dlYkNvcmUuZXhwLmluCmluZGV4IDJkZTJlNjcwYjVmMWY2MDQ2M2YyOWFhNDkwOGI4 MmE3Y2Y4YTJmNGIuLjNjMTc4ZTc2ODBhMzdiY2EyZDIwZTUzOThmMzRiMjAyMDIzMDA2NGIgMTAw NjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL1dlYkNvcmUuZXhwLmluCisrKyBiL1NvdXJjZS9XZWJD b3JlL1dlYkNvcmUuZXhwLmluCkBAIC0xNjk5LDYgKzE2OTksNyBAQCBfX1pOM0pTQzEzUnVudGlt ZU1ldGhvZEMyRVBOU18xNEpTR2xvYmFsT2JqZWN0RVBOU185U3RydWN0dXJlRVJOM1dURjZWZWN0 b3JJUE5TXwogX19aTjNKU0M4QmluZGluZ3MxMFJvb3RPYmplY3REMUV2CiBfX1pOM0pTQzhCaW5k aW5nczEzUnVudGltZU9iamVjdDEwcHV0VmlydHVhbEVQTlNfOUV4ZWNTdGF0ZUVSS05TXzEwSWRl bnRpZmllckVOU183SlNWYWx1ZUVSTlNfMTVQdXRQcm9wZXJ0eVNsb3RFCiBfX1pOM0pTQzhCaW5k aW5nczEzUnVudGltZU9iamVjdDExZ2V0Q2FsbERhdGFFUE5TXzZKU0NlbGxFUk5TXzhDYWxsRGF0 YUUKK19fWk4zSlNDOEJpbmRpbmdzMTNSdW50aW1lT2JqZWN0MTRkZWxldGVQcm9wZXJ0eUVQTlNf NkpTQ2VsbEVQTlNfOUV4ZWNTdGF0ZUVSS05TXzEwSWRlbnRpZmllckUKIF9fWk4zSlNDOEJpbmRp bmdzMTNSdW50aW1lT2JqZWN0MTRmaW5pc2hDcmVhdGlvbkVQTlNfMTRKU0dsb2JhbE9iamVjdEUK IF9fWk4zSlNDOEJpbmRpbmdzMTNSdW50aW1lT2JqZWN0MTZnZXRDb25zdHJ1Y3REYXRhRVBOU182 SlNDZWxsRVJOU18xM0NvbnN0cnVjdERhdGFFCiBfX1pOM0pTQzhCaW5kaW5nczEzUnVudGltZU9i amVjdDE4Z2V0T3duUHJvcGVydHlTbG90RVBOU182SlNDZWxsRVBOU185RXhlY1N0YXRlRVJLTlNf MTBJZGVudGlmaWVyRVJOU18xMlByb3BlcnR5U2xvdEUK