1217436 - Make nsIdentifierMapEntry::mIdContentList a nsAutoTArray to save an allocation

Status: RESOLVED FIXED

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

Description

[Biru [:poiru]]

See bug 819090 comment 32 and onwards.

Comment 1

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

Using non-moveable PLDHashEntryHdr is not good at all for the performance, but seems like in common
cases the allocation is even slower.
We do need to get SmallVoidArray back in some form, but that shouldn't block this bug.

Comment 2

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

Attachment: id_hash_autoarray.diff

Comment 3

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

Commit message could be
-m "Bug 1217436 - Make nsIdentifierMapEntry::mIdContentList an AutoTArray to save an allocation, r=nfroyd"

Comment 4

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

I'm still wondering if 
Element* mFirstElement;
nsTArray<Element*> mIdContentList; 
should be used here to allow memmoves.
That should give better perf but possible make the code quite a bit more complicated.
But that is for another bug.

Comment 5

[Nathan Froyd [:froydnj]]

Comment on attachment 8857686 [details] [diff] [review]
id_hash_autoarray.diff

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

::: dom/base/nsDocument.h
@@ +156,5 @@
>    explicit nsIdentifierMapEntry(const nsAString* aKey) :
>      nsStringHashKey(aKey), mNameContentList(nullptr)
>    {
>    }
> +  nsIdentifierMapEntry(nsIdentifierMapEntry&& aOther) :

Is it possible to delete the copy constructor (and maybe copy assignment, too) here just to make sure, or does that run into weird compile errors somewhere?

Comment 6

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

Attachment: delete copy ctor and assigment operator

Still compiling.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0bd3edbb660b
Make nsIdentifierMapEntry::mIdContentList an AutoTArray to save an allocation, r=nfroyd

Comment 8

[Iris Hsiao [:ihsiao]]

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