Optimize StringImpl::lower() for 8-bit strings

Optimize StringImpl::lower() for 8-bit strings

The StringImpl::lower() method is used very intensively in blink, for example Intel Vtunes Amplifier profiler shows this method as a biggest hotspot by CPU usage viewpont when the 'AttributeDescendantSelector.html' CSS performance test is running.

This path optimizes cycles in StringImpl::lower() for 8-bit strings in order to avoid unneeded iterations and to re-use the already calculated data as much as possible.

After this patch is applied the 'AttributeDescendantSelector.html' performance test shows approx. 13% results improvement (linux desktop x64):

before
avg 561.7456430409343 runs/s
median 562.560813181673 runs/s
stdev 11.976426704572622 runs/s
min 529.8020262095021 runs/s
max 577.3626598774758 runs/s

after
avg 631.9779597646024 runs/s
median 636.3358333128829 runs/s
stdev 12.060471875689105 runs/s
min 602.4807143680587 runs/s
max 642.217924282401 runs/s

A unit test has been added to verify the method's behavior.

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=197102

[Mikhail, 2015-06-12 13:29:47]

mikhail.pozdnyakov@intel.com changed reviewers:
+ tkent@chromium.org

[Mikhail, 2015-06-12 13:29:48]

PTAL

[tkent, 2015-06-15 01:12:56]

lgtm

https://codereview.chromium.org/1181013008/diff/1/Source/wtf/text/StringImpl.cpp
File Source/wtf/text/StringImpl.cpp (right):

https://codereview.chromium.org/1181013008/diff/1/Source/wtf/text/StringImpl....
Source/wtf/text/StringImpl.cpp:506: unsigned needsLowering = m_length;
The name |needsLowering| sounds a boolean.  should be |firstIndexToBeLowered| or
something?

https://codereview.chromium.org/1181013008/diff/1/Source/wtf/text/WTFStringTe...
File Source/wtf/text/WTFStringTest.cpp (right):

https://codereview.chromium.org/1181013008/diff/1/Source/wtf/text/WTFStringTe...
Source/wtf/text/WTFStringTest.cpp:338: TEST(WTFString, lower)
TEST(StringTest, Lower)

https://codereview.chromium.org/1181013008/diff/1/Source/wtf/text/WTFStringTe...
Source/wtf/text/WTFStringTest.cpp:340: ASSERT_STREQ("link",
String("LINK").lower().ascii().data());
Use EXPECT_STREQ.
Same for the following lines.

[Mikhail, 2015-06-15 07:49:13]

On 2015/06/15 01:12:56, tkent wrote:
> lgtm
> 
>
https://codereview.chromium.org/1181013008/diff/1/Source/wtf/text/StringImpl.cpp
> File Source/wtf/text/StringImpl.cpp (right):
> 
>
https://codereview.chromium.org/1181013008/diff/1/Source/wtf/text/StringImpl....
> Source/wtf/text/StringImpl.cpp:506: unsigned needsLowering = m_length;
> The name |needsLowering| sounds a boolean.  should be |firstIndexToBeLowered|
or
> something?
> 
>
https://codereview.chromium.org/1181013008/diff/1/Source/wtf/text/WTFStringTe...
> File Source/wtf/text/WTFStringTest.cpp (right):
> 
>
https://codereview.chromium.org/1181013008/diff/1/Source/wtf/text/WTFStringTe...
> Source/wtf/text/WTFStringTest.cpp:338: TEST(WTFString, lower)
> TEST(StringTest, Lower)
> 
>
https://codereview.chromium.org/1181013008/diff/1/Source/wtf/text/WTFStringTe...
> Source/wtf/text/WTFStringTest.cpp:340: ASSERT_STREQ("link",
> String("LINK").lower().ascii().data());
> Use EXPECT_STREQ.
> Same for the following lines.

done, thanks for review!

[Mikhail, 2015-06-15 07:49:30]

The CQ bit was checked by mikhail.pozdnyakov@intel.com

[Mikhail, 2015-06-15 07:49:30]

The patchset sent to the CQ was uploaded after l-g-t-m from tkent@chromium.org
Link to the patchset: https://codereview.chromium.org/1181013008/#ps20001
(title: "Updated according to review comments")

[commit-bot: I haz the power, 2015-06-15 07:49:45]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1181013008/20001

[commit-bot: I haz the power, 2015-06-15 07:51:59]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-06-15 07:52:00]

Try jobs failed on following builders:
  mac_blink_compile_dbg on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/mac_blink_compile_dbg/bu...)

[Mikhail, 2015-06-15 08:28:12]

The CQ bit was checked by mikhail.pozdnyakov@intel.com

[Mikhail, 2015-06-15 08:28:12]

The patchset sent to the CQ was uploaded after l-g-t-m from tkent@chromium.org
Link to the patchset: https://codereview.chromium.org/1181013008/#ps40001
(title: "rebased")

[commit-bot: I haz the power, 2015-06-15 08:28:21]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1181013008/40001

[commit-bot: I haz the power, 2015-06-15 08:49:05]

Committed patchset #3 (id:40001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=197102