Closed
Bug 1392790
Opened 7 years ago
Closed 7 years ago
CustomElementReactionsStack::PopAndInvokeElementQueue() shows up in performance profiles
Categories
(Core :: DOM: Core & HTML, enhancement)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla57
Tracking | Status | |
---|---|---|
firefox57 | --- | fixed |
People
(Reporter: smaug, Assigned: jdai)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 1 obsolete file)
4.02 KB,
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
CustomElementReactionsStack::PopAndInvokeElementQueue() shows up in performance profiles even if dom.webcomponents.customelements.enabled and dom.webcomponents.enabled both are false.
Reporter | ||
Comment 1•7 years ago
|
||
In this case it is in set_value in HTMLInputElementBinding.cpp (generated code).
Comment 2•7 years ago
|
||
Oh, we do this for every operations, attributes, setters, or deleters annotated with the [CEReactions] extended attribute [1]. Can we check if the webcomponent preferences are enabled in our codegen?
[1] http://searchfox.org/mozilla-central/rev/48ea452803907f2575d81021e8678634e8067fc2/dom/bindings/Codegen.py#7842-7851
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → jdai
Assignee | ||
Comment 3•7 years ago
|
||
In this patch, I add webcomponent preference check in order to avoid doing [CEReactions] when webcomponent preference are disabled.
Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=030c21cf02f706c3a9a4ffe70fda392373e922b6&filter-tier=1&group_state=expanded&selectedJob=125074622
Attachment #8900127 -
Flags: review?(bugs)
Reporter | ||
Comment 4•7 years ago
|
||
Comment on attachment 8900127 [details] [diff] [review]
patch, v1
GetCustomElementReactionsStack call should be also inside IsCustomElementEnabled check.
Also, could we inline CustomElementRegistry::IsCustomElementEnabled().
This is extremely hot code after all.
Even once we enable custom elements, this code should be fast for documents which don't have any custom elements. But that optimization can be done in a different bug.
Attachment #8900127 -
Flags: review?(bugs) → review-
Assignee | ||
Comment 5•7 years ago
|
||
(In reply to Olli Pettay [:smaug] from comment #4)
Thanks for your review.
> Comment on attachment 8900127 [details] [diff] [review]
> patch, v1
>
> GetCustomElementReactionsStack call should be also inside
> IsCustomElementEnabled check.
Will do.
> Also, could we inline CustomElementRegistry::IsCustomElementEnabled().
> This is extremely hot code after all.
Sure. I will address them in next patch.
>
>
>
> Even once we enable custom elements, this code should be fast for documents
> which don't have any custom elements. But that optimization can be done in a
> different bug.
File bug 1392974.
Assignee | ||
Comment 6•7 years ago
|
||
Address comment 4.
Attachment #8900127 -
Attachment is obsolete: true
Attachment #8900173 -
Flags: review?(bugs)
Reporter | ||
Updated•7 years ago
|
Attachment #8900173 -
Flags: review?(bugs) → review+
Assignee | ||
Updated•7 years ago
|
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e2a544ea360c
Avoid doing [CEReactions] if custom element preference is disabled. r=smaug
Keywords: checkin-needed
Comment 8•7 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Comment 9•7 years ago
|
||
we see a performance improvement from this patch:
== Change summary for alert #8971 (as of August 23 2017 14:33 UTC) ==
Improvements:
9% dromaeo_dom summary linux64 pgo e10s 2,033.09 -> 2,208.32
8% dromaeo_dom summary linux64 opt e10s 1,920.19 -> 2,081.49
For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=8971
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•