Closed Bug 995047 Opened 6 years ago Closed 6 years ago

Change stack/exception APIs to hand out AString instead of AUTF8String

Categories

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

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla31

People

(Reporter: bzbarsky, Assigned: bzbarsky)

Details

Attachments

(1 file)

Makes them faster from JS, and removes some internal conversions that were sucking perf-wise.  Speeds up my testcase in bug 920116 by 25% or so.
Comment on attachment 8405126 [details] [diff] [review]
Change out stack/exception APIs to hand out AString instead of AUTF8String for filename/functionname, so we end up with fewer string conversions in practice.

>diff --git a/dom/base/DOMException.cpp b/dom/base/DOMException.cpp

> /* readonly attribute AUTF8String filename; */
Not AUTH8String anymore

> NS_IMETHODIMP
>-Exception::GetFilename(nsACString& aFilename)
>+Exception::GetFilename(nsAString& aFilename)
> {

> void
> Exception::GetFilename(nsString& retval)
> {
Do we really need both these? xpidl method
would work just fine with webidl, or am I missing something here?


>-  nsCString str;
> #ifdef DEBUG
>   DebugOnly<nsresult> rv =
> #endif
>-  GetFilename(str);
>+  GetFilename(retval);
I don't even see why this ends up calling the other GetFilename, especially
in opt builds.




>     JS::FrameDescription& desc = mStackDescription->FrameAt(mIndex);
>     if (JSFlatString *name = desc.funDisplayName()) {
>-      CopyUTF16toUTF8(JS_GetFlatStringChars(name), mFunname);
>+      mFunname.Assign(JS_GetFlatStringChars(name),
>+                      // XXXbz Can't JS_GetStringLength on JSFlatString!
>+                      JS_GetStringLength(JS_FORGET_STRING_FLATNESS(name)));
uh


So if we could have just one GetFilename, r=me
Attachment #8405126 - Flags: review?(bugs) → review+
> Not AUTH8String anymore

Done.

> Do we really need both these?

No, good catch.

> I don't even see why this ends up calling the other GetFilename

Even better catch.  It doesn't.  The code doesn't even compile in a debug build...

I've removed the non-XPCOM GetFilename.

> uh

Yeah...
https://hg.mozilla.org/mozilla-central/rev/0071ce761fca
https://hg.mozilla.org/mozilla-central/rev/5e3f28be5556
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.