[webkit-reviews] review denied: [Bug 65491] Implement a faster path for painting tables with overflowing cells : [Attachment 103686] Better version: do not repaint the same cell twice, tweaked the change for more performance.

[bugzilla-daemon at webkit.org]

Dave Hyatt <hyatt at apple.com> has denied Julien Chaffraix
<jchaffraix at webkit.org>'s request for review:
Bug 65491: Implement a faster path for painting tables with overflowing cells
https://bugs.webkit.org/show_bug.cgi?id=65491

Attachment 103686: Better version: do not repaint the same cell twice, tweaked
the change for more performance.
https://bugs.webkit.org/attachment.cgi?id=103686&action=review

------- Additional Comments from Dave Hyatt <hyatt at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=103686&action=review


Minusing for the HashSet improvement that can be made.

> Source/WebCore/ChangeLog:21
> +	   (WebCore::RenderTableSection::layoutRows): Added some code to
accumulute the overflowing cells

Typo: accumulate

> Source/WebCore/rendering/RenderTableSection.cpp:1064
> +	       HashSet<RenderTableCell*> spanningAndOverflowingCells =
m_overflowingCells;

Don't do it this way. Just check both hash tables in the code below. This is
going to do a complete copy of the HashTable and that's slow.

> Source/WebCore/rendering/RenderTableSection.cpp:1073
> +			   if
(spanningAndOverflowingCells.contains(current.cells[i]))
> +			       continue;

Just check both HashSets individually here.