[webkit-reviews] review granted: [Bug 121242] Use bitfields for data members in FontPlatformData. : [Attachment 211452] Patch: use bitfields for boolean and enum members in FontPlatformData.

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Yongjun Zhang
<yongjun_zhang at apple.com>'s request for review:
Bug 121242: Use bitfields for data members in FontPlatformData.
https://bugs.webkit.org/show_bug.cgi?id=121242

Attachment 211452: Patch: use bitfields for boolean and enum members in
FontPlatformData.
https://bugs.webkit.org/attachment.cgi?id=211452&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=211452&action=review


> Source/WebCore/ChangeLog:7
> +	   Some data members in FontPlatformData are in type boolean or enum;
we could use bitfields to pack them
> +	   and reduce FontPlatformData's memory footprint.

How much does this save?

> Source/WebCore/platform/graphics/FontPlatformData.h:227
>      float m_size;
> -    FontWidthVariant m_widthVariant;
>  
> -private:
>  #if OS(DARWIN)
>      NSFont* m_font;
>  #elif PLATFORM(WIN)

I believe that having the float (32-bit) before the pointers (64-bit) will
waste space on 64-bit.

> Source/WebCore/platform/graphics/FontPlatformData.h:257
> +    bool m_syntheticBold : 1;
> +    bool m_syntheticOblique : 1;
> +    unsigned m_orientation : 2; // FontOrientation
> +    unsigned m_widthVariant : 3; // FontWidthVariant
> +
> +    bool m_isColorBitmapFont : 1;
> +    bool m_isCompositeFontReference : 1;
>  #if OS(DARWIN)
> -    bool m_isPrinterFont;
> +    bool m_isPrinterFont : 1;
>  #endif
>  
>  #if PLATFORM(WIN)
> -    bool m_useGDI;
> +    bool m_useGDI : 1;
>  #endif

On the Windows compiler, I believe these will only pack together if they all
use the same type. So if we want this to be small there, they need to all be
unsigned instead of a mix of unsigned and bool.