Open Bug 1844878 Opened 1 year ago Updated 10 months ago

Improve preservation of DOM wrappers

Categories

(Core :: JavaScript Engine, task, P1)

task

Tracking

()

People

(Reporter: iain, Unassigned)

References

(Blocks 1 open bug)

Details

(Whiteboard: [sp3])

Attachments

(1 file)

To ensure that we don't lose information when assigning a property to a DOM wrapper object and then discarding the wrapper, the code that autogenerates our DOM classes creates an addProperty hook for each class. One significant downside of this approach is that we have to call the hook whenever a property is added, even though every call after the first is redundant. In addition to the overhead of the calls, this also impedes optimization in some cases, because the hook could have arbitrary side-effects.

I've seen this show up in a few places in sp3 perf reports: for example, selection_select in charts-observable-plot, and Ni in React-stockcharts.

One alternative would be to add a new class flag that tells the engine to call a callback whenever the first property is added to an object. This would reduce unnecessary overhead.

Attached file WIP: Bug 1844878: WIP wrapper hook (deleted) —

This is an extremely rough first draft. It doesn't even compile yet.

Blocks: speedometer3
Whiteboard: [sp3]

I've been trying to estimate the benefit of this optimization but I'm not sure what work it would eliminate.

In this full sp3 profile I found 500 samples in CallAddPropertyHook, which corresponds to 0.27% of sp3 overall. How much of the work in this profile would we expect this fix to eliminate?

I also see CallAddPropertyHook called in this TodoMVC-Angular comparison report in the two functions at position #4 and #6.

Flags: needinfo?(iireland)

It's a little tricky to estimate. This patch eliminates calls to the add property hook after the first one, but the first one is the expensive one anyway. On the other hand, it also lets us use faster paths for subsequent addprops, which is hard to measure without just writing the patch. To some extent, the improvement depends on the fraction of wrapper objects that have more than one property added.

As a rough estimate: the samples in that profile inside nsWrapperCache::PreserveWrapper are mostly still necessary (except for the if (PreservingWrapper()) return; part, which I'm assuming is trivial). Dropping the two implementations of that function leaves 174/500 samples. I would hope for the fix to eliminate a solid chunk of those samples.

The amount of code left to be written is relatively small. Mostly I just need to find time to talk to a DOM person about making the DOM-side code of my prototype less criminal.

Flags: needinfo?(iireland)

Ah I see. Thanks!

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: