LGTM.
Another possible optimization is to not find all the indices first, but do
it
inline. It saves the intermediate data structure. In that case you don't
know
the length of the resulting string, but it's often sufficient to allocate a
string at the end of newspace, and extend or crop it as needed.
At least in the case where the replacement's length is the same as the
pattern's
length, you do know the exact length of the result before finding matches,
but
specializing for just that case might be a little too specific.
http://codereview.chromium.org/7782028/diff/1/src/runtime.cc
File src/runtime.cc (right):
http://codereview.chromium.org/7782028/diff/1/src/runtime.cc#newcode2874
src/runtime.cc:2874: Handle<String> replacement =
Handle<String>::null()) {
Don't use an optional argument, just pass the null-handle in the few
cases where we replace with the empty string.
http://codereview.chromium.org/7782028/diff/1/src/runtime.cc#newcode2881
src/runtime.cc:2881:
String::cast(pattern_regexp->DataAt(JSRegExp::kAtomPatternIndex));
Assert that the regexp is atomic.
http://codereview.chromium.org/7782028/diff/1/src/runtime.cc#newcode2906
src/runtime.cc:2906: String::WriteToFlat(*subject,
Would it be worth it to check that that subject_pos is actually less
than indices.at(i)? I.e, there is something to write.
http://codereview.chromium.org/7782028/diff/1/src/runtime.cc#newcode2911
src/runtime.cc:2911: // Replace match.
Move comment down one line.
http://codereview.chromium.org/7782028/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
