Directly use C++ builtin of ArrayPush for String.prototype.split. (issue 17283007)

5 views
Skip to first unread message

yan...@chromium.org

unread,
Jun 19, 2013, 11:20:46 AM6/19/13
to sven...@chromium.org, v8-...@googlegroups.com
Reviewers: Sven Panne,

Description:
Directly use C++ builtin of ArrayPush for String.prototype.split.

R=sven...@chromium.org
BUG=v8:2737

Please review this at https://codereview.chromium.org/17283007/

SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge

Affected files:
M src/string.js


Index: src/string.js
diff --git a/src/string.js b/src/string.js
index
5db937bc77ca446b935cca394ef8366528006076..7e186871ba87547466eec7cc7eb2b714d3074e48
100644
--- a/src/string.js
+++ b/src/string.js
@@ -644,6 +644,8 @@ function StringSplit(separator, limit) {
}


+var ArrayPushBuiltin = $Array.prototype.push;
+
function StringSplitOnRegExp(subject, separator, limit, length) {
%_Log('regexp', 'regexp-split,%0S,%1r', [subject, separator]);

@@ -657,19 +659,21 @@ function StringSplitOnRegExp(subject, separator,
limit, length) {
var currentIndex = 0;
var startIndex = 0;
var startMatch = 0;
- var result = new InternalArray();
+ var result = [];

outer_loop:
while (true) {

if (startIndex === length) {
- result.push(%_SubString(subject, currentIndex, length));
+ %_CallFunction(result, %_SubString(subject, currentIndex, length),
+ ArrayPushBuiltin);
break;
}

var matchInfo = DoRegExpExec(separator, subject, startIndex);
if (matchInfo == null || length === (startMatch =
matchInfo[CAPTURE0])) {
- result.push(%_SubString(subject, currentIndex, length));
+ %_CallFunction(result, %_SubString(subject, currentIndex, length),
+ ArrayPushBuiltin);
break;
}
var endIndex = matchInfo[CAPTURE1];
@@ -680,7 +684,8 @@ function StringSplitOnRegExp(subject, separator, limit,
length) {
continue;
}

- result.push(%_SubString(subject, currentIndex, startMatch));
+ %_CallFunction(result, %_SubString(subject, currentIndex, startMatch),
+ ArrayPushBuiltin);

if (result.length === limit) break;

@@ -689,16 +694,17 @@ function StringSplitOnRegExp(subject, separator,
limit, length) {
var start = matchInfo[i++];
var end = matchInfo[i++];
if (end != -1) {
- result.push(%_SubString(subject, start, end));
+ %_CallFunction(result, %_SubString(subject, start, end),
+ ArrayPushBuiltin);
} else {
- result.push(void 0);
+ %_CallFunction(result, void 0, ArrayPushBuiltin);
}
if (result.length === limit) break outer_loop;
}

startIndex = currentIndex = endIndex;
}
- return %MoveArrayContents(result, []);
+ return result;
}




sven...@chromium.org

unread,
Jun 20, 2013, 4:02:06 AM6/20/13
to yan...@chromium.org, v8-...@googlegroups.com
LGTM. Just for the record: I tried a different approach, still using the
InternalArray, but with 'result[result.length] = foo' instead of
'result.push(foo)'. Funnily enough, this didn't show any significant
improvement. So moving the array contents seems to be slow, which is
strange...

https://codereview.chromium.org/17283007/

yan...@chromium.org

unread,
Jun 20, 2013, 4:13:27 AM6/20/13
to sven...@chromium.org, v8-...@googlegroups.com
Committed patchset #1 manually as r15223 (presubmit successful).

https://codereview.chromium.org/17283007/
Reply all
Reply to author
Forward
0 new messages