Bug 73154 – String.prototype.toLower should be optimized for 8 bit strings

Bug 73154 - String.prototype.toLower should be optimized for 8 bit strings


Summary:	String.prototype.toLower should be optimized for 8 bit strings

Status:	RESOLVED FIXED

Product:	WebKit
Classification:	Unclassified
Component:	JavaScriptCore
Version:	528+ (Nightly build)
Platform:	All All

Importance:	P2 Normal
Assigned To:	Michael Saboff

URL:
Keywords:

Depends on:
Blocks:
	Show dependency tree / graph

Reported:	2011-11-25 21:16 PST by Michael Saboff
Modified:	2011-11-26 09:40 PST (History)
CC List:	1 user (show)

See Also:

Attachments
Patch (2.14 KB, patch) 2011-11-25 21:23 PST, Michael Saboff	fpizlo: review+	Details \| Formatted Diff \| Diff
View All Add an attachment (proposed patch, testcase, etc.)

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

Description Michael Saboff 2011-11-25 21:16:03 PST

String.protoype.toLower() uses UString.characters() and should be optimized for the underlying character size.

Comment 1 Michael Saboff 2011-11-25 21:23:46 PST

Created attachment 116647 [details]
Patch

This speeds up sun spider string-tagcloud by 7%.

Comment 2 Filip Pizlo 2011-11-25 21:36:05 PST

Comment on attachment 116647 [details]
Patch

Can you put the perf effect in the ChangeLog?

Comment 3 Michael Saboff 2011-11-25 22:17:39 PST

Committed r101187: <http://trac.webkit.org/changeset/101187>

Comment 4 Darin Adler 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 Michael Saboff 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).