Improve string library performance. String concat and indexOf.

Improve string library performance. String concat and indexOf.
Rename String concatAll to concatRange. Optimize concatRange for OneByteStrings (avoids natives), optimize indexOf for one character onByteStrings.

R=iposva@google.com

Committed: https://code.google.com/p/dart/source/detail?r=29472

[Ivan Posva, 2013-10-28 23:35:09]

LGTMwC

-Ivan

https://codereview.chromium.org/42443002/diff/380001/runtime/lib/string_patch...
File runtime/lib/string_patch.dart (right):

https://codereview.chromium.org/42443002/diff/380001/runtime/lib/string_patch...
runtime/lib/string_patch.dart:87: // TODO(5413632): Compare hash codes when both
are present.
I don't think comparing hash codes is effective, as I don't think you get them
frequently enough. So we might want to just remove this TODO.

https://codereview.chromium.org/42443002/diff/380001/runtime/lib/string_patch...
runtime/lib/string_patch.dart:92: final int len = this.length;
?

https://codereview.chromium.org/42443002/diff/380001/runtime/lib/string_patch...
runtime/lib/string_patch.dart:519: if (start == 0 && (end == numValues)) {
(start == 0)

https://codereview.chromium.org/42443002/diff/380001/runtime/lib/string_patch...
runtime/lib/string_patch.dart:592: final int len = this.length;
Not sure whether we should be using this. or not. Also no types inside methods.

https://codereview.chromium.org/42443002/diff/380001/runtime/lib/string_patch...
runtime/lib/string_patch.dart:594: if ((pattern._cid == _OneByteString._classId)
&&
What about an ExternalOneByteString?

https://codereview.chromium.org/42443002/diff/380001/runtime/lib/string_patch...
runtime/lib/string_patch.dart:597: final int patternCu0 = pattern.codeUnitAt(0);
Another quick test you could do: If the pattern is outside of the one byte
string range then you will not find it.

[srdjan, 2013-10-29 17:02:29]

https://codereview.chromium.org/42443002/diff/380001/runtime/lib/string_patch...
File runtime/lib/string_patch.dart (right):

https://codereview.chromium.org/42443002/diff/380001/runtime/lib/string_patch...
runtime/lib/string_patch.dart:87: // TODO(5413632): Compare hash codes when both
are present.
On 2013/10/28 23:35:09, Ivan Posva wrote:
> I don't think comparing hash codes is effective, as I don't think you get them
> frequently enough. So we might want to just remove this TODO.

Done.

https://codereview.chromium.org/42443002/diff/380001/runtime/lib/string_patch...
runtime/lib/string_patch.dart:92: final int len = this.length;
On 2013/10/28 23:35:09, Ivan Posva wrote:
> ?

Removed type.

https://codereview.chromium.org/42443002/diff/380001/runtime/lib/string_patch...
runtime/lib/string_patch.dart:519: if (start == 0 && (end == numValues)) {
On 2013/10/28 23:35:09, Ivan Posva wrote:
> (start == 0)

Done.

https://codereview.chromium.org/42443002/diff/380001/runtime/lib/string_patch...
runtime/lib/string_patch.dart:592: final int len = this.length;
On 2013/10/28 23:35:09, Ivan Posva wrote:
> Not sure whether we should be using this. or not. Also no types inside
methods.

The name' length' is too generic, therefore I feel it makes in this case for a
more readable code. Let me know if you disagree.

https://codereview.chromium.org/42443002/diff/380001/runtime/lib/string_patch...
runtime/lib/string_patch.dart:594: if ((pattern._cid == _OneByteString._classId)
&&
On 2013/10/28 23:35:09, Ivan Posva wrote:
> What about an ExternalOneByteString?

Added: TODO. The inner loop should stay monomorphic. Will need to add a test.

https://codereview.chromium.org/42443002/diff/380001/runtime/lib/string_patch...
runtime/lib/string_patch.dart:597: final int patternCu0 = pattern.codeUnitAt(0);
On 2013/10/28 23:35:09, Ivan Posva wrote:
> Another quick test you could do: If the pattern is outside of the one byte
> string range then you will not find it.

Good idea, once I allow two byte strings.

[srdjan, 2013-10-29 18:18:48]

Committed patchset #9 manually as r29472 (presubmit successful).