Bug 20818

Summary:

'instanceof' operator is slow

Product:

WebKit

Reporter:

Cameron Zwarich (cpst) <zwarich>

Component:

JavaScriptCore

Assignee:

Maciej Stachowiak <mjs>

Status:

RESOLVED FIXED

Severity:

Normal

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Bug Depends on:

Bug Blocks:

20813

Attachments:

Description	Flags
enable PIC of the "prototype" property lookup, this saves half the time...	none
enable PIC of the "prototype" property lookup (this time for sure)	none
replace implementsHasInstance virtual call with a TypeInfo flag	none
use a TypeInfo bit to inline hasInfo and do the checks in a smarter order in the common case	none
inline JIT the fast paths of instanceof	zwarich: review+

Cameron Zwarich (cpst)

Reported 2008-09-12 17:30:10 PDT

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.

Attachments
enable PIC of the "prototype" property lookup, this saves half the time... (2.23 KB, patch) 2008-09-14 18:52 PDT, Maciej Stachowiak	no flags	Details Formatted Diff Diff
enable PIC of the "prototype" property lookup (this time for sure) (11.85 KB, patch) 2008-09-14 19:01 PDT, Maciej Stachowiak	no flags	Details Formatted Diff Diff
replace implementsHasInstance virtual call with a TypeInfo flag (15.21 KB, patch) 2008-09-22 07:55 PDT, Maciej Stachowiak	no flags	Details Formatted Diff Diff
use a TypeInfo bit to inline hasInfo and do the checks in a smarter order in the common case (15.44 KB, patch) 2008-09-22 19:24 PDT, Maciej Stachowiak	no flags	Details Formatted Diff Diff
inline JIT the fast paths of instanceof (8.10 KB, patch) 2008-09-23 10:02 PDT, Maciej Stachowiak	zwarich: review+	Details Formatted Diff Diff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.

Maciej Stachowiak

Comment 1 2008-09-14 18:52:39 PDT

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

Maciej Stachowiak

Comment 2 2008-09-14 18:53:49 PDT

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

Maciej Stachowiak

Comment 3 2008-09-14 19:01:28 PDT

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

Cameron Zwarich (cpst)

Comment 4 2008-09-14 19:07:01 PDT

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 5 2008-09-14 19:23:19 PDT

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

Comment 6 2008-09-22 07:55:07 PDT

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

Cameron Zwarich (cpst)

Comment 7 2008-09-22 08:00:59 PDT

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)

Comment 8 2008-09-22 11:23:02 PDT

(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

Comment 9 2008-09-22 19:20:06 PDT

Reopening since there is more speedup yet to come.

Maciej Stachowiak

Comment 10 2008-09-22 19:20:26 PDT

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

Comment 11 2008-09-22 19:24:59 PDT

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 12 2008-09-22 19:49:12 PDT

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

Comment 13 2008-09-23 10:02:33 PDT

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 14 2008-09-23 10:10:17 PDT

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

Comment 15 2008-09-23 20:48:28 PDT

OK, now I believe this is done.

Note You need to log in before you can comment on or make changes to this bug.