Open Bug 1237451 Opened 8 years ago Updated 3 years ago

Accessing document.body in a loop which is doing dom modifications is surprisingly slow

Categories

(Core :: DOM: Core & HTML, defect, P5)

Product:
Core
Component:
DOM: Core & HTML
Version:
36 Branch
Type:
defect

Tracking

()

People

(Reporter: smaug, Unassigned)

References

Details

I would have expected document.body code path to be rather well optimized, 
but https://bugzilla.mozilla.org/attachment.cgi?id=8704844 (a testcase for bug 40988)
shows we're accessing document.body via Proxy stuff and that seems to be surprisingly slow.

Zoom says up to 4% of the relevant js execution time is spent trying to access body, Gecko profiler says it is closer to 1%.

( Proxy handling shows up also with style.left/top setters. )

Does anything prevent us using [Cached] with document.body?
A few things going on here:

1)  For the specific case of document.something we should be taking a fast path in general _if_ the mExpandoAndGeneration.generation hasn't changed.  If it has, then because document is [OverrideBuiltins] we have to take the slow path.  Put another way, if the JS engine caches (as part of its jitcode) that looking up .body on document gets the getter from HTMLDocument.prototype but then someone adds a <img name="body"> to the document, then suddenly document.body is that <img>, not the <body> and the information the JS engine cached is invalid.  Now in practice we mutate mExpandoAndGeneration.generation from nsDocument::AddToNameTable, nsDocument::RemoveFromNameTable, nsDocument::AddToIdTable, nsDocument::RemoveFromIdTable, and nsDocument::DestroyElementMaps.  I wouldn't expect any of those to get hit in this testcase, though.

2)  I looked into why we're not actually taking the baseline fast path here.  The reason for that is that someone broke (or wrote wrong initially!) the baseline IC for this and we apparently have no regression tests.  I filed bug 1237501 on that.  That said, I don't know that it helps much here, because long-term this testcase runs in Ion and in Ion the IC works fine, afaict.  Are you sure you're seeing proxy stuff for document.body?

3)  We can't use [Cached] for document.body because we don't support [Cached] on proxy bindings.  We don't have a bug tracking this, apparently, so I filed bug 1237503.  And bug 1237504 on the oft-discussed but apparently not filed JS engine issue preventing us supporting it.
Depends on: 1237501

Bulk-downgrade of unassigned, untouched DOM/Storage bug's priority.

If you have reason to believe, this is wrong, please write a comment and ni :jstutte.

Severity: normal → S4
Priority: -- → P5
You need to log in before you can comment on or make changes to this bug.