Closed Bug 1355787 Opened 3 years ago Closed 3 years ago

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

Categories

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

50 Branch
enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: smaug, Assigned: smaug)

References

(Blocks 1 open bug)

Details

Attachments

(3 files)

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.
Assignee: nobody → bugs
Attached file a testcase
The patch for this bug + the patch for bug 1217436 will cut 20% of the testcase.
Attached file another test
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"
Attachment #8857688 - Flags: review?(nfroyd)
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?! ;)
Attachment #8857688 - Flags: review?(nfroyd) → review+
Pushed by opettay@mozilla.com:
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
https://hg.mozilla.org/mozilla-central/rev/0a73f87d5741
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Depends on: 1356874
You need to log in before you can comment on or make changes to this bug.