1430951 - Avoid element name atomizing to improve performance of LookupCustomElementDefinition

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement, P2)

Description

[Edgar Chen [:edgar]]

From profile [1] in bug 1396575 comment #9, I found we spend some time atomizing aLocalName when looking up definition [2]. And the caller usually pass nodeInfo->LocalName() as aLocalName, like [3]. It looks like we could use nodeInfo->NameAtom() given that these codes are only called with element's nodeInfo which the LocalName() and NameAtom() should be the same value [4].

[1] https://perf-html.io/public/708b78ee5c83e942269ea6b20795464b61337624/calltree/?hiddenThreads=&invertCallstack&range=2.6524_2.7236&thread=5&threadOrder=0-2-3-4-5-1&transforms=f-combined-xAi-i&v=2
[2] https://searchfox.org/mozilla-central/rev/41925c0b6c6d58578690121de439d2a8d3d690f3/dom/base/CustomElementRegistry.cpp#290
[3] https://searchfox.org/mozilla-central/rev/41925c0b6c6d58578690121de439d2a8d3d690f3/dom/base/nsContentUtils.cpp#10012
[4] https://searchfox.org/mozilla-central/rev/48cbb200aa027a0a379b6004b6196a167344b865/dom/base/NodeInfo.cpp#79,89

Comment 1

[Edgar Chen [:edgar]]

Attachment: Bug 1430951 - Avoid element name atomizing to improve performance of LookupCustomElementDefinition;

Since we are dealing with the element (nodeInfo->LocalName() and NameAtom() are the same value),
we could use nodeInfo->NameAtom() instead.

Review commit: https://reviewboard.mozilla.org/r/213432/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/213432/

Comment 2

[Edgar Chen [:edgar]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d203d320356831ad9a127d0cb0f5d7a51d618b5c&group_state=expanded&filter-tier=1

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8943142 [details]
Bug 1430951 - Avoid element name atomizing to improve performance of LookupCustomElementDefinition;

https://reviewboard.mozilla.org/r/213432/#review219336

::: dom/base/nsContentUtils.cpp:9926
(Diff revision 1)
>    NodeInfo* nodeInfo = aElement->NodeInfo();
>    RefPtr<nsAtom> typeAtom =
>      aElement->GetCustomElementData()->GetCustomElementType();
>    CustomElementDefinition* definition =
>      nsContentUtils::LookupCustomElementDefinition(nodeInfo->GetDocument(),
> -                                                  nodeInfo->LocalName(),
> +                                                  nodeInfo->NameAtom(),

Could you assert somewhere here that
MOZ_ASSERT(nodeInfo->NameAtom()->Equals(nodeIno->LocalName()))

Comment 4

[Edgar Chen [:edgar]]

Comment on attachment 8943142 [details]
Bug 1430951 - Avoid element name atomizing to improve performance of LookupCustomElementDefinition;

https://reviewboard.mozilla.org/r/213432/#review219336

> Could you assert somewhere here that
> MOZ_ASSERT(nodeInfo->NameAtom()->Equals(nodeIno->LocalName()))

Ｗill do.

Comment 5

[Edgar Chen [:edgar]]

Comment on attachment 8943142 [details]
Bug 1430951 - Avoid element name atomizing to improve performance of LookupCustomElementDefinition;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/213432/diff/1-2/

Comment 6

[Edgar Chen [:edgar]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d203d320356831ad9a127d0cb0f5d7a51d618b5c&filter-tier=1&group_state=expanded

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/53d7ccf627fa
Avoid element name atomizing to improve performance of LookupCustomElementDefinition; r=smaug

Comment 8

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/53d7ccf627fa