From c9b3e45cca7a8d067c40d6d8edbc7b5276ed7229 Mon Sep 17 00:00:00 2001 From: "sandholm@chromium.org" Date: Thu, 8 Apr 2010 14:42:27 +0000 Subject: [PATCH] Adding boolean saveAnswer property of RegExpCache to avoid unnecessary cloning of the regexp answer object/array. Review URL: http://codereview.chromium.org/1556019 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@4364 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/regexp.js | 24 +++++++++++++++++------- src/string.js | 46 +++++++++++++++++++++++----------------------- 2 files changed, 40 insertions(+), 30 deletions(-) diff --git a/src/regexp.js b/src/regexp.js index 9adf978e12..f4252f908f 100644 --- a/src/regexp.js +++ b/src/regexp.js @@ -126,6 +126,9 @@ function RegExpCache() { this.replaceString = 0; this.lastIndex = 0; this.answer = 0; + // answerSaved marks whether the contents of answer is valid for a cache + // hit in RegExpExec, StringMatch and StringSplit. + this.answerSaved = false; } @@ -133,6 +136,7 @@ var regExpCache = new RegExpCache(); function CloneRegexpAnswer(array) { + if (array == null) return null; var len = array.length; var answer = new $Array(len); for (var i = 0; i < len; i++) { @@ -151,17 +155,19 @@ function RegExpExec(string) { } var cache = regExpCache; + var saveAnswer = false; if (%_ObjectEquals(cache.type, 'exec') && %_ObjectEquals(cache.lastIndex, this.lastIndex) && %_ObjectEquals(cache.regExp, this) && %_ObjectEquals(cache.subject, string)) { - var last = cache.answer; - if (last == null) { - return last; + if (cache.answerSaved) { + return CloneRegexpAnswer(cache.answer); } else { - return CloneRegexpAnswer(last); + saveAnswer = true; } + } else { + cache.answerSaved = false; } if (%_ArgumentsLength() == 0) { @@ -196,6 +202,7 @@ function RegExpExec(string) { cache.regExp = this; cache.subject = s; cache.answer = matchIndices; // Null. + cache.answerSaved = true; // Safe since no cloning is needed. cache.type = 'exec'; return matchIndices; // No match. } @@ -225,15 +232,18 @@ function RegExpExec(string) { result.input = s; if (this.global) { this.lastIndex = lastMatchInfo[CAPTURE1]; - return result; } else { cache.regExp = this; cache.subject = s; cache.lastIndex = lastIndex; - cache.answer = result; + if (saveAnswer) { + cache.answer = CloneRegexpAnswer(result); + cache.answerSaved = true; + } cache.type = 'exec'; - return CloneRegexpAnswer(result); } + return result; + } diff --git a/src/string.js b/src/string.js index 302dcdeb2e..300baf968b 100644 --- a/src/string.js +++ b/src/string.js @@ -170,15 +170,15 @@ function StringMatch(regexp) { if (!regexp.global) return regexp.exec(subject); var cache = regExpCache; + var saveAnswer = false; if (%_ObjectEquals(cache.type, 'match') && %_ObjectEquals(cache.regExp, regexp) && %_ObjectEquals(cache.subject, subject)) { - var last = cache.answer; - if (last == null) { - return last; + if (cache.answerSaved) { + return CloneRegexpAnswer(cache.answer); } else { - return CloneRegexpAnswer(last); + saveAnswer = true; } } @@ -188,12 +188,9 @@ function StringMatch(regexp) { cache.type = 'match'; cache.regExp = regexp; cache.subject = subject; - cache.answer = result; - if (result == null) { - return result; - } else { - return CloneRegexpAnswer(result); - } + if (saveAnswer) cache.answer = CloneRegexpAnswer(result); + cache.answerSaved = saveAnswer; + return result; } @@ -596,11 +593,16 @@ function StringSplit(separator, limit) { } var cache = regExpCache; + var saveAnswer = false; if (%_ObjectEquals(cache.type, 'split') && %_ObjectEquals(cache.regExp, separator) && %_ObjectEquals(cache.subject, subject)) { - return CloneRegexpAnswer(cache.answer); + if (cache.answerSaved) { + return CloneRegexpAnswer(cache.answer); + } else { + saveAnswer = true; + } } cache.type = 'split'; @@ -610,6 +612,7 @@ function StringSplit(separator, limit) { %_Log('regexp', 'regexp-split,%0S,%1r', [subject, separator]); if (length === 0) { + cache.answerSaved = true; if (splitMatch(separator, subject, 0, 0) != null) { cache.answer = []; return []; @@ -622,20 +625,19 @@ function StringSplit(separator, limit) { var startIndex = 0; var result = []; + outer_loop: while (true) { if (startIndex === length) { result[result.length] = subject.slice(currentIndex, length); - cache.answer = result; - return CloneRegexpAnswer(result); + break; } var matchInfo = splitMatch(separator, subject, currentIndex, startIndex); if (IS_NULL(matchInfo)) { result[result.length] = subject.slice(currentIndex, length); - cache.answer = result; - return CloneRegexpAnswer(result); + break; } var endIndex = matchInfo[CAPTURE1]; @@ -647,10 +649,7 @@ function StringSplit(separator, limit) { } result[result.length] = SubString(subject, currentIndex, matchInfo[CAPTURE0]); - if (result.length === limit) { - cache.answer = result; - return CloneRegexpAnswer(result); - } + if (result.length === limit) break; var num_captures = NUMBER_OF_CAPTURES(matchInfo); for (var i = 2; i < num_captures; i += 2) { @@ -661,14 +660,15 @@ function StringSplit(separator, limit) { } else { result[result.length] = void 0; } - if (result.length === limit) { - cache.answer = result; - return CloneRegexpAnswer(result); - } + if (result.length === limit) break outer_loop; } startIndex = currentIndex = endIndex; } + if (saveAnswer) cache.answer = CloneRegexpAnswer(result); + cache.answerSaved = saveAnswer; + return result; + }