RESOLVED FIXED 69347
Shrink HTMLTableCellElement.
https://bugs.webkit.org/show_bug.cgi?id=69347
Summary Shrink HTMLTableCellElement.
Andreas Kling
Reported 2011-10-04 10:31:17 PDT
We don't need to cache the rowspan and colspan attributes on the element.
Attachments
Proposed patch (3.47 KB, patch)
2011-10-04 10:32 PDT, Andreas Kling
koivisto: review+
kling: commit-queue-
Patch for landing (r=anttik) (3.38 KB, patch)
2011-10-04 10:50 PDT, Andreas Kling
no flags
Andreas Kling
Comment 1 2011-10-04 10:32:06 PDT
Created attachment 109640 [details] Proposed patch
Antti Koivisto
Comment 2 2011-10-04 10:37:05 PDT
r=me
Darin Adler
Comment 3 2011-10-04 10:45:41 PDT
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
Comment 4 2011-10-04 10:48:18 PDT
(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
Comment 5 2011-10-04 10:50:33 PDT
Created attachment 109646 [details] Patch for landing (r=anttik)
WebKit Review Bot
Comment 6 2011-10-04 13:38:50 PDT
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
Comment 7 2011-10-04 13:38:55 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.