add function to test whether string contents are definitely one byte

add function to test whether string contents are definitely one byte

R=yangguo@chromium.org
BUG=

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

[Yang, 2013-06-06 09:12:54]

Some comments, but looking good so far.

https://codereview.chromium.org/16530003/diff/1/include/v8.h
File include/v8.h (right):

https://codereview.chromium.org/16530003/diff/1/include/v8.h#newcode1537
include/v8.h:1537: bool DefinitelyContainsOnlyOneByte() const;
Is "Definitely" necessary? :)

https://codereview.chromium.org/16530003/diff/1/src/api.cc
File src/api.cc (right):

https://codereview.chromium.org/16530003/diff/1/src/api.cc#newcode4345
src/api.cc:4345: right_as_cons = NULL;
I think it's a bit easier to read if you just set cons_string at this point and
continue instead of falling through.

And isn't the recurse/iterate trick recursing over the shorter string and
iterating over the longer one, to reduce recursion depth? Example: maximally
unbalanced tree.

https://codereview.chromium.org/16530003/diff/1/src/api.cc#newcode4381
src/api.cc:4381: return helper.Check(*str);
If we find that the string only contains one byte chars, we could change the map
so that HasOnlyOneByteChars() returns true.

[dcarney, 2013-06-06 09:17:21]

https://codereview.chromium.org/16530003/diff/1/include/v8.h
File include/v8.h (right):

https://codereview.chromium.org/16530003/diff/1/include/v8.h#newcode1537
include/v8.h:1537: bool DefinitelyContainsOnlyOneByte() const;
On 2013/06/06 09:12:59, Yang wrote:
> Is "Definitely" necessary? :)

i'll kill it

https://codereview.chromium.org/16530003/diff/1/src/api.cc
File src/api.cc (right):

https://codereview.chromium.org/16530003/diff/1/src/api.cc#newcode4345
src/api.cc:4345: right_as_cons = NULL;
On 2013/06/06 09:12:59, Yang wrote:
> I think it's a bit easier to read if you just set cons_string at this point
and
> continue instead of falling through.

okay

> And isn't the recurse/iterate trick recursing over the shorter string and
> iterating over the longer one, to reduce recursion depth? Example: maximally
> unbalanced tree.

yes. this is why we have code reviews

https://codereview.chromium.org/16530003/diff/1/src/api.cc#newcode4381
src/api.cc:4381: return helper.Check(*str);
On 2013/06/06 09:12:59, Yang wrote:
> If we find that the string only contains one byte chars, we could change the
map
> so that HasOnlyOneByteChars() returns true.


will do

[dcarney, 2013-06-06 12:17:09]

okay, addressed comments.  added tests and random check which is relevant to
blink.

doing the map transitions is a bunch of work, which i'll do once this is in
blink and i can judge the need

[Yang, 2013-06-06 12:57:35]

LGTM with comments.

https://codereview.chromium.org/16530003/diff/6001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/16530003/diff/6001/src/objects.cc#newcode1212
src/objects.cc:1212: }
Is this really necessary? We compare the new resource with the old string
content right below. If the old (possibly two-byte) string has non-one-byte
chars, that would fail already.

https://codereview.chromium.org/16530003/diff/6001/test/cctest/test-api.cc
File test/cctest/test-api.cc (right):

https://codereview.chromium.org/16530003/diff/6001/test/cctest/test-api.cc#ne...
test/cctest/test-api.cc:16910: balanced = String::Concat(balanced, right);
I wouldn't call that quite balanced, but alright :)

[dcarney, 2013-06-06 13:11:03]

https://codereview.chromium.org/16530003/diff/6001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/16530003/diff/6001/src/objects.cc#newcode1212
src/objects.cc:1212: }
On 2013/06/06 12:57:36, Yang wrote:
> Is this really necessary? We compare the new resource with the old string
> content right below. If the old (possibly two-byte) string has non-one-byte
> chars, that would fail already.

the comparison below only compares lower bytes of the string

[Yang, 2013-06-06 13:11:47]

On 2013/06/06 13:11:03, dcarney wrote:
> https://codereview.chromium.org/16530003/diff/6001/src/objects.cc
> File src/objects.cc (right):
> 
> https://codereview.chromium.org/16530003/diff/6001/src/objects.cc#newcode1212
> src/objects.cc:1212: }
> On 2013/06/06 12:57:36, Yang wrote:
> > Is this really necessary? We compare the new resource with the old string
> > content right below. If the old (possibly two-byte) string has non-one-byte
> > chars, that would fail already.
> 
> the comparison below only compares lower bytes of the string

hm that's true. LGTM then.

[dcarney, 2013-06-06 13:16:59]

Committed patchset #2 manually as r14976 (presubmit successful).