188945 2018-08-25 06:57:30 -0700 Shrink size of HTMLCollection 2018-08-30 03:17:42 -0700 1 1 1 Unclassified WebKit New Bugs WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 ysuzuki ysuzuki cdumez darin esprehn+autocc ews-watchlist gyuyoung.kim rniwa simon.fraser webkit-bug-importer oldest_to_newest 1453675 0 ysuzuki 2018-08-25 06:57:30 -0700 Shrink size of HTMLCollection 1453676 1 348077 ysuzuki 2018-08-25 06:57:53 -0700 Created attachment 348077 Patch 1453698 2 348077 darin 2018-08-25 17:18:59 -0700 Comment on attachment 348077 Patch I wish there is something we could do to make sure people don’t accidentally undo this when making other changes in the future. 1453830 3 ysuzuki 2018-08-27 01:52:02 -0700 (In reply to Darin Adler from comment #2) > Comment on attachment 348077 [details] > Patch > > I wish there is something we could do to make sure people don’t accidentally > undo this when making other changes in the future. I think the only reasonable way we have right now is adding `static_assert(sizeof(HTMLCollection) <= 40, "")`. But I don't think it is good since the size of HTMLCollection is only the part of the problem. The rather important thing is HTMLCollection had many paddings. The size can increase due to reasonable reasons. But increasing paddings should be avoided. So it should be caught. The ideal way would be having a test using dump-class-layout that ensures the padding rate is small in some important classes. But this is not achievable without tooling and test infrastructure support. So, in the meantime, I'll land this patch as is. Simon, do you have any idea about introducing such a test (described above) into WebKit tests? 1453831 4 ysuzuki 2018-08-27 01:52:46 -0700 Committed r235357: <https://trac.webkit.org/changeset/235357> 1453832 5 webkit-bug-importer 2018-08-27 01:53:30 -0700 <rdar://problem/43746564> 1453912 6 simon.fraser 2018-08-27 09:28:38 -0700 (In reply to Yusuke Suzuki from comment #3) > (In reply to Darin Adler from comment #2) > > Comment on attachment 348077 [details] > > Patch > > > Simon, do you have any idea about introducing such a test (described above) > into WebKit tests? We do have testing of dump-class-layout in webkitpy tests, so it wouldn't be hard to rig some up somehow. 1453918 7 348077 simon.fraser 2018-08-27 09:32:16 -0700 Comment on attachment 348077 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=348077&action=review > Source/WebCore/html/HTMLCollection.h:107 > const unsigned m_collectionType : 5; > const unsigned m_invalidationType : 4; Would be nice to see comments say what the enum type is. 1455190 8 ysuzuki 2018-08-30 03:17:42 -0700 Committed r235497: <https://trac.webkit.org/changeset/235497> 348077 2018-08-25 06:57:53 -0700 2018-08-25 17:18:59 -0700 Patch bug-188945-20180825225752.patch text/plain 2640 ysuzuki U3VidmVyc2lvbiBSZXZpc2lvbjogMjM1MzM4CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggMzAyYzk0MmE4Y2I3ZTQz ZDVkZmM0NmQ1Y2JiMTg5NWU3OWI0YjE3Ni4uZjZjYTg4MjdmODIwMzQ1MWRmNWM3ZTVlMDRlZmU3 ODM2NTIwZWE3NSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE4IEBACisyMDE4LTA4LTI1ICBZdXN1 a2UgU3V6dWtpICA8eXVzdWtlc3V6dWtpQHNsb3dzdGFydC5vcmc+CisKKyAgICAgICAgU2hyaW5r IHNpemUgb2YgSFRNTENvbGxlY3Rpb24KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcv c2hvd19idWcuY2dpP2lkPTE4ODk0NQorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09Q UyEpLgorCisgICAgICAgIFNocmluayB0aGUgc2l6ZSBvZiBIVE1MQ29sbGVjdGlvbiBieSByZW9y ZGVyaW5nIG1lbWJlcnMuCisKKyAgICAgICAgTm8gYmVoYXZpb3IgY2hhbmdlLgorCisgICAgICAg ICogaHRtbC9IVE1MQ29sbGVjdGlvbi5jcHA6CisgICAgICAgIChXZWJDb3JlOjpIVE1MQ29sbGVj dGlvbjo6SFRNTENvbGxlY3Rpb24pOgorICAgICAgICAqIGh0bWwvSFRNTENvbGxlY3Rpb24uaDoK KwogMjAxOC0wOC0yNCAgUnlvc3VrZSBOaXdhICA8cm5pd2FAd2Via2l0Lm9yZz4KIAogICAgICAg ICBDbGljayBldmVudCBmcm9tIGNsaWNrKCkgaXMgbm90IGNvbXBvc2VkCmRpZmYgLS1naXQgYS9T b3VyY2UvV2ViQ29yZS9odG1sL0hUTUxDb2xsZWN0aW9uLmNwcCBiL1NvdXJjZS9XZWJDb3JlL2h0 bWwvSFRNTENvbGxlY3Rpb24uY3BwCmluZGV4IDVmNTk2YTMyZWM2MjQyMTNmYTdhYjQ0MmEyMWI4 Njg2MmFmN2FhY2YuLjNlMzRhMzZkMmI4ZjRhYWNiMzQzMjA3YmUwNzc4MDZkMWE5NDg0NDQgMTAw NjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL2h0bWwvSFRNTENvbGxlY3Rpb24uY3BwCisrKyBiL1Nv dXJjZS9XZWJDb3JlL2h0bWwvSFRNTENvbGxlY3Rpb24uY3BwCkBAIC0xMDgsMTAgKzEwOCwxMCBA QCBzdGF0aWMgTm9kZUxpc3RJbnZhbGlkYXRpb25UeXBlIGludmFsaWRhdGlvblR5cGVFeGNsdWRp bmdJZEFuZE5hbWVBdHRyaWJ1dGVzKENvbAogfQogCiBIVE1MQ29sbGVjdGlvbjo6SFRNTENvbGxl Y3Rpb24oQ29udGFpbmVyTm9kZSYgb3duZXJOb2RlLCBDb2xsZWN0aW9uVHlwZSB0eXBlKQotICAg IDogbV9vd25lck5vZGUob3duZXJOb2RlKQotICAgICwgbV9jb2xsZWN0aW9uVHlwZSh0eXBlKQor ICAgIDogbV9jb2xsZWN0aW9uVHlwZSh0eXBlKQogICAgICwgbV9pbnZhbGlkYXRpb25UeXBlKGlu dmFsaWRhdGlvblR5cGVFeGNsdWRpbmdJZEFuZE5hbWVBdHRyaWJ1dGVzKHR5cGUpKQogICAgICwg bV9yb290VHlwZShyb290VHlwZUZyb21Db2xsZWN0aW9uVHlwZSh0eXBlKSkKKyAgICAsIG1fb3du ZXJOb2RlKG93bmVyTm9kZSkKIHsKICAgICBBU1NFUlQobV9yb290VHlwZSA9PSBzdGF0aWNfY2Fz dDx1bnNpZ25lZD4ocm9vdFR5cGVGcm9tQ29sbGVjdGlvblR5cGUodHlwZSkpKTsKICAgICBBU1NF UlQobV9pbnZhbGlkYXRpb25UeXBlID09IHN0YXRpY19jYXN0PHVuc2lnbmVkPihpbnZhbGlkYXRp b25UeXBlRXhjbHVkaW5nSWRBbmROYW1lQXR0cmlidXRlcyh0eXBlKSkpOwpkaWZmIC0tZ2l0IGEv U291cmNlL1dlYkNvcmUvaHRtbC9IVE1MQ29sbGVjdGlvbi5oIGIvU291cmNlL1dlYkNvcmUvaHRt bC9IVE1MQ29sbGVjdGlvbi5oCmluZGV4IDc1YWM0MzQ1MTgwNzY2MDlhNDFhZjJhNGFmNzUxNzEw ZDRlYjZjZDAuLjQxZDc3ZmM3NzI0NmQ2MmNhZGI0NTQxMjY4MWViODcyZDBiMTJhNDAgMTAwNjQ0 Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL2h0bWwvSFRNTENvbGxlY3Rpb24uaAorKysgYi9Tb3VyY2Uv V2ViQ29yZS9odG1sL0hUTUxDb2xsZWN0aW9uLmgKQEAgLTEwMSwxNCArMTAxLDE1IEBAIGNsYXNz IEhUTUxDb2xsZWN0aW9uIDogcHVibGljIE5vZGVMaXN0IHsKICAgICBlbnVtIFJvb3RUeXBlIHsg SXNSb290ZWRBdE5vZGUsIElzUm9vdGVkQXREb2N1bWVudCB9OwogICAgIHN0YXRpYyBSb290VHlw ZSByb290VHlwZUZyb21Db2xsZWN0aW9uVHlwZShDb2xsZWN0aW9uVHlwZSk7CiAKLSAgICBSZWY8 Q29udGFpbmVyTm9kZT4gbV9vd25lck5vZGU7Ci0KLSAgICBtdXRhYmxlIHN0ZDo6dW5pcXVlX3B0 cjxDb2xsZWN0aW9uTmFtZWRFbGVtZW50Q2FjaGU+IG1fbmFtZWRFbGVtZW50Q2FjaGU7CiAgICAg bXV0YWJsZSBMb2NrIG1fbmFtZWRFbGVtZW50Q2FjaGVBc3NpZ25tZW50TG9jazsKLSAgICAKKwog ICAgIGNvbnN0IHVuc2lnbmVkIG1fY29sbGVjdGlvblR5cGUgOiA1OwogICAgIGNvbnN0IHVuc2ln bmVkIG1faW52YWxpZGF0aW9uVHlwZSA6IDQ7CiAgICAgY29uc3QgdW5zaWduZWQgbV9yb290VHlw ZSA6IDE7CisKKyAgICBSZWY8Q29udGFpbmVyTm9kZT4gbV9vd25lck5vZGU7CisKKyAgICBtdXRh YmxlIHN0ZDo6dW5pcXVlX3B0cjxDb2xsZWN0aW9uTmFtZWRFbGVtZW50Q2FjaGU+IG1fbmFtZWRF bGVtZW50Q2FjaGU7CiB9OwogCiBpbmxpbmUgQ29udGFpbmVyTm9kZSYgSFRNTENvbGxlY3Rpb246 OnJvb3ROb2RlKCkgY29uc3QK