1851236 - Optimize KeyboardEvent_Binding::_constructor

Status: NEW

(Core :: DOM: Events, enhancement)

Description

[Markus Stange [:mstange]]

Profile: https://share.firefox.dev/45RsilM

0.5% of the measured time on Speedometer 3 is spent inside KeyboardEvent_Binding::_constructor.
We should see if we can optimize it some more.

Comment 1

[Markus Stange [:mstange]]

One thing I noticed: for eventDict.key, we convert the key JSString* to an nsString and then to a KeyNameIndex. The nsString in the middle wastes an alloc+free.

Comment 2

[Markus Stange [:mstange]]

And we QI the global object twice: First to EventTarget and then to nsPIDOMWindowInner.

Comment 3

[Markus Stange [:mstange]]

Event_Binding::_constructor, which we spend 0.2% of sp3 in, looks similar: https://share.firefox.dev/3R0pkHr

Comment 4

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

This is one case which is totally unrealistic in sp3. Web pages don't in general create their own events (nor do they dispatch many events).
But sure, we should optimize this.

I wonder if codegenerator should use nsAutoString for dictionaries, that would at least help with the alloc/free.
Or even faster would be some way more complicated setup where bindings layer would pass JSString value when available to the dictionary and only if JSString wasn't passed, but some other type, then conversion would happen (I think that would be still working per webidl spec).

Comment 5

[Markus Stange [:mstange]]

Chrome appears to avoid the string allocation by only doing it once for this JS string. The JS string is "externalized" the first time it gets converted. This means that its internal storage is replaced with their version of the refcounted nsStringBuffer (blink::StringImpl).
They can do this because blink::StringImpl supports both one-byte and two-byte strings, and even atom strings (even static atoms).

Comment 6

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

I wonder if it helped if we changed the contents of a JSString to external nsStringBuffer in case we allocate such, and JSString itself is either a rope or linear.

Comment 7

[Markus Stange [:mstange]]

Updated profile: https://share.firefox.dev/3rfo9t3 (5% fewer samples than comment 0)