Bug 73154 - String.prototype.toLower should be optimized for 8 bit strings
: String.prototype.toLower should be optimized for 8 bit strings
Status: RESOLVED FIXED
: WebKit
JavaScriptCore
: 528+ (Nightly build)
: All All
: P2 Normal
Assigned To:
:
:
:
:
  Show dependency treegraph
 
Reported: 2011-11-25 21:16 PST by
Modified: 2011-11-26 09:40 PST (History)


Attachments
Patch (2.14 KB, patch)
2011-11-25 21:23 PST, Michael Saboff
fpizlo: review+
Review Patch | Details | Formatted Diff | Diff


Note

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


Description From 2011-11-25 21:16:03 PST
String.protoype.toLower() uses UString.characters() and should be optimized for the underlying character size.
------- Comment #1 From 2011-11-25 21:23:46 PST -------
Created an attachment (id=116647) [details]
Patch

This speeds up sun spider string-tagcloud by 7%.
------- Comment #2 From 2011-11-25 21:36:05 PST -------
(From update of attachment 116647 [details])
Can you put the perf effect in the ChangeLog?
------- Comment #3 From 2011-11-25 22:17:39 PST -------
Committed r101187: <http://trac.webkit.org/changeset/101187>
------- Comment #4 From 2011-11-26 08:30:38 PST -------
This removes the optimization for the case where the string was already all lowercase. It seems we could easily retain that by checking if the result of s.impl()->lower() is == s.impl() and if so, returning JSValue::encode(sVal).
------- Comment #5 From 2011-11-26 09:40:21 PST -------
(In reply to comment #4)
> This removes the optimization for the case where the string was already all lowercase. It seems we could easily retain that by checking if the result of s.impl()->lower() is == s.impl() and if so, returning JSValue::encode(sVal).

I had played with this a little and didn't get the return right so it failed some sputnik tests.  I just coded up a correct "already lower case" return and will be posting it in another bug.

It appears that the fast return is worth maybe a little in sun spider performance (.2ms overall).