Bug 187450 – Shrink WebCore::Pair

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Simon Fraser (smfr)]

Shrink WebCore::Pair

[Simon Fraser (smfr)]

Created attachment 344550 [details]
Patch

[Simon Fraser (smfr)]

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 %

[Yusuke Suzuki]

(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!

[Sam Weinig]

Comment on attachment 344550 [details]
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?

[Chris Dumez]

Comment on attachment 344550 [details]
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.

[Simon Fraser (smfr)]

Will fix.

[Simon Fraser (smfr)]

https://trac.webkit.org/changeset/233652/webkit

[Radar WebKit Bug Importer]

<rdar://problem/41984422>