Improve regexp split, replace and test.

Improve regexp split, replace and test.

[Lasse Reichstein, 2010-12-17 10:38:30]

L-sorta-GTM.
Most of the changes are ugly inlining of existing functions.
It shouldn't be necessary if Crankshaft could inline them.
Maybe work on getting them acceptable for automatical inlining instead of doing
it manually!

http://codereview.chromium.org/5959002/diff/1/src/regexp.js
File src/regexp.js (right):

http://codereview.chromium.org/5959002/diff/1/src/regexp.js#newcode265
src/regexp.js:265: (this.ignoreCase ? 'i' : '')
How about:
  (this.ignoreCase ? this.multiline ? "im" : "i" : this.multiline ? "m" : "")
to avoid the string-concatenation?

http://codereview.chromium.org/5959002/diff/1/src/regexp.js#newcode268
src/regexp.js:268: if (%_RegExpExec(regexp_val, string, 0, lastMatchInfo) ===
null) {
Use DoRegExpExec, or remember to clear lastMatchIndoOverride.

http://codereview.chromium.org/5959002/diff/1/src/string.js
File src/string.js (right):

http://codereview.chromium.org/5959002/diff/1/src/string.js#newcode257
src/string.js:257: if (next > 0) builder.elements.push(SubString(string, 0,
next));
Consider extracting elements from builder, so you don't need the
builder.elements redirection.
But it's a nasty hack to access the underlying array directly. It better be
significantly faster!

http://codereview.chromium.org/5959002/diff/1/src/string.js#newcode574
src/string.js:574: if (IS_NULL_OR_UNDEFINED(matchInfo)
Why accept undefined?

http://codereview.chromium.org/5959002/diff/1/src/string.js#newcode587
src/string.js:587: if (currentIndex + 1 == startMatch) {
Does this really pay off? It seems to be simply inlining the definition of
SubString, and we don't want to do that everywhere.

[sandholm, 2010-12-17 11:50:24]

http://codereview.chromium.org/5959002/diff/1/src/regexp.js
File src/regexp.js (right):

http://codereview.chromium.org/5959002/diff/1/src/regexp.js#newcode265
src/regexp.js:265: (this.ignoreCase ? 'i' : '')
On 2010/12/17 10:38:30, Lasse Reichstein wrote:
> How about:
>   (this.ignoreCase ? this.multiline ? "im" : "i" : this.multiline ? "m" : "")
> to avoid the string-concatenation?

Done.

http://codereview.chromium.org/5959002/diff/1/src/regexp.js#newcode268
src/regexp.js:268: if (%_RegExpExec(regexp_val, string, 0, lastMatchInfo) ===
null) {
On 2010/12/17 10:38:30, Lasse Reichstein wrote:
> Use DoRegExpExec, or remember to clear lastMatchIndoOverride.

The result is only used if there was no match. Otherwise, the "actual" exec call
is made below in which case the lastMatchInfoOverride gets overridden.

http://codereview.chromium.org/5959002/diff/1/src/string.js
File src/string.js (right):

http://codereview.chromium.org/5959002/diff/1/src/string.js#newcode257
src/string.js:257: if (next > 0) builder.elements.push(SubString(string, 0,
next));
On 2010/12/17 10:38:30, Lasse Reichstein wrote:
> Consider extracting elements from builder, so you don't need the
> builder.elements redirection.
> But it's a nasty hack to access the underlying array directly. It better be
> significantly faster!

Done.

http://codereview.chromium.org/5959002/diff/1/src/string.js#newcode574
src/string.js:574: if (IS_NULL_OR_UNDEFINED(matchInfo)
On 2010/12/17 10:38:30, Lasse Reichstein wrote:
> Why accept undefined?

I am just applying the same semantics as the inlined splitMatch function. I will
change it to (matchInfo == null) which seems easier on the eyes.

http://codereview.chromium.org/5959002/diff/1/src/string.js#newcode587
src/string.js:587: if (currentIndex + 1 == startMatch) {
On 2010/12/17 10:38:30, Lasse Reichstein wrote:
> Does this really pay off? It seems to be simply inlining the definition of
> SubString, and we don't want to do that everywhere.

It is faster. I generally only inline call sites where the SubString call is in
a loop.