LGTM The long comment is more an idea for what to look at next, you don't have to address it for this change.
http://codereview.chromium.org/1367004/diff/1/3 File src/conversions.cc (right): http://codereview.chromium.org/1367004/diff/1/3#newcode458 src/conversions.cc:458: ++current; This is changing the implementation for strings that end in '.'. Do we have good test coverage for that? http://codereview.chromium.org/1367004/diff/1/3#newcode587 src/conversions.cc:587: // fractional part it could be parsed fatser (without check for fatser -> faster check -> checks http://codereview.chromium.org/1367004/diff/1/3#newcode588 src/conversions.cc:588: // spaces, overflow, etc). etc -> etc. http://codereview.chromium.org/1367004/diff/1/3#newcode608 src/conversions.cc:608: } else if (!fractional_part && significant_digits <= kMaxDigitsInInt) { I like this but I wonder whether it can't be done even faster by moving up the check for really simple cases into runtime.cc. In this way we could avoid making doubles and then converting back into integers. We should just be able to return a Smi immediately in the simple case. There's also the situation where a string is a positive integer where the hash has been calculated. In this case the iteger value can be got without calculations. See String::AsArrayIndex and String::SlowAsArrayIndex. This will be the case for all symbols (and all single-character strings '0' - '9') as well as all two-character strings '10' - '99' made with +. http://codereview.chromium.org/1367004 -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev To unsubscribe from this group, send email to v8-dev+unsubscribegooglegroups.com or reply to this email with the words "REMOVE ME" as the subject.
