Bug 187450

Summary:

Shrink WebCore::Pair

Product:

WebKit

Reporter:

Simon Fraser (smfr) <simon.fraser>

Component:

New Bugs

Assignee:

Simon Fraser (smfr) <simon.fraser>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

cdumez, koivisto, sam, simon.fraser, webkit-bug-importer, ysuzuki, zalan

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	sam: review+, cdumez: commit-queue-

Simon Fraser (smfr)

Reported 2018-07-08 10:59:43 PDT

Shrink WebCore::Pair

Attachments
Patch (2.48 KB, patch) 2018-07-08 11:01 PDT, Simon Fraser (smfr)	sam: review+ cdumez: commit-queue-	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

Simon Fraser (smfr)

Comment 1 2018-07-08 11:01:34 PDT

Created attachment 344550 [details] Patch

Simon Fraser (smfr)

Comment 2 2018-07-08 11:07:15 PDT

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

Comment 3 2018-07-08 11:48:57 PDT

(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 4 2018-07-08 14:42:55 PDT

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 5 2018-07-09 08:52:27 PDT

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)

Comment 6 2018-07-09 11:17:19 PDT

Will fix.

Simon Fraser (smfr)

Comment 7 2018-07-09 12:24:18 PDT

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

Radar WebKit Bug Importer

Comment 8 2018-07-09 12:25:28 PDT

<rdar://problem/41984422>

Note You need to log in before you can comment on or make changes to this bug.