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

Status: RESOLVED FIXED

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

Description

[Nathan Froyd [:froydnj]]

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.

Comment 1

[Boris Zbarsky [:bzbarsky, bz on IRC]]

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.

Comment 2

[Nathan Froyd [:froydnj]]

Attachment: move nsString construction out of mozilla::dom::Register for a codesize win

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.

Comment 3

[Boris Zbarsky [:bzbarsky, bz on IRC]]

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.

Comment 4

[Nathan Froyd [:froydnj]]

(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.

Comment 5

[Boris Zbarsky [:bzbarsky, bz on IRC]]

> Why do the nsScriptNamespaceManager interfaces take nsAFlatString

I don't know offhand....

Comment 6

[Nathan Froyd [:froydnj]]

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.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/f54a80137626