Bug 236486

Summary: Make WidgetHierarchyUpdatesSuspensionScope cheaper if it has nothing to do
Product: WebKit Reporter: Cameron McCormack (:heycam) <heycam>
Component: Layout and RenderingAssignee: Cameron McCormack (:heycam) <heycam>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, changseok, darin, esprehn+autocc, ews-watchlist, glenn, kondapallykalyan, pdr, simon.fraser, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Description Cameron McCormack (:heycam) 2022-02-10 23:15:10 PST
With content that does a lot of DOM manipulation, we can create and destroy a WidgetHierarchyUpdatesSuspensionScope on the stack many times.  When this object has nothing to do, it calls an out of line function.  We can pull out the check for whether it needs to call moveWidgets() into the inline destructor.

This is a 1% saving on the jQuery-TodoMVC subtest of Speedometer 2, though the effect on the top line score is minimal.
Comment 1 Cameron McCormack (:heycam) 2022-02-10 23:17:15 PST
Created attachment 451648 [details]
Patch
Comment 2 Darin Adler 2022-02-11 11:36:27 PST
Comment on attachment 451648 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=451648&action=review

> Source/WebCore/rendering/RenderWidget.h:60
> +    widgetNewParentMap().set(&widget, frame);

For maps, please keep in mind that add is the slightly more efficient operation. The difference is that add will not overwrite the value in an existing map entry, whereas set will overwrite it. If either is OK, then add is preferred for slightly better performance and code size.
Comment 3 Cameron McCormack (:heycam) 2022-02-11 12:50:07 PST
(In reply to Darin Adler from comment #2)
> For maps, please keep in mind that add is the slightly more efficient
> operation. The difference is that add will not overwrite the value in an
> existing map entry, whereas set will overwrite it. If either is OK, then add
> is preferred for slightly better performance and code size.

This is existing code, and I am not entirely sure whether it is expected that scheduleWidgetToMove will only ever be called for a given Widget once before the moves are effected and the map is cleared.  But the add vs set distinction is a good nugget of knowledge to learn, thanks.
Comment 4 EWS 2022-02-12 07:30:17 PST
Committed r289692 (247177@main): <https://commits.webkit.org/247177@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 451648 [details].
Comment 5 Radar WebKit Bug Importer 2022-02-12 07:31:17 PST
<rdar://problem/88855693>