Closed Bug 453702 Opened 11 years ago Closed 11 years ago

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

Categories

(Core :: XPConnect, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: benjamin, Assigned: bzbarsky)

Details

(Keywords: perf)

Attachments

(1 file)

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.
+ 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)
This shaves off 13-15% of the time or so on this test.
Assignee: jorendorff → bzbarsky
Status: NEW → ASSIGNED
Attachment #337068 - Flags: superreview?(jst)
Attachment #337068 - Flags: review?(jorendorff)
Summary: Dromaeo (v2) "DOM Query: childNodes" 11.3% of time is spent in js_ValueToNumber → [FIX]Dromaeo (v2) "DOM Query: childNodes" 11.3% of time is spent in js_ValueToNumber
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?
Attachment #337068 - Flags: review?(jorendorff) → review+
Attachment #337068 - Flags: superreview?(jst) → superreview+
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.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
(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)
Note: A lot of files have pre-existing trailing whitespace. A hook like that might have unexpected consequences (especially in XPConnect!).
Yeah, I don't want it nuking whitespace on lines of code I didn't touch.
You need to log in before you can comment on or make changes to this bug.