[webkit-reviews] review granted: [Bug 100057] Replace NodeRareData hash map with a union on m_renderer : [Attachment 171948] Patch

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has granted Elliott Sprehn
<esprehn at chromium.org>'s request for review:
Bug 100057: Replace NodeRareData hash map with a union on m_renderer
https://bugs.webkit.org/show_bug.cgi?id=100057

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

------- Additional Comments from Eric Seidel <eric at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=171948&action=review


This LGTM, but I expect we're going to have some minor perf fallout from this. 
Code has long assumed that renderer() was basically free, and now it's a
branch.

> Source/WebCore/dom/Node.cpp:503
> +    delete m_data.m_rareData;

I assume manual delete is needed because of our use of a union?

> Source/WebCore/dom/Node.h:835
> +    // When a node has rare data we move the renderer into the rare data.

You might also mention somnewhere in this header that this makes renderer()
slightly more expensive than before.  Previously renderer() was assumed free.
:)