Optimizing BuildResultFromMatchInfo, StringReplace and StringSplit.

Optimizing BuildResultFromMatchInfo, StringReplace and StringSplit.

[Lasse Reichstein, 2010-12-13 09:29:12]

LGTM if you name your constants.

http://codereview.chromium.org/5708006/diff/2001/src/regexp.js
File src/regexp.js (right):

http://codereview.chromium.org/5708006/diff/2001/src/regexp.js#newcode126
src/regexp.js:126: if (start + 1 == end) {
Why not just lop from match 0/j = 3 instead of special-casting the first result?
You always perform the loop test anyway.

http://codereview.chromium.org/5708006/diff/2001/src/regexp.js#newcode131
src/regexp.js:131: var j = 5;
var j = REGEXP_FIRST_CAPTURE + 2;

http://codereview.chromium.org/5708006/diff/2001/src/string.js
File src/string.js (right):

http://codereview.chromium.org/5708006/diff/2001/src/string.js#newcode238
src/string.js:238: if (!IS_STRING(replace)) replace =
NonStringToString(replace);
Is this really faster than the macro:

macro TO_STRING_INLINE(arg) = (IS_STRING(%IS_VAR(arg)) ? arg :
NonStringToString(arg));

If so, our compiler should maybe detect assignments from a variable to itself.

http://codereview.chromium.org/5708006/diff/2001/src/string.js#newcode596
src/string.js:596: result.push(SubString(subject, currentIndex,
matchInfo[CAPTURE0]));
No special casing of this substring?

http://codereview.chromium.org/5708006/diff/2001/src/string.js#newcode599
src/string.js:599: var num_captures_plus3 = NUMBER_OF_CAPTURES(matchInfo) + 3;
Use REGEXP_FIRST_CAPTURE instead of 3.

http://codereview.chromium.org/5708006/diff/2001/src/string.js#newcode600
src/string.js:600: for (var i = 5; i < num_captures_plus3; ) {
And REGEXP_FIRST_CAPTURE + 2 instead of 5.

[sandholm, 2010-12-13 12:03:35]

Changed a few more bits. 
Please have another look.

http://codereview.chromium.org/5708006/diff/2001/src/regexp.js
File src/regexp.js (right):

http://codereview.chromium.org/5708006/diff/2001/src/regexp.js#newcode126
src/regexp.js:126: if (start + 1 == end) {
On 2010/12/13 09:29:13, Lasse Reichstein wrote:
> Why not just lop from match 0/j = 3 instead of special-casting the first
result?
> You always perform the loop test anyway.

The array lookup for start is re-used in the RegExpConstructResult call and the
'end != -1' check is not needed.

http://codereview.chromium.org/5708006/diff/2001/src/regexp.js#newcode131
src/regexp.js:131: var j = 5;
On 2010/12/13 09:29:13, Lasse Reichstein wrote:
> var j = REGEXP_FIRST_CAPTURE + 2;

Done.

http://codereview.chromium.org/5708006/diff/2001/src/string.js
File src/string.js (right):

http://codereview.chromium.org/5708006/diff/2001/src/string.js#newcode238
src/string.js:238: if (!IS_STRING(replace)) replace =
NonStringToString(replace);
On 2010/12/13 09:29:13, Lasse Reichstein wrote:
> Is this really faster than the macro:
> 
> macro TO_STRING_INLINE(arg) = (IS_STRING(%IS_VAR(arg)) ? arg :
> NonStringToString(arg));
> 
> If so, our compiler should maybe detect assignments from a variable to itself.

No, not really. I reverted this change.

http://codereview.chromium.org/5708006/diff/2001/src/string.js#newcode596
src/string.js:596: result.push(SubString(subject, currentIndex,
matchInfo[CAPTURE0]));
On 2010/12/13 09:29:13, Lasse Reichstein wrote:
> No special casing of this substring?

I have only inlined SubString calls inside loops.

http://codereview.chromium.org/5708006/diff/2001/src/string.js#newcode599
src/string.js:599: var num_captures_plus3 = NUMBER_OF_CAPTURES(matchInfo) + 3;
On 2010/12/13 09:29:13, Lasse Reichstein wrote:
> Use REGEXP_FIRST_CAPTURE instead of 3.

Done.

http://codereview.chromium.org/5708006/diff/2001/src/string.js#newcode600
src/string.js:600: for (var i = 5; i < num_captures_plus3; ) {
On 2010/12/13 09:29:13, Lasse Reichstein wrote:
> And REGEXP_FIRST_CAPTURE + 2 instead of 5.

Done.

[Lasse Reichstein, 2010-12-13 12:08:58]

LGTM

http://codereview.chromium.org/5708006/diff/9001/src/string.js
File src/string.js (right):

http://codereview.chromium.org/5708006/diff/9001/src/string.js#newcode590
src/string.js:590: var num_captures_plus3 =
"_plus3" might be a little too specific.
How about "matchinfo_length"?
(not to be confused with matchinfo.length, which may be longer :)

[sandholm, 2010-12-13 12:18:15]

http://codereview.chromium.org/5708006/diff/9001/src/string.js
File src/string.js (right):

http://codereview.chromium.org/5708006/diff/9001/src/string.js#newcode590
src/string.js:590: var num_captures_plus3 =
On 2010/12/13 12:08:58, Lasse Reichstein wrote:
> "_plus3" might be a little too specific.
> How about "matchinfo_length"?
> (not to be confused with matchinfo.length, which may be longer :)

Done.