Closed Bug 1376799 Opened 4 years ago Closed 4 years ago

Optimize Object.prototype.toString more


(Core :: JavaScript Engine, enhancement)

Not set



Tracking Status
firefox56 --- fixed


(Reporter: jandem, Assigned: jandem)


(Blocks 1 open bug)


(Whiteboard: [qf:p1])


(1 file)

Bug 1369042 was a decent win, but there's a bunch more slowness to eliminate here.
Attached patch PatchSplinter Review
This patch:

* Moves JSObject::callHook, JSObject::isCallable etc to jsobjinlines.h These functions are just a few branches in the common case so we don't want OOL calls.

* Adds a fast path to determine builtinTag in obj_toString. I moved the existing code into GetBuiltinTagSlow, that code is now only used for proxies. In debug builds we check these return the same result. This adds some duplication but considering how hot this code is I think it's worth it.

* I made the (most common) PlainObject case a bit faster by avoiding the strcmp.

For the micro-benchmark below I get:

before: 786 ms
after:  536 ms

So about a 30% improvement.

function f() {
    var res = "";
    var objs = [{}, [1, 2], new Date, function(){}];
    var toString = Object.prototype.toString;
    var t = new Date;
    for (var i = 0; i < 5000000; i++) {
        for (var j = 0; j < 4; j++)
            res =[j]);
    print(new Date - t);
    return res;
Attachment #8881877 - Flags: review?(evilpies)
Comment on attachment 8881877 [details] [diff] [review]

Review of attachment 8881877 [details] [diff] [review]:

I am a bit surprised that reimplementing GetBuiltinClass makes such a huge difference, is that just call overhead?

::: js/src/builtin/Object.cpp
@@ +519,3 @@
>      } else {
> +        // Optimize the non-proxy case to bypass GetBuiltinClass.
> +        if (clasp == &PlainObject::class_ || clasp == &UnboxedPlainObject::class_) {

This needs some comment about this being an optimization.

@@ -516,5 @@
>          if (!builtinTag) {
>              const char* className = GetObjectClassName(cx, obj);
> -            // "[object Object]" is by far the most common case at this point,
> -            // so we optimize it here.
> -            if (strcmp(className, "Object") == 0) {

Some other classes also use Object like Date.prototype, but it's probably not that common.
Attachment #8881877 - Flags: review?(evilpies) → review+
(In reply to Tom Schuster [:evilpie] from comment #2)
> I am a bit surprised that reimplementing GetBuiltinClass makes such a huge
> difference, is that just call overhead?

Call overhead too but worst case GetBuiltinClass does 20 Class comparisons or so and then we return to obj_toString where we do a second switch on the ESClass value.
Pushed by
Optimize Object.prototype.toString. r=evilpie
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Significant effect on SMv2. Marking p1 for future reference.
Whiteboard: [qf:p1]
You need to log in before you can comment on or make changes to this bug.