Some string optimizations:

Some string optimizations:

 * Inlined checks for strings and regexps.
 * Rewrote split for the non-regexp case.
 * Implemented one-char case for lastIndexOf.

Committed: http://code.google.com/p/v8/source/detail?r=3917

[Vitaly Repeshko, 2010-02-15 23:33:06]

Søren, Mads,

This change gives about 10% on http://dromaeo.com/?dromaeo-object-string. (Some
of the very low hanging fruits.) Unfortunately, the benchmark is quite noisy.
I'm checking whether the inlined checks are really worth the extra complexity.
If yes, I'll port to other architectures in this change.


Thanks,
Vitaly

[Søren Thygesen Gjesse, 2010-02-16 08:22:12]

LGTM

Have you looked at whether there is an increase in code size?

http://codereview.chromium.org/596122/diff/2001/3005
File src/runtime.cc (right):

http://codereview.chromium.org/596122/diff/2001/3005#newcode2495
src/runtime.cc:2495: FlattenString(sub);
Maybe this should still be a TryFlattenIfNotFlat and then skip the fast path if
flattening did not succeed.

http://codereview.chromium.org/596122/diff/2001/3005#newcode2515
src/runtime.cc:2515: }
Please make an else block for improved redability.

http://codereview.chromium.org/596122/diff/2001/3007
File src/string.js (right):

http://codereview.chromium.org/596122/diff/2001/3007#newcode742
src/string.js:742: return TO_STRING_INLINE(str).replace(/</g, "&lt;")
Either for space indentation or alignment.

[Vitaly Repeshko, 2010-02-17 18:09:12]

Ported to other architectures. Please take another look.


Thanks,
Vitaly

http://codereview.chromium.org/596122/diff/2001/3005
File src/runtime.cc (right):

http://codereview.chromium.org/596122/diff/2001/3005#newcode2495
src/runtime.cc:2495: FlattenString(sub);
On 2010/02/16 08:22:12, Søren Gjesse wrote:
> Maybe this should still be a TryFlattenIfNotFlat and then skip the fast path
if
> flattening did not succeed.

I think it should be kept consistent with indexOf implementation.

http://codereview.chromium.org/596122/diff/2001/3005#newcode2515
src/runtime.cc:2515: }
On 2010/02/16 08:22:12, Søren Gjesse wrote:
> Please make an else block for improved redability.

Done.

http://codereview.chromium.org/596122/diff/2001/3007
File src/string.js (right):

http://codereview.chromium.org/596122/diff/2001/3007#newcode742
src/string.js:742: return TO_STRING_INLINE(str).replace(/</g, "&lt;")
On 2010/02/16 08:22:12, Søren Gjesse wrote:
> Either for space indentation or alignment.

Done.

[Vitaly Repeshko, 2010-02-17 23:10:11]

On 2010/02/17 18:09:12, Vitaly wrote:
> Ported to other architectures. Please take another look.

Actually, no, wait. I just found a special case in codegen-ia32.cc
(VisitCompareOperation) that already generates (slightly different) inlined code
for typeof(foo) === 'bar'. This means the extra speed up comes only from the
fact that we avoid ToString calls. In other words I think inlining ToString
makes sense, but inlining the type checks doesn't since it's already done in the
compiler. I'll rework the change and remeasure performance.

-- Vitaly

[Vitaly Repeshko, 2010-02-18 18:01:10]

On 2010/02/17 23:10:11, Vitaly wrote:
> On 2010/02/17 18:09:12, Vitaly wrote:
> > Ported to other architectures. Please take another look.
> 
> Actually, no, wait. I just found a special case in codegen-ia32.cc
> (VisitCompareOperation) that already generates (slightly different) inlined
code
> for typeof(foo) === 'bar'. This means the extra speed up comes only from the
> fact that we avoid ToString calls. In other words I think inlining ToString
> makes sense, but inlining the type checks doesn't since it's already done in
the
> compiler. I'll rework the change and remeasure performance.

I removed the unnecessary %_IsString, but kept the fast case of ToString inlined
and kept %_IsRegExp.

Dromaeo strings benchmark still shows the same improvement.

Now, I think, the change is ready.


-- Vitaly

[Mads Ager (chromium), 2010-02-19 09:56:29]

LGTM

http://codereview.chromium.org/596122/diff/8001/8007
File src/macros.py (right):

http://codereview.chromium.org/596122/diff/8001/8007#newcode107
src/macros.py:107: macro TO_STRING_INLINE(arg)     = (IS_STRING(%IS_VAR(arg)) ?
arg : NonStringToString(arg));
Remove extra spaces before '=' on this line.  I would be in favor of removing
all of the alignment whitespace here, but you don't have to do that in this
change.

[Vitaly Repeshko, 2010-02-19 13:12:35]

Thanks! Submitted.


-- Vitaly

http://codereview.chromium.org/596122/diff/8001/8007
File src/macros.py (right):

http://codereview.chromium.org/596122/diff/8001/8007#newcode107
src/macros.py:107: macro TO_STRING_INLINE(arg)     = (IS_STRING(%IS_VAR(arg)) ?
arg : NonStringToString(arg));
On 2010/02/19 09:56:29, Mads Ager wrote:
> Remove extra spaces before '=' on this line.  I would be in favor of removing
> all of the alignment whitespace here, but you don't have to do that in this
> change.

Done.