Bug 20333

Summary: improve JavaScript speed when handling single-character strings
Product: WebKit Reporter: Darin Adler <darin>
Component: JavaScriptCoreAssignee: Darin Adler <darin>
Status: RESOLVED FIXED    
Severity: Enhancement CC: ap, ggaren, zwarich
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.5   
Attachments:
Description Flags
some early work in progress
none
eliminate JSValue::type()
none
remove the need for InternalFunction to return a UString& to its name
none
work in progress
none
patch mjs: review+

Description Darin Adler 2008-08-08 13:07:32 PDT
We think this will work!
Comment 1 Darin Adler 2008-08-08 13:08:00 PDT
Created attachment 22708 [details]
some early work in progress
Comment 2 Darin Adler 2008-08-12 21:26:34 PDT
Created attachment 22766 [details]
eliminate JSValue::type()
Comment 3 Geoffrey Garen 2008-08-15 00:10:20 PDT
Comment on attachment 22766 [details]
eliminate JSValue::type()

r=me

Please try not to conflict with bug 20389.

It looks like you re-autogenerated a bunch of SVG tests, too. Was that intentional?

+        if (v1 == JSImmediate::from(0))
+            return !JSImmediate::isImmediate(v2)
+                && static_cast<JSCell*>(v2)->isNumber()
+                && static_cast<JSNumberCell*>(v2)->value() == 0;
+        return v2 == JSImmediate::from(0)
+            && !JSImmediate::isImmediate(v1)
+            && static_cast<JSCell*>(v1)->isNumber()
+            && static_cast<JSNumberCell*>(v1)->value() == 0;

This surprised me a little bit. Is there a case where 0 gets stored on the heap instead of as an immediate? How do we know this doesn't happen with other numbers?
Comment 4 Darin Adler 2008-08-15 08:49:06 PDT
(In reply to comment #3)
> It looks like you re-autogenerated a bunch of SVG tests, too. Was that
> intentional?

No. That was an accident and I didn't notice it when creating the patch.

> +        if (v1 == JSImmediate::from(0))
> +            return !JSImmediate::isImmediate(v2)
> +                && static_cast<JSCell*>(v2)->isNumber()
> +                && static_cast<JSNumberCell*>(v2)->value() == 0;
> +        return v2 == JSImmediate::from(0)
> +            && !JSImmediate::isImmediate(v1)
> +            && static_cast<JSCell*>(v1)->isNumber()
> +            && static_cast<JSNumberCell*>(v1)->value() == 0;
> 
> This surprised me a little bit. Is there a case where 0 gets stored on the heap
> instead of as an immediate? How do we know this doesn't happen with other
> numbers?

This is to handle negative 0.
Comment 5 Darin Adler 2008-08-20 10:24:59 PDT
Comment on attachment 22766 [details]
eliminate JSValue::type()

Clearing review flag since this patch was landed and the bug is not yet fixed.
Comment 6 Darin Adler 2008-08-21 15:14:33 PDT
Created attachment 22926 [details]
remove the need for InternalFunction to return a UString& to its name

Geoff, this is what we talked about.
Comment 7 Cameron Zwarich (cpst) 2008-08-22 17:33:41 PDT
Comment on attachment 22926 [details]
remove the need for InternalFunction to return a UString& to its name

I don't think you want to commit the first change to the XCode project, because Geoff committed changes to all of the XCode projects to update them to XCode 3.1 and that undoes one of them. However, I am no XCode wizard, so I may be wrong.

Otherwise r=me.
Comment 8 Darin Adler 2008-08-29 17:26:06 PDT
Comment on attachment 22926 [details]
remove the need for InternalFunction to return a UString& to its name

I've decided not to make this change at this time. I'm taking a different approach to optimizing the single character case.
Comment 9 Darin Adler 2008-08-29 17:26:37 PDT
Created attachment 23077 [details]
work in progress
Comment 10 Darin Adler 2008-08-30 16:04:13 PDT
Created attachment 23083 [details]
patch
Comment 11 Maciej Stachowiak 2008-08-30 19:03:57 PDT
r=me

Consider breaking SmallStringStorage into its own header and implementation file.