Bug 20818 – 'instanceof' operator is slow

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Cameron Zwarich (cpst)]

JSObject::hasInstance() shows up as 3.1% of the Shark profile for the V8 Earley-Boyer benchmark, so we should try to speed it up a bit. The isObject() check in the op_instanceof also shows up as 2.4% of the Shark profile, but I will make a separate bug for that.

[Maciej Stachowiak]

Created attachment 23427 [details]
enable PIC of the "prototype" property lookup, this saves half the time...

[Maciej Stachowiak]

(This will probably require more work to save the rest of the time).

[Maciej Stachowiak]

Created attachment 23428 [details]
enable PIC of the "prototype" property lookup (this time for sure)

[Cameron Zwarich (cpst)]

Comment on attachment 23428 [details]
enable PIC of the "prototype" property lookup (this time for sure)

r=me

You have a double newline on line 818 of CodeGenerator.cpp.

[Maciej Stachowiak]

Comment on attachment 23428 [details]
enable PIC of the "prototype" property lookup (this time for sure)

Unflagging and obsoleting because this has been landed.

[Maciej Stachowiak]

Created attachment 23650 [details]
replace implementsHasInstance virtual call with a TypeInfo flag

[Cameron Zwarich (cpst)]

Comment on attachment 23650 [details]
replace implementsHasInstance virtual call with a TypeInfo flag

r=me, assuming you've removed the tab in CodeGeneratorJS.pm and you put the speedup in the ChangeLog.

[David Kilzer (:ddkilzer)]

(In reply to comment #7)
> (From update of attachment 23650 [details] [edit])
> r=me, assuming you've removed the tab in CodeGeneratorJS.pm and you put the
> speedup in the ChangeLog.

http://trac.webkit.org/changeset/36766

[Maciej Stachowiak]

Reopening since there is more speedup yet to come.

[Maciej Stachowiak]

Comment on attachment 23650 [details]
replace implementsHasInstance virtual call with a TypeInfo flag

Unflag and obsolete this patch since it ahs been landed.

[Maciej Stachowiak]

Created attachment 23690 [details]
use a TypeInfo bit to inline hasInfo and do the checks in a smarter order in the common case

[Darin Adler]

Comment on attachment 23690 [details]
use a TypeInfo bit to inline hasInfo and do the checks in a smarter order in the common case

From the caller's point of view, customHasInstance is still just hasInstance, so I'm not sure it was an improvement to rename it.

Since you wrote different hasInstance code in cti_op_instanceof, there's no need for the standardHasInstance inline function, so you could have also just left it  alone.

You could have named the flag OverridesHasInstance.

This:

    valueCell->structureID()->typeInfo().type() != ObjectType

Is this same as !isObject(). Why did you write it the longer way?

r=me with or without changes

[Maciej Stachowiak]

Created attachment 23705 [details]
inline JIT the fast paths of instanceof

I am not sure it is a good idea to emit quite so much code inline for instanceof.

[Cameron Zwarich (cpst)]

Comment on attachment 23705 [details]
inline JIT the fast paths of instanceof

Your ASSERT on Machine.cpp:4203 is in the wrong style; the ||'s should be on the beginning of the lines, not the end.

Other than that, r=me.

[Darin Adler]

OK, now I believe this is done.