1355787 - nsIdentifierMapEntry should let one to use either strings or atoms as keys

Status: RESOLVED FIXED

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

Description

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

Right now when binding, the already atomized id attribute is stringified and then
assigned to internal string member variable. NodeInfos has the setup where both nsIAtoms and strings can be used, and I think that would help here to reduce the overhead of converting to string.

Comment 1

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

Attachment: a testcase

The patch for this bug + the patch for bug 1217436 will cut 20% of the testcase.

Comment 2

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

Attachment: another test

Comment 3

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

Attachment: atom_string_idtable.diff

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7a828e9a68ec60f2b1d9ad6c184a2265f0550ff2

Comment 4

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

Comment on attachment 8857688 [details] [diff] [review]
atom_string_idtable.diff

This is on top of bug 1217436.

-m "Bug 1355787, nsIdentifierMapEntry should let one to use either strings or atoms as keys to avoid slow string assignments when possible. r=nfroyd"

Comment 5

[Nathan Froyd [:froydnj]]

Comment on attachment 8857688 [details] [diff] [review]
atom_string_idtable.diff

Review of attachment 8857688 [details] [diff] [review]:
-----------------------------------------------------------------

r=me

::: dom/base/nsDocument.h
@@ +147,3 @@
>  {
>  public:
> +  struct AtomOrString

In an ideal world, I think this would just be:

using AtomOrString = mozilla::Variant<nsCOMPtr<nsIAtom>, nsString>;

but maybe you like to be explicit here. :)

@@ +181,4 @@
>    {
>    }
>    nsIdentifierMapEntry(nsIdentifierMapEntry&& aOther) :
> +    mKey(mozilla::Move(aOther.GetKey())),

mozilla::Move in a patch from smaug?!  What is the world coming to?! ;)

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0a73f87d5741
nsIdentifierMapEntry should let one to use either strings or atoms as keys to avoid slow string assignments when possible. r=nfroyd

Comment 7

[Iris Hsiao [:ihsiao]]

https://hg.mozilla.org/mozilla-central/rev/0a73f87d5741