Bug 25605 – Reduce the size of GlyphPage

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Simon Fraser (smfr)]

GlyphPage wastes a lot of space with padding, because it has a 256-size array of GlyphData, which is { unsigned short, pointer }. In 64-bit, that's 6 bytes of padding.

[Simon Fraser (smfr)]

<rdar://problem/6864062>

[Simon Fraser (smfr)]

Created attachment 30086 [details]
Patch, changelog

[Darin Adler]

Comment on attachment 30086 [details]
Patch, changelog

How did you test the performance impact of this change?

[Simon Fraser (smfr)]

I'm going to run PLTs today.

[Simon Fraser (smfr)]

PLTs show this is a small perf gain (0.25%).

[Darin Adler]

Comment on attachment 30086 [details]
Patch, changelog

> +    GlyphData()
> +        : glyph(0)
> +        , fontData(0)
> +    {
> +    }
> +    GlyphData(Glyph g, const SimpleFontData* f)
> +        : glyph(g)
> +        , fontData(f)
> +    {
> +    }

Could just write the above as one constructor with default arguments:

    GlyphData(Glyph g = 0, const SimpleFontData* f = 0)

instead.

> +        memcpy(m_glyphs, other.m_glyphs, GlyphPage::size * sizeof(m_glyphs[0]));
> +        memcpy(m_glyphFontData, other.m_glyphFontData, GlyphPage::size * sizeof(m_glyphFontData[0]));

This is in a member of GlyphPage, so you can omit the GlyphPage:: from the use of size. In fact, you should just use sizeof(m_glyphs) and sizeof(m_glyphFontData).

> +        memset(m_glyphs, 0, GlyphPage::size * sizeof(m_glyphs[0]));
> +        memset(m_glyphFontData, 0, GlyphPage::size * sizeof(m_glyphFontData[0]));

Ditto.

r=me

[Simon Fraser (smfr)]

http://trac.webkit.org/changeset/43364