Currently all RenderBoxes are calling clearOverrideSize. However a RenderBox could have an override size in only 3 cases: * if it's a child of a RenderDeprecatedFlexibleBox * if it's a child of a RenderFlexibleBox * if it's a RenderTableCell That would remove 2 hash lookups from RenderBox::willBeDestroyed.
Created attachment 128899 [details] Proposed refactoring.
Comment on attachment 128899 [details] Proposed refactoring. View in context: https://bugs.webkit.org/attachment.cgi?id=128899&action=review r~ from me :-D > Source/WebCore/rendering/RenderBox.cpp:-251 > - clearOverrideSize(); > - I am not convinced by this change. You are spreading complexity to avoid one very simple call. Could you find if this has any impact? > Source/WebCore/rendering/RenderBox.cpp:802 > - if (hasOverrideHeight()) > + if (gOverrideHeightMap) > gOverrideHeightMap->remove(this); > - if (hasOverrideWidth()) > + if (gOverrideWidthMap) > gOverrideWidthMap->remove(this); This looks good.
Comment on attachment 128899 [details] Proposed refactoring. Let's r- until you benchmark it.
> Let's r- until you benchmark it. Thanks for doubting my change. I tested on html5-full-renderer and it looks like it regressed a bit the benchmark (around 1%, small but significant). It looks like the branches we introduced were killing us and / or my assumption about the use of RenderDeprecatedFlexibleBox or RenderFlexibleBox were wrong. I will re-purpose the bug to center around the good part and put a new patch for review.
Re-purposing the bug on the part that is safe.
Created attachment 129089 [details] Smaller, better change. No change on html5-full-render.html this time.
Damn review thief! ;)
(In reply to comment #7) > Damn review thief! ;) :'( ... <:o)
Comment on attachment 129089 [details] Smaller, better change. No change on html5-full-render.html this time. Clearing flags on attachment: 129089 Committed r109028: <http://trac.webkit.org/changeset/109028>
All reviewed patches have been landed. Closing bug.