187450 2018-07-08 10:59:43 -0700 Shrink WebCore::Pair 2018-07-09 12:25:28 -0700 1 1 1 Unclassified WebKit New Bugs WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 simon.fraser simon.fraser cdumez koivisto sam simon.fraser webkit-bug-importer ysuzuki zalan oldest_to_newest 1440251 0 simon.fraser 2018-07-08 10:59:43 -0700 Shrink WebCore::Pair 1440252 1 344550 simon.fraser 2018-07-08 11:01:34 -0700 Created attachment 344550 Patch 1440254 2 simon.fraser 2018-07-08 11:07:15 -0700 Before +0 < 40> Pair +0 < 8> __vtbl_ptr_type * _vptr +8 < 4> WTF::RefCounted<WebCore::Pair> WTF::RefCounted<WebCore::Pair> +8 < 4> WTF::RefCountedBase WTF::RefCountedBase +8 < 4> unsigned int m_refCount +12 < 4> <PADDING: 4 bytes> +16 < 8> WTF::RefPtr<WebCore::CSSPrimitiveValue, WTF::DumbPtrTraits<WebCore::CSSPrimitiveValue> > m_first +16 < 8> WTF::DumbPtrTraits<WebCore::CSSPrimitiveValue>::StorageType m_ptr +24 < 8> WTF::RefPtr<WebCore::CSSPrimitiveValue, WTF::DumbPtrTraits<WebCore::CSSPrimitiveValue> > m_second +24 < 8> WTF::DumbPtrTraits<WebCore::CSSPrimitiveValue>::StorageType m_ptr +32 < 4> WebCore::Pair::IdenticalValueEncoding m_encoding +36 < 4> <PADDING: 4 bytes> Total byte size: 40 Total pad bytes: 8 Padding percentage: 20.00 % After 24$ $ ./Tools/Scripts/dump-class-layout -c Release WebCore Pair +0 < 32> Pair +0 < 8> __vtbl_ptr_type * _vptr +8 < 4> WTF::RefCounted<WebCore::Pair> WTF::RefCounted<WebCore::Pair> +8 < 4> WTF::RefCountedBase WTF::RefCountedBase +8 < 4> unsigned int m_refCount +12 < 1> WebCore::Pair::IdenticalValueEncoding m_encoding +13 < 3> <PADDING: 3 bytes> +16 < 8> WTF::RefPtr<WebCore::CSSPrimitiveValue, WTF::DumbPtrTraits<WebCore::CSSPrimitiveValue> > m_first +16 < 8> WTF::DumbPtrTraits<WebCore::CSSPrimitiveValue>::StorageType m_ptr +24 < 8> WTF::RefPtr<WebCore::CSSPrimitiveValue, WTF::DumbPtrTraits<WebCore::CSSPrimitiveValue> > m_second +24 < 8> WTF::DumbPtrTraits<WebCore::CSSPrimitiveValue>::StorageType m_ptr Total byte size: 32 Total pad bytes: 3 Padding percentage: 9.38 % 1440255 3 ysuzuki 2018-07-08 11:48:57 -0700 (In reply to Simon Fraser (smfr) from comment #2) > Before > +0 < 40> Pair > +0 < 8> __vtbl_ptr_type * _vptr > +8 < 4> WTF::RefCounted<WebCore::Pair> WTF::RefCounted<WebCore::Pair> > +8 < 4> WTF::RefCountedBase WTF::RefCountedBase > +8 < 4> unsigned int m_refCount > +12 < 4> <PADDING: 4 bytes> > +16 < 8> WTF::RefPtr<WebCore::CSSPrimitiveValue, > WTF::DumbPtrTraits<WebCore::CSSPrimitiveValue> > m_first > +16 < 8> WTF::DumbPtrTraits<WebCore::CSSPrimitiveValue>::StorageType > m_ptr > +24 < 8> WTF::RefPtr<WebCore::CSSPrimitiveValue, > WTF::DumbPtrTraits<WebCore::CSSPrimitiveValue> > m_second > +24 < 8> WTF::DumbPtrTraits<WebCore::CSSPrimitiveValue>::StorageType > m_ptr > +32 < 4> WebCore::Pair::IdenticalValueEncoding m_encoding > +36 < 4> <PADDING: 4 bytes> > Total byte size: 40 > Total pad bytes: 8 > Padding percentage: 20.00 % > > > After > 24$ $ ./Tools/Scripts/dump-class-layout -c Release WebCore Pair > +0 < 32> Pair > +0 < 8> __vtbl_ptr_type * _vptr > +8 < 4> WTF::RefCounted<WebCore::Pair> WTF::RefCounted<WebCore::Pair> > +8 < 4> WTF::RefCountedBase WTF::RefCountedBase > +8 < 4> unsigned int m_refCount > +12 < 1> WebCore::Pair::IdenticalValueEncoding m_encoding > +13 < 3> <PADDING: 3 bytes> > +16 < 8> WTF::RefPtr<WebCore::CSSPrimitiveValue, > WTF::DumbPtrTraits<WebCore::CSSPrimitiveValue> > m_first > +16 < 8> WTF::DumbPtrTraits<WebCore::CSSPrimitiveValue>::StorageType > m_ptr > +24 < 8> WTF::RefPtr<WebCore::CSSPrimitiveValue, > WTF::DumbPtrTraits<WebCore::CSSPrimitiveValue> > m_second > +24 < 8> WTF::DumbPtrTraits<WebCore::CSSPrimitiveValue>::StorageType > m_ptr > Total byte size: 32 > Total pad bytes: 3 > Padding percentage: 9.38 % Pair is a final class. And it only inherits RefCounted<Pair>. So, if we remove `virtual ~Pair()`, I think we can drop vtable pointer further! 1440263 4 344550 sam 2018-07-08 14:42:55 -0700 Comment on attachment 344550 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=344550&action=review > Source/WebCore/ChangeLog:8 > + Move m_encoding to pick in with m_refCount, shrinking the class from 40 to 32 bytes. Do you mean 'pack' here? 1440396 5 344550 cdumez 2018-07-09 08:52:27 -0700 Comment on attachment 344550 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=344550&action=review > Source/WebCore/css/Pair.h:33 > class Pair final : public RefCounted<Pair> { I think Yusuke is right that we should drop the virtual destructor here. We do not need a vtable. 1440465 6 simon.fraser 2018-07-09 11:17:19 -0700 Will fix. 1440500 7 simon.fraser 2018-07-09 12:24:18 -0700 https://trac.webkit.org/changeset/233652/webkit 1440501 8 webkit-bug-importer 2018-07-09 12:25:28 -0700 <rdar://problem/41984422> 344550 2018-07-08 11:01:34 -0700 2018-07-09 08:52:27 -0700 Patch bug-187450-20180708110133.patch text/plain 2538 simon.fraser U3VidmVyc2lvbiBSZXZpc2lvbjogMjMzNjE2CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNmJjZjVkOGZhY2E3NTQ0 MTk5NjlkM2U2ZjQ0NTg5NTBjNjFkZTM3My4uZmQxNTFlMjVhZTQ2M2Q5NGJlODg0MGE5MTc5MzE5 YTlhY2UwZmUxMiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE3IEBACisyMDE4LTA3LTA4ICBTaW1v biBGcmFzZXIgIDxzaW1vbi5mcmFzZXJAYXBwbGUuY29tPgorCisgICAgICAgIFNocmluayBXZWJD b3JlOjpQYWlyCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9p ZD0xODc0NTAKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAg ICBNb3ZlIG1fZW5jb2RpbmcgdG8gcGljayBpbiB3aXRoIG1fcmVmQ291bnQsIHNocmlua2luZyB0 aGUgY2xhc3MgZnJvbSA0MCB0byAzMiBieXRlcy4KKyAgICAgICAgQWxzbyBtYWtlIHRoZSBlbnVt IHRha2Ugb25seSBhIGJ5dGUsIGluIGNhc2UgaXQgZ2V0cyB1c2VkIGVsc2V3aGVyZQorICAgICAg ICAKKyAgICAgICAgVGhlcmUgYXJlIGFib3V0IDUwMCBQYWlycyBvbiBueXRpbWVzLmNvbSwgc28g dGhpcyBtZW1vcnkgc2F2aW5nIGlzIG5vbi10cml2aWFsLgorCisgICAgICAgICogY3NzL1BhaXIu aDoKKwogMjAxOC0wNy0wNyAgU2ltb24gRnJhc2VyICA8c2ltb24uZnJhc2VyQGFwcGxlLmNvbT4K IAogICAgICAgICBPcHRpbWl6ZSBwYWNraW5nIG9mIFJvb3RJbmxpbmVCb3gKZGlmZiAtLWdpdCBh L1NvdXJjZS9XZWJDb3JlL2Nzcy9QYWlyLmggYi9Tb3VyY2UvV2ViQ29yZS9jc3MvUGFpci5oCmlu ZGV4IDU2YWQwZDJiMDViNTdjMmEyZDc2MjIzNDBjMmQzYjcyNjMzYTA4ZWUuLmFlNmMyZjI4OTM1 ZTVjOTQyNDYyODYxN2FlMTdhYTE1MmU4ZGQxZDAgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3Jl L2Nzcy9QYWlyLmgKKysrIGIvU291cmNlL1dlYkNvcmUvY3NzL1BhaXIuaApAQCAtMzIsNyArMzIs NyBAQCBjbGFzcyBDU1NQcmltaXRpdmVWYWx1ZTsKIC8vIGl0IChlbGltaW5hdGluZyBzb21lIGV4 dHJhIC13ZWJraXQtIGludGVybmFsIHByb3BlcnRpZXMpLgogY2xhc3MgUGFpciBmaW5hbCA6IHB1 YmxpYyBSZWZDb3VudGVkPFBhaXI+IHsKIHB1YmxpYzoKLSAgICBlbnVtIGNsYXNzIElkZW50aWNh bFZhbHVlRW5jb2RpbmcgeworICAgIGVudW0gY2xhc3MgSWRlbnRpY2FsVmFsdWVFbmNvZGluZyA6 IHVpbnQ4X3QgewogICAgICAgICBEb05vdENvYWxlc2NlLAogICAgICAgICBDb2FsZXNjZQogICAg IH07CkBAIC02MiwxMiArNjIsMjAgQEAgcHVibGljOgogICAgIGJvb2wgZXF1YWxzKGNvbnN0IFBh aXImIG90aGVyKSBjb25zdCB7IHJldHVybiBjb21wYXJlQ1NTVmFsdWVQdHIobV9maXJzdCwgb3Ro ZXIubV9maXJzdCkgJiYgY29tcGFyZUNTU1ZhbHVlUHRyKG1fc2Vjb25kLCBvdGhlci5tX3NlY29u ZCk7IH0KIAogcHJpdmF0ZToKLSAgICBQYWlyKFJlZlB0cjxDU1NQcmltaXRpdmVWYWx1ZT4mJiBm aXJzdCwgUmVmUHRyPENTU1ByaW1pdGl2ZVZhbHVlPiYmIHNlY29uZCkgOiBtX2ZpcnN0KFdURk1v dmUoZmlyc3QpKSwgbV9zZWNvbmQoV1RGTW92ZShzZWNvbmQpKSB7IH0KLSAgICBQYWlyKFJlZlB0 cjxDU1NQcmltaXRpdmVWYWx1ZT4mJiBmaXJzdCwgUmVmUHRyPENTU1ByaW1pdGl2ZVZhbHVlPiYm IHNlY29uZCwgSWRlbnRpY2FsVmFsdWVFbmNvZGluZyBlbmNvZGluZykgOiBtX2ZpcnN0KFdURk1v dmUoZmlyc3QpKSwgbV9zZWNvbmQoV1RGTW92ZShzZWNvbmQpKSwgbV9lbmNvZGluZyhlbmNvZGlu ZykgeyB9CisgICAgUGFpcihSZWZQdHI8Q1NTUHJpbWl0aXZlVmFsdWU+JiYgZmlyc3QsIFJlZlB0 cjxDU1NQcmltaXRpdmVWYWx1ZT4mJiBzZWNvbmQpCisgICAgICAgIDogbV9maXJzdChXVEZNb3Zl KGZpcnN0KSkKKyAgICAgICAgLCBtX3NlY29uZChXVEZNb3ZlKHNlY29uZCkpCisgICAgeyB9CiAK KyAgICBQYWlyKFJlZlB0cjxDU1NQcmltaXRpdmVWYWx1ZT4mJiBmaXJzdCwgUmVmUHRyPENTU1By aW1pdGl2ZVZhbHVlPiYmIHNlY29uZCwgSWRlbnRpY2FsVmFsdWVFbmNvZGluZyBlbmNvZGluZykK KyAgICAgICAgOiBtX2VuY29kaW5nKGVuY29kaW5nKQorICAgICAgICAsIG1fZmlyc3QoV1RGTW92 ZShmaXJzdCkpCisgICAgICAgICwgbV9zZWNvbmQoV1RGTW92ZShzZWNvbmQpKQorICAgIHsgfQor CisgICAgSWRlbnRpY2FsVmFsdWVFbmNvZGluZyBtX2VuY29kaW5nIHsgSWRlbnRpY2FsVmFsdWVF bmNvZGluZzo6Q29hbGVzY2UgfTsKICAgICBSZWZQdHI8Q1NTUHJpbWl0aXZlVmFsdWU+IG1fZmly c3Q7CiAgICAgUmVmUHRyPENTU1ByaW1pdGl2ZVZhbHVlPiBtX3NlY29uZDsKLSAgICBJZGVudGlj YWxWYWx1ZUVuY29kaW5nIG1fZW5jb2RpbmcgeyBJZGVudGljYWxWYWx1ZUVuY29kaW5nOjpDb2Fs ZXNjZSB9OwogfTsKIAogfSAvLyBuYW1lc3BhY2UK