Make nsIdentifierMapEntry::mIdContentList a nsAutoTArray to save an allocation

RESOLVED FIXED in Firefox 55

Status

()

Core
DOM: Core & HTML
RESOLVED FIXED
2 years ago
8 months ago

People

(Reporter: poiru, Assigned: smaug)

Tracking

(Blocks: 1 bug)

Trunk
mozilla55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox44 affected, firefox55 fixed)

Details

Attachments

(2 attachments)

(Reporter)

Description

2 years ago
See bug 819090 comment 32 and onwards.
Assignee: birunthan → bugs
Blocks: 1355472
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.
Created attachment 8857686 [details] [diff] [review]
id_hash_autoarray.diff
Attachment #8857686 - Flags: review?(nfroyd)
Commit message could be
-m "Bug 1217436 - Make nsIdentifierMapEntry::mIdContentList an AutoTArray to save an allocation, r=nfroyd"
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 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?
Attachment #8857686 - Flags: review?(nfroyd) → review+
Created attachment 8858082 [details] [diff] [review]
delete copy ctor and assigment operator

Still compiling.

Comment 7

8 months ago
Pushed by opettay@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0bd3edbb660b
Make nsIdentifierMapEntry::mIdContentList an AutoTArray to save an allocation, r=nfroyd

Comment 8

8 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/0bd3edbb660b
Status: ASSIGNED → RESOLVED
Last Resolved: 8 months ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.