Closed Bug 965634 Opened 6 years ago Closed 6 years ago

mozilla::dom::Register includes redundant nsString constructor/destructor calls

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla29

People

(Reporter: froydnj, Unassigned)

Details

(Whiteboard: [qa-])

Attachments

(1 file)

Register is rather large, ~27K on x86-64.  One of the things that makes it large is constructing an nsString for every call it does (mostly to RegisterDefineDOMInterface), and then destructing it after the call.  This nsString is copied into a global hashtable.

Might it be worth it to pass in a raw char16_t*?  We'd have to compute the length at runtime (or add another parameter to RegisterDefineDOMInterface, or something), but maybe that'd be worth it for cutting off a significant chunk of code.
Passing a char16_t* and separate length argument makes total sense to me.

We could also try adding a template ctor to FakeDependentString that takes char16_t[N], or something, and using that here.
Here's a first cut at not constructing the nsStrings in Register() itself.  The
template methods can optionally be compiled out of line if they are called more
than once.

On both x86-64 Linux and ARM Android, this cuts ~12K of space off the size of
Register(), even after accounting for the ~30 RegisterDefineDOMInterface template
helper methods that have been out-of-lined.

Trying to stuff everything into a single string would only win ~1.5K on ARM, not
sure if it's worth it.  Would also complicate the code generator, as we'd have
to keep track of the lengths ourselves, and pass the lengths as arguments from
Register.  Might come out to less 1.5K of win in the end.
Attachment #8368099 - Flags: review?(bzbarsky)
Comment on attachment 8368099 [details] [diff] [review]
move nsString construction out of mozilla::dom::Register for a codesize win

r=me, though I do wonder whether using FakeDependentString instead of nsDependentString would let you shave off more codesize.
Attachment #8368099 - Flags: review?(bzbarsky) → review+
(In reply to Boris Zbarsky [:bz] from comment #3)
> r=me, though I do wonder whether using FakeDependentString instead of
> nsDependentString would let you shave off more codesize.

I think it might help a little bit, but including BindingUtils.h in nsScriptNamespaceManager.h seems excessive.  Why do the nsScriptNamespaceManager interfaces take nsAFlatString, rather than plain old nsAString?  The compiler is unhappy about trying to pass FakeDependentString aka nsAString where nsAFlatString is desired.
> Why do the nsScriptNamespaceManager interfaces take nsAFlatString

I don't know offhand....
Try informs me that using nsDependentString results in mass mochitest bustage (things like XMLHttpRequest not getting registered, apparently).  However, since we're using this nice template, we can use nsLiteralString instead.  It doesn't change the codesize win and it greens things up, so I'm just going to check the patch in with that change.

We can worry about FakeDependentString or similar in another bug.
https://hg.mozilla.org/mozilla-central/rev/f54a80137626
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Whiteboard: [qa-]
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.