Bug 236486 - Make WidgetHierarchyUpdatesSuspensionScope cheaper if it has nothing to do
Summary: Make WidgetHierarchyUpdatesSuspensionScope cheaper if it has nothing to do
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Cameron McCormack (:heycam)
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-02-10 23:15 PST by Cameron McCormack (:heycam)
Modified: 2022-02-12 07:31 PST (History)
11 users (show)

See Also:


Attachments
Patch (4.13 KB, patch)
2022-02-10 23:17 PST, Cameron McCormack (:heycam)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>