Shrink HTMLTableCellElement.

[Andreas Kling]

We don't need to cache the rowspan and colspan attributes on the element.

[Andreas Kling]

Created attachment 109640 [details]
Proposed patch

[Antti Koivisto]

r=me

[Darin Adler]

Comment on attachment 109640 [details]
Proposed patch

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

> Source/WebCore/ChangeLog:10
> +        Don't cache the rowspan and colspan attributes on the element.
> +        This shrinks HTMLTabelCellElement by one CPU word, reducing memory
> +        consumption by 80 kB (on 64-bit) when loading the full HTML5 spec.

What’s the performance impact? How often are these functions called?

> Source/WebCore/html/HTMLTableCellElement.cpp:59
> +    if (colSpanValue.isNull())
> +        return 1;

Null check seems unneeded, the next line gives 1 without a special case.

> Source/WebCore/html/HTMLTableCellElement.cpp:67
> +    if (rowSpanValue.isNull())
> +        return 1;

Null check seems unneeded, the next line gives 1 without a special case.

[Andreas Kling]

(In reply to comment #3)
> (From update of attachment 109640 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=109640&action=review
> 
> > Source/WebCore/ChangeLog:10
> > +        Don't cache the rowspan and colspan attributes on the element.
> > +        This shrinks HTMLTabelCellElement by one CPU word, reducing memory
> > +        consumption by 80 kB (on 64-bit) when loading the full HTML5 spec.
> 
> What’s the performance impact? How often are these functions called?

Sorry, I responded to Antti on IRC about this. They are only called once from RenderTableCell::updateFromElement(). They are then cached on the renderer as well.

> > Source/WebCore/html/HTMLTableCellElement.cpp:59
> > +    if (colSpanValue.isNull())
> > +        return 1;
> 
> Null check seems unneeded, the next line gives 1 without a special case.

You are right, thanks! I'll land without that.

> > Source/WebCore/html/HTMLTableCellElement.cpp:67
> > +    if (rowSpanValue.isNull())
> > +        return 1;
> 
> Null check seems unneeded, the next line gives 1 without a special case.

Ditto.

[Andreas Kling]

Created attachment 109646 [details]
Patch for landing (r=anttik)

[WebKit Review Bot]

Comment on attachment 109646 [details]
Patch for landing (r=anttik)

Clearing flags on attachment: 109646

Committed r96638: <http://trac.webkit.org/changeset/96638>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.