1376799 - Optimize Object.prototype.toString more

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement)

Description

[Jan de Mooij [:jandem]]

Bug 1369042 was a decent win, but there's a bunch more slowness to eliminate here.

Comment 1

[Jan de Mooij [:jandem]]

Attachment: Patch

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 = toString.call(objs[j]);
    }
    print(new Date - t);
    return res;
}
f();

Comment 2

[Tom S [:evilpie]]

Comment on attachment 8881877 [details] [diff] [review]
Patch

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.

Comment 3

[Jan de Mooij [:jandem]]

(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.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c199e7e82e60
Optimize Object.prototype.toString. r=evilpie

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c199e7e82e60

Comment 6

[:Harald Kirschner :digitarald]

Significant effect on SMv2. Marking p1 for future reference.