Bug 71835

Summary: Move CSSPrimitiveValue bitfields up into CSSValue.
Product: WebKit Reporter: Andreas Kling <kling>
Component: CSSAssignee: Andreas Kling <kling>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, koivisto, macpherson, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Palatial patch
none
Patch without double ChangeLog.. darin: review+

Description Andreas Kling 2011-11-08 10:38:01 PST
We can shrink CSSPrimitiveValue by one CPU word if we move the bitfields up into CSSValue.
Comment 1 Andreas Kling 2011-11-08 10:42:54 PST
Created attachment 114113 [details]
Palatial patch
Comment 2 Andreas Kling 2011-11-08 10:49:24 PST
Comment on attachment 114113 [details]
Palatial patch

View in context: https://bugs.webkit.org/attachment.cgi?id=114113&action=review

> Source/WebCore/ChangeLog:55
> +2011-11-08  Andreas Kling  <kling@webkit.org>

Herp derp double ChangeLog.
Comment 3 Andreas Kling 2011-11-08 10:52:09 PST
Created attachment 114118 [details]
Patch without double ChangeLog..
Comment 4 Darin Adler 2011-11-08 13:48:19 PST
Comment on attachment 114118 [details]
Patch without double ChangeLog..

View in context: https://bugs.webkit.org/attachment.cgi?id=114118&action=review

We should figure out where to put the kind of comment or compile time assertion that will prevent people from adding virtual functions back into CSSValue or adding data members to CSSPrimitiveValue.

> Source/WebCore/css/CSSValue.h:136
>      CSSValue(ClassType classType)

I think this constructor should be explicit.

> Source/WebCore/css/CSSValue.h:137
> +        : m_primitiveUnitType(0)

Might be nice to have some assertions that this is still 0 somewhere.

> Source/WebCore/css/CSSValue.h:197
> +    // These bits are only used by CSSPrimitiveValue but kept here
> +    // to maximize struct packing.

Great comment!

> Source/WebCore/css/CSSValue.h:198
> +    signed m_primitiveUnitType : 8; // CSSPrimitiveValue::UnitTypes

Why signed instead of unsigned?
Comment 5 Darin Adler 2011-11-08 13:48:50 PST
I wonder if there’s an optimization possible where we don’t even allocate separate value objects on the heap until websites start using the CSS DOM.
Comment 6 Antti Koivisto 2011-11-08 15:58:59 PST
(In reply to comment #5)
> I wonder if there’s an optimization possible where we don’t even allocate separate value objects on the heap until websites start using the CSS DOM.

Yes, that's really the goal. Stylesheet data structures should be separated from the CSSOM objects and the latter should only be constructed on demand. We can have faster and more compact data structures (no parent pointers, values allocated from array) and gain ability to cache and share parsed stylesheets.
Comment 7 Andreas Kling 2011-11-09 03:12:51 PST
Committed r99679: <http://trac.webkit.org/changeset/99679>