453702 - [FIX]Dromaeo (v2) "DOM Query: childNodes" 11.3% of time is spent in js_ValueToNumber

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[Benjamin Smedberg]

11.3% of the "DOM Query: childNodes" test in dromaeo(v2) is spent in js_ValueToNumber, and a lot of that is in js_strtod.

The test seems to only use integer keys, and shouldn't really be spending any time here at all.

Assigning to jorendorff for first-crack. Feel free to kick it back to me whenever.

Comment 1

[Benjamin Smedberg]

+ 100.0% js_Interpret (libmozjs.dylib)
| + 33.9% js_GetPropertyHelper (libmozjs.dylib)
| | + 28.7% js_LookupPropertyWithFlags (libmozjs.dylib)
| | | + 17.9% XPC_WN_Helper_NewResolve(JSContext*, JSObject*, long, unsigned, JSObject**) (XUL)
| | | | + 12.5% nsGenericArraySH::NewResolve(nsIXPConnectWrappedNative*, JSContext*, JSObject*, long, unsigned, JSObject**, int*) (XUL)
| | | | | + 12.0% nsDOMClassInfo::GetArrayIndexFromId(JSContext*, long, int*) (XUL)
| | | | | | + 11.3% JS_ValueToNumber (libmozjs.dylib)
| | | | | | | + 11.0% js_ValueToNumber (libmozjs.dylib)
| | | | | | | | + 7.4% js_strtod (libmozjs.dylib)
| | | | | | | | | + 4.5% JS_strtod (libmozjs.dylib)

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: Don't keep trying to turn "length" into a number

This shaves off 13-15% of the time or so on this test.

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 337068 [details] [diff] [review]
Don't keep trying to turn "length" into a number

>--- a/dom/src/base/nsDOMClassInfo.cpp
>+    return NS_OK;
>+  }
>+  

Super-uber-drive-by-nit: delete the trailing whitespace on this line, please?

Comment 4

[Boris Zbarsky [:bzbarsky]]

Pushed changeset 534edf352f6b.  I forgot to address the super-nit, though...  Ideally I'd prefer an emacs mode that just doesn't put those spaces in (or rather removes them on enter), if someone can suggest one.

Comment 5

[Fabien Tassin]

(In reply to comment #4)
> Ideally I'd prefer an emacs mode that just doesn't put those spaces in (or
> rather removes them on enter), if someone can suggest one.

http://www.splode.com/~friedman/software/emacs-lisp/src/whitespace.el
and
(autoload 'nuke-trailing-whitespace "whitespace" nil t)
(add-hook 'write-file-hooks 'nuke-trailing-whitespace)

Comment 6

[Blake Kaplan (:mrbkap) (inactive)]

Note: A lot of files have pre-existing trailing whitespace. A hook like that might have unexpected consequences (especially in XPConnect!).

Comment 7

[Boris Zbarsky [:bzbarsky]]

Yeah, I don't want it nuking whitespace on lines of code I didn't touch.