Recursion limit for one-char string replace and retire String::kMinNonFlatLength.

Recursion limit for one-char string replace and retire String::kMinNonFlatLength.


TEST=mjsunit/string-replace-one-char.js

Committed: https://code.google.com/p/v8/source/detail?r=10422

[Yang, 2012-01-17 13:07:34]

PTAL.

http://codereview.chromium.org/9231017/diff/1/src/runtime.cc
File src/runtime.cc (right):

http://codereview.chromium.org/9231017/diff/1/src/runtime.cc#newcode3290
src/runtime.cc:3290: const int kRecursionLimit = 0x1000;
Recursion limit is chosen so that regexpdna still benefits from this fast path.

http://codereview.chromium.org/9231017/diff/1/src/string.js
File src/string.js (right):

http://codereview.chromium.org/9231017/diff/1/src/string.js#newcode248
src/string.js:248: subject.length > 0xFF &&
Dealing with short cons string the old way is better than traversing it.  This
especially makes celtic kane happy.

[Erik Corry, 2012-01-17 13:50:23]

LGTM

http://codereview.chromium.org/9231017/diff/1/src/runtime.cc
File src/runtime.cc (right):

http://codereview.chromium.org/9231017/diff/1/src/runtime.cc#newcode3292
src/runtime.cc:3292: for (int i = 0; i < 2; i++) {
I think this would be clearer without the loop, just duplicating the call.

http://codereview.chromium.org/9231017/diff/1/test/mjsunit/string-replace-one...
File test/mjsunit/string-replace-one-char.js (right):

http://codereview.chromium.org/9231017/diff/1/test/mjsunit/string-replace-one...
test/mjsunit/string-replace-one-char.js:39: "$", "###");
It would be nice to test with a replacement string like "x$0x" to see that the
'$' detection is working.

[Erik Corry, 2012-01-17 13:51:21]

http://codereview.chromium.org/9231017/diff/1/src/runtime.cc
File src/runtime.cc (right):

http://codereview.chromium.org/9231017/diff/1/src/runtime.cc#newcode3300
src/runtime.cc:3300: if (!result.is_null()) return *result;
Should you have a test for found == false here and then return the original
string?

[Yang, 2012-01-17 14:29:04]

http://codereview.chromium.org/9231017/diff/1/src/runtime.cc
File src/runtime.cc (right):

http://codereview.chromium.org/9231017/diff/1/src/runtime.cc#newcode3292
src/runtime.cc:3292: for (int i = 0; i < 2; i++) {
On 2012/01/17 13:50:24, Erik Corry wrote:
> I think this would be clearer without the loop, just duplicating the call.

Done.

http://codereview.chromium.org/9231017/diff/1/src/runtime.cc#newcode3300
src/runtime.cc:3300: if (!result.is_null()) return *result;
On 2012/01/17 13:51:21, Erik Corry wrote:
> Should you have a test for found == false here and then return the original
> string?

Allocation is only ever done if found==true. In all other cases, either a
null-handle or the original subject string is returned, so there is no need for
an additional check.

http://codereview.chromium.org/9231017/diff/1/test/mjsunit/string-replace-one...
File test/mjsunit/string-replace-one-char.js (right):

http://codereview.chromium.org/9231017/diff/1/test/mjsunit/string-replace-one...
test/mjsunit/string-replace-one-char.js:39: "$", "###");
On 2012/01/17 13:50:24, Erik Corry wrote:
> It would be nice to test with a replacement string like "x$0x" to see that the
> '$' detection is working.

Done. I don't know what I was thinking.