Closed Bug 1352874 Opened 7 years ago Closed 7 years ago

Improve nsHtml5AtomTable performance

Categories

(Core :: DOM: HTML Parser, enhancement)

36 Branch
enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: smaug, Assigned: smaug)

References

Details

Attachments

(1 file)

Hashtable lookups are too slow for hot code paths so need to probably do some caching.
Attached patch v1Splinter Review
This is similar to Bug 1352235.
The cache seems to catch 80-90% of the calls to the nsHtml5AtomTable::GetAtom method when doing some random browsing.


(I need to figure out how to generalize this kind of cache, but that shouldn't block fixing perf issues.)

https://treeherder.mozilla.org/#/jobs?repo=try&revision=026cb223772ef7b977bce8222752dd1c9deac974
Attachment #8853806 - Flags: review?(hsivonen)
Comment on attachment 8853806 [details] [diff] [review]
v1

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

r+, but since atoms were made thread-safe in bug 1275755, the whole notion of parser-scoped atoms should probably be removed.
Attachment #8853806 - Flags: review?(hsivonen) → review+
Additionally: Did you happen to look at why this code path is hot? That this code path would be performance-sensitive is surprising, since the standard element and attribute names should be covered by the pre-interned nsHtml5ElementName and nsHtml5AttributeName instances, which already have static atoms attached to them. Is this some kind of Custom Element problem? Facebook metadata problem?
Flags: needinfo?(bugs)
Getting threadsafe atoms is slow, which is why bug 1351303 makes them reasonable fast for main thread.
The parser code is different enough that local cache was the easiest fix.

And IIRC the test is using some data- attributes.
Flags: needinfo?(bugs)
Pushed by opettay@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/922e2f268ab3
Improve nsHtml5AtomTable performance, r=hsivonen
https://hg.mozilla.org/mozilla-central/rev/922e2f268ab3
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.