Make rendering tables with <colgroups> twice as fast by avoiding walking the DOM for colgroups 4 times for each cell
Created attachment 171160 [details] Patch
This is sadly not an algorithmic fix (it's lame that we even need to lookup these <colgroup> elements so often), it's just making these dumb lookups faster. Looking at the profile after this fix, we're still spending 21% of our time in slowColElement, we're just now walking this Vector instead of the DOM.
Comment on attachment 171160 [details] Patch 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?
(In reply to comment #3) > (From update of attachment 171160 [details]) > 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? Correct. I split slowColElement off of colElement in an earlier patch to allow the no-columns (common) case to be inline and fast. It was a big win on dglazkov's biggrid iirc.
Comment on attachment 171160 [details] Patch Clearing flags on attachment: 171160 Committed r132764: <http://trac.webkit.org/changeset/132764>
All reviewed patches have been landed. Closing bug.
I do think some sort of state to prevent having to re-walk the columns every time would be useful. It's not yet clear to me what the access pattern of this data is yet.
This fix may not be necessary in the end. If we can make collapsed borders not so expensive we won't need this complexity.
Reopening to attach new patch.
Created attachment 171503 [details] Patch for landing
Sorry, got confused.