Bug 73154

Summary: String.prototype.toLower should be optimized for 8 bit strings
Product: WebKit Reporter: Michael Saboff <msaboff>
Component: JavaScriptCoreAssignee: Michael Saboff <msaboff>
Status: RESOLVED FIXED    
Severity: Normal CC: darin
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch fpizlo: review+

Michael Saboff
Reported 2011-11-25 21:16:03 PST
String.protoype.toLower() uses UString.characters() and should be optimized for the underlying character size.
Attachments
Patch (2.14 KB, patch)
2011-11-25 21:23 PST, Michael Saboff 		fpizlo: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Michael Saboff
Comment 1 2011-11-25 21:23:46 PST
Created attachment 116647 [details] Patch This speeds up sun spider string-tagcloud by 7%.
Filip Pizlo
Comment 2 2011-11-25 21:36:05 PST
Comment on attachment 116647 [details] Patch Can you put the perf effect in the ChangeLog?
Michael Saboff
Comment 3 2011-11-25 22:17:39 PST
Committed r101187: <http://trac.webkit.org/changeset/101187>
Darin Adler
Comment 4 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).
Michael Saboff
Comment 5 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).
Note You need to log in before you can comment on or make changes to this bug.