This is totally evil, but the XPCOM string constructors/destructors are pain and suffering perf-wise. The patch coming up gets the overhead per string argument down from 15ns to what looks like 4ns on my hardware.

Comment on attachment 641691 [details] [diff] [review] Speed up string argument conversion in DOM bindings by faking an XPCOM string. Review of attachment 641691 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/bindings/BindingUtils.h @@ +752,5 @@ > + MOZ_STATIC_ASSERT(sizeof(FakeDependentString) == sizeof(nsDependentString), > + "Must have right object size"); > + // XXXbz would be nice to assert things about member order too, > + // but those are protected on nsDependentString. > + } Could we add an inner class that inherits from nsDependentString so we have access to the members on nsDependentString and can statically assert things about them?

Comment on attachment 642679 [details] [diff] [review] Excellent idea. With that inner class, asserting the object layout now. Review of attachment 642679 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/bindings/BindingUtils.h @@ +760,5 @@ > + mLength = aLength; > + } > + > + void SetIsVoid(bool aSetVoid) { > + // aSetVoid true means voided, false means empty string. I think we should be compatible with the nsAString stuff (false just unsets the flag, true sets the flag and truncates).

> I think we should be compatible with the nsAString stuff The thing is, that requires me to set mData and mLength in the constructor, only to have it clobbered by the Rebind() in the common case... I guess I can do that if you feel pretty strongly about it. Alternately, I could change the naming of SetIsVoid (and of Rebind if needed): the only caller is ConvertJSValueToString and we can just change the contract of its interaction with the string. In particular, we could just have a SetNull() with no argument for the null case, and a SetEmpty() for the empty case.

(In reply to Boris Zbarsky (:bz) from comment #6) > In particular, we could just have a SetNull() > with no argument for the null case, and a SetEmpty() for the empty case. Let's do this (though maybe Truncate instead of SetEmpty?).

Sounds good.

Sorry, push backed out for compilation errors on all platforms: https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=9deb8edb5070 eg: https://tbpl.mozilla.org/php/getParsedLog.php?id=13605808&tree=Mozilla-Inbound { In file included from ../../../../js/xpconnect/wrappers/XrayWrapper.cpp:22:0: ../../../dist/include/nsTSubstring.h:639:18: error: 'nsAString_internal::char_type* nsAString_internal::mData' is protected ../../../dist/include/mozilla/dom/BindingUtils.h:794:77: error: within this context ../../../dist/include/nsTSubstring.h:640:17: error: 'nsAString_internal::size_type nsAString_internal::mLength' is protected ../../../dist/include/mozilla/dom/BindingUtils.h:795:79: error: within this context ../../../dist/include/nsTSubstring.h:641:16: error: 'PRUint32 nsAString_internal::mFlags' is protected ../../../dist/include/mozilla/dom/BindingUtils.h:796:78: error: within this context make[6]: *** [XrayWrapper.o] Error 1 } https://hg.mozilla.org/integration/mozilla-inbound/rev/8212be806c67

Comment on attachment 642679 [details] [diff] [review] Excellent idea. With that inner class, asserting the object layout now. >+ void SetIsVoid(bool aSetVoid) { >+ // aSetVoid true means voided, false means empty string. >+ mData = nsnull; This is not a good idea; BeginReading()[0] is still supported although not many people do it.

Relanded with an attempt to make the static asserts compile on gcc and maybe msvc, not just clang: https://hg.mozilla.org/integration/mozilla-inbound/rev/0e6c79d17f42 > This is not a good idea; Hrm. Replaced with nsDependentString::char_traits::sEmptyBuffer : https://hg.mozilla.org/integration/mozilla-inbound/rev/2b31d5caf239

https://hg.mozilla.org/mozilla-central/rev/0e6c79d17f42 https://hg.mozilla.org/mozilla-central/rev/2b31d5caf239