Bug 20820

Summary: op_stricteq is slow
Product: WebKit Reporter: Cameron Zwarich (cpst) <zwarich>
Component: JavaScriptCoreAssignee: Maciej Stachowiak <mjs>
Status: RESOLVED FIXED    
Severity: Normal    
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 20813    
Attachments:
Description Flags
more speedups for the CTI case
none
inline JIT the fast paths of the strict equal operator oliver: review+

Description Cameron Zwarich (cpst) 2008-09-12 18:04:37 PDT
op_stricteq is 1.7% of the Shark profile of the Earley-Boyer benchmark. We should determine why this is the case and fix it.
Comment 1 Maciej Stachowiak 2008-09-20 22:15:30 PDT
Created attachment 23619 [details]
more speedups for the CTI case
Comment 2 Darin Adler 2008-09-20 22:26:22 PDT
Comment on attachment 23619 [details]
more speedups for the CTI case

+        (JSC::Machine::cti_op_stricteq): Check for pointer equality first; use inline version
+        of strictEqualSlowCase; remove unneeded exception check.

I don't see any "check for pointer equality first" change.

-            r[dst] = jsBoolean(reinterpret_cast<intptr_t>(src1) == reinterpret_cast<intptr_t>(src2));
+            r[dst]= jsBoolean(reinterpret_cast<intptr_t>(src1) == reinterpret_cast<intptr_t>(src2));

Should change this back. No reason to delete the space before that "=".

-    if (JSImmediate::isEitherImmediate(src1, src2) & (src1 != JSImmediate::from(0)) & (src2 != JSImmediate::from(0)))
+    if (JSImmediate::isEitherImmediate(src1, src2) & (src1 != JSImmediate::zeroImmediate()) & (src2 != JSImmediate::zeroImmediate()))

JSImmediate::from(0) doesn't get constant folded?

r=me
Comment 3 Maciej Stachowiak 2008-09-20 22:53:36 PDT
(In reply to comment #2)
> (From update of attachment 23619 [details] [edit])
> +        (JSC::Machine::cti_op_stricteq): Check for pointer equality first; use
> inline version
> +        of strictEqualSlowCase; remove unneeded exception check.
> 
> I don't see any "check for pointer equality first" change.

Oh right, I had to back that out after writing the ChangeLog because it was wrong for NaN. I'll fix ChangeLog too.

> 
> -            r[dst] = jsBoolean(reinterpret_cast<intptr_t>(src1) ==
> reinterpret_cast<intptr_t>(src2));
> +            r[dst]= jsBoolean(reinterpret_cast<intptr_t>(src1) ==
> reinterpret_cast<intptr_t>(src2));
> 
> Should change this back. No reason to delete the space before that "=".
>

I fixed that.
 
> -    if (JSImmediate::isEitherImmediate(src1, src2) & (src1 !=
> JSImmediate::from(0)) & (src2 != JSImmediate::from(0)))
> +    if (JSImmediate::isEitherImmediate(src1, src2) & (src1 !=
> JSImmediate::zeroImmediate()) & (src2 != JSImmediate::zeroImmediate()))
> 
> JSImmediate::from(0) doesn't get constant folded?
> 

I think both get constant folded, but I figued it was better style to use zeroImmediate().
Comment 4 Maciej Stachowiak 2008-09-21 03:29:44 PDT
Created attachment 23624 [details]
inline JIT the fast paths of the strict equal operator
Comment 5 Maciej Stachowiak 2008-09-21 03:30:00 PDT
Comment on attachment 23619 [details]
more speedups for the CTI case

unflagging since this is landed
Comment 6 Darin Adler 2008-09-22 14:07:16 PDT
http://trac.webkit.org/changeset/36738