Parsing some kinds of integers and junk values in Runtime_NumberToString. For...

Parsing some kinds of integers and junk values in Runtime_NumberToString. For that values the string is converted into int and then into SMI. It lets to avoid floating point operations.


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

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

[SeRya, 2010-03-30 12:44:39]

http://compute3.aar:9013/golem/r4309-v8-serya-SmiParsing-vs-4309-v8.html

[Florian Loitsch, 2010-03-31 10:56:52]

LGTM

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

http://codereview.chromium.org/1572004/diff/1/2#newcode4481
src/runtime.cc:4481: // the 'I' character ('Infinity'). All of that have codes
greater than '9'
All of that have codes less than '9'

http://codereview.chromium.org/1572004/diff/1/2#newcode4486
src/runtime.cc:4486: } else if (len - start_pos < 10 && AreDigits(data,
start_pos, len)) {
Maybe add comment (or similar):
The maximal/minimal smi has 10 digits. If the string has less digits we know it
will fit into the smi-data type.

[SeRya, 2010-03-31 11:19:58]

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

http://codereview.chromium.org/1572004/diff/1/2#newcode4481
src/runtime.cc:4481: // the 'I' character ('Infinity'). All of that have codes
greater than '9'
On 2010/03/31 10:56:52, Florian Loitsch wrote:
> All of that have codes less than '9'

Actually 'not greater than'. Fixed.

http://codereview.chromium.org/1572004/diff/1/2#newcode4486
src/runtime.cc:4486: } else if (len - start_pos < 10 && AreDigits(data,
start_pos, len)) {
On 2010/03/31 10:56:52, Florian Loitsch wrote:
> Maybe add comment (or similar):
> The maximal/minimal smi has 10 digits. If the string has less digits we know
it
> will fit into the smi-data type.

Done.

[Tobias Kaes, 2010-03-31 14:31:27]

http://codereview.chromium.org/1572004/diff/8001/9001
File src/runtime.cc (right):

http://codereview.chromium.org/1572004/diff/8001/9001#newcode4483
src/runtime.cc:4483: if (data[start_pos] != 'I') {
Sorry for the noise, just wondered if that works if start_pos == len. Are all
strings guaranteed to be null terminated?

[SeRya, 2010-03-31 15:16:14]

http://codereview.chromium.org/1572004/diff/8001/9001
File src/runtime.cc (right):

http://codereview.chromium.org/1572004/diff/8001/9001#newcode4483
src/runtime.cc:4483: if (data[start_pos] != 'I') {
On 2010/03/31 14:31:27, Tobias Kaes wrote:
> Sorry for the noise, just wondered if that works if start_pos == len. Are all
> strings guaranteed to be null terminated?

I guess it is not. In worse case it could cause unreasonable fall into the slow
case. Anyway, memory outside the string data shouldn't be accessed.

[Tobias Kaes, 2010-04-02 13:59:22]

> On 2010/03/31 14:31:27, Tobias Kaes wrote:
> > Sorry for the noise, just wondered if that works if start_pos == len. Are
all
> > strings guaranteed to be null terminated?

On 2010/03/31 15:16:14, SeRya wrote:
> I guess it is not. In worse case it could cause unreasonable fall into the
slow
> case. Anyway, memory outside the string data shouldn't be accessed.

Did the change make it into svn? If not someone should make sure it doesn't get
lost.

[Florian Loitsch, 2010-04-03 12:55:41]

Checking for the empty string LGTM.
I can't comment on the other change in the regexp part.

http://codereview.chromium.org/1572004/diff/17001/18001
File src/runtime.cc (right):

http://codereview.chromium.org/1572004/diff/17001/18001#newcode3360
src/runtime.cc:3360: int start = register_vector[i * 2];
unrelated patch?

[SeRya, 2010-04-05 07:54:35]

http://codereview.chromium.org/1572004/diff/17001/18001
File src/runtime.cc (right):

http://codereview.chromium.org/1572004/diff/17001/18001#newcode3360
src/runtime.cc:3360: int start = register_vector[i * 2];
On 2010/04/03 12:55:41, Florian Loitsch wrote:
> unrelated patch?

This change came with update (it is not shown as a difference in my working
copy).