[webkit-reviews] review granted: [Bug 100630] Make rendering tables with <colgroups> twice as fast by avoiding walking the DOM for colgroups 4 times for each cell : [Attachment 171160] Patch

[bugzilla-daemon at webkit.org]

Ojan Vafai <ojan at chromium.org> has granted Eric Seidel <eric at webkit.org>'s
request for review:
Bug 100630: Make rendering tables with <colgroups> twice as fast by avoiding
walking the DOM for colgroups 4 times for each cell
https://bugs.webkit.org/show_bug.cgi?id=100630

Attachment 171160: Patch
https://bugs.webkit.org/attachment.cgi?id=171160&action=review

------- Additional Comments from Ojan Vafai <ojan at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=171160&action=review


Algorithmically, I wonder if you could save time by creating a
colElementIterator class. Looks like at least some of the callers of colElement
just iterate through the columns in order. Would be nice to not have to always
start from the beginning of the list. Either way, this patch is fine.

> Source/WebCore/rendering/RenderTable.cpp:792
> +RenderTableCol* RenderTable::slowColElement(unsigned col, bool* startEdge,
bool* endEdge) const

Not from your patch, but it seems silly to have a method called slowColElement
when the only caller is in colElement which just does an early return and then
calls this. May as well move the early return into this function and have this
be colElement. I guess maybe colElement gets inlined?