http://codereview.chromium.org/10386090/diff/1/src/ia32/regexp-macro-assembler-ia32.h File src/ia32/regexp-macro-assembler-ia32.h (right):
http://codereview.chromium.org/10386090/diff/1/src/ia32/regexp-macro-assembler-ia32.h#newcode139 src/ia32/regexp-macro-assembler-ia32.h:139: // For the case of global regular expression, we have room to store more more than one -> at least one ? http://codereview.chromium.org/10386090/diff/1/src/ia32/regexp-macro-assembler-ia32.h#newcode160 src/ia32/regexp-macro-assembler-ia32.h:160: // The highest bit on the output registers count signals success. This lowest bit? http://codereview.chromium.org/10386090/diff/1/src/jsregexp.cc File src/jsregexp.cc (right): http://codereview.chromium.org/10386090/diff/1/src/jsregexp.cc#newcode200 src/jsregexp.cc:200: if (parse_result.simple && It would be interesting to remove this stuff so we never AtomCompile, but always use the regexp engine. With these changes it might be faster, and it would simplify things a lot. http://codereview.chromium.org/10386090/diff/1/src/jsregexp.cc#newcode904 src/jsregexp.cc:904: RegExpCompiler(int capture_count, You don't need this change. http://codereview.chromium.org/10386090/diff/1/src/jsregexp.cc#newcode981 src/jsregexp.cc:981: bool ignore_case, You don't need this change. http://codereview.chromium.org/10386090/diff/1/src/jsregexp.cc#newcode5796 src/jsregexp.cc:5796: RegExpCompiler compiler(data->capture_count, Or this. http://codereview.chromium.org/10386090/diff/1/src/regexp-macro-assembler.h File src/regexp-macro-assembler.h (right): http://codereview.chromium.org/10386090/diff/1/src/regexp-macro-assembler.h#newcode186 src/regexp-macro-assembler.h:186: // Set wether the regular expression has the global flag. Failures in a A wether is a castrated billy-goat :-) http://codereview.chromium.org/10386090/diff/1/src/regexp-macro-assembler.h#newcode187 src/regexp-macro-assembler.h:187: // global regexp may still mean success overal. overall. http://codereview.chromium.org/10386090/diff/1/src/runtime.cc File src/runtime.cc (right): http://codereview.chromium.org/10386090/diff/1/src/runtime.cc#newcode3798 src/runtime.cc:3798: static int SearchRegExpNoCaptureMultiple( I wonder if we can unify this and SearchRegExpMultiple without a performance hit. http://codereview.chromium.org/10386090/diff/1/src/runtime.cc#newcode3818 src/runtime.cc:3818: int result = How about giving this variable a name like number_of_matches, and getting rid of RE_SUCCESS and RE_FAILURE. Just comparing == 0 and > 0 makes sense if the variable has the right name. http://codereview.chromium.org/10386090/diff/1/src/runtime.cc#newcode3851 src/runtime.cc:3851: if (match_start != match_end) { I don't see this logic replicated in the regexp-macro-assembler. This suggests to me that we are missing good tests for it. http://codereview.chromium.org/10386090/diff/1/src/runtime.cc#newcode3855 src/runtime.cc:3855: if (pos > subject_length) { unneeded edit http://codereview.chromium.org/10386090/diff/1/src/runtime.cc#newcode3940 src/runtime.cc:3940: match = isolate->factory()->NewProperSubString(subject, Not sure why we need NewProperSubString here. http://codereview.chromium.org/10386090/diff/1/src/runtime.cc#newcode3949 src/runtime.cc:3949: for (int i = 1; i <= capture_count; i++) { I think you could just start this loop at 0 and remove some code duplication. http://codereview.chromium.org/10386090/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
