Avoid unnecessary cloning of regexp answer

Adding boolean saveAnswer property of RegExpCache to avoid unnecessary cloning of the regexp answer object/array.

Committed: http://code.google.com/p/v8/source/detail?r=4364

[Lasse Reichstein, 2010-04-07 20:38:43]

If I understand this correctly:
- the first time we execute a regexp on an input, we set saveAnswer to false
(cache miss)
- the second time we execute a regexp on the same input (first cache hit), we
set saveAnswer to true and saves and clones the result.
- any subsequent time we execute the regexp on the same input (further cache
hits), we return a clone of the result.

This avoids a cloning of the result of regexps that are only executed once.

If this is correct, LGTM.

[Erik Corry, 2010-04-08 08:47:18]

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

http://codereview.chromium.org/1556019/diff/1/2#newcode181
src/regexp.js:181: s = ToString(string);
Here we can execute arbitrary user code so we can reenter, but this time
saveAnswer has been set to true without a valid answer in the cache :-(.

[Lasse Reichstein, 2010-04-08 09:31:38]

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

http://codereview.chromium.org/1556019/diff/1/2#newcode181
src/regexp.js:181: s = ToString(string);
Very good catch.
A solution would be to change cache.saveAnswer to cache.answerSaved, and make
saveAnswer a local variable inside this function that determines what is stored
at the end.

[sandholm, 2010-04-08 10:24:09]

On 2010/04/08 09:31:38, Lasse Reichstein wrote:
> http://codereview.chromium.org/1556019/diff/1/2
> File src/regexp.js (right):
> 
> http://codereview.chromium.org/1556019/diff/1/2#newcode181
> src/regexp.js:181: s = ToString(string);
> Very good catch.
> A solution would be to change cache.saveAnswer to cache.answerSaved, and make
> saveAnswer a local variable inside this function that determines what is
stored
> at the end.

My words exactly. 
I will do just that.

[sandholm, 2010-04-08 11:19:02]

Uploaded a patch set. 
Please have another look.

[Erik Corry, 2010-04-08 13:35:43]

http://codereview.chromium.org/1556019/diff/7001/8001
File src/regexp.js (right):

http://codereview.chromium.org/1556019/diff/7001/8001#newcode130
src/regexp.js:130: // hit in RegExpExec, StringMatch and StringSplit
Add full stop to comment.

http://codereview.chromium.org/1556019/diff/7001/8001#newcode240
src/regexp.js:240: cache.answerSaved = saveAnswer;
Is this safe?

[sandholm, 2010-04-08 14:26:02]

http://codereview.chromium.org/1556019/diff/7001/8001
File src/regexp.js (right):

http://codereview.chromium.org/1556019/diff/7001/8001#newcode130
src/regexp.js:130: // hit in RegExpExec, StringMatch and StringSplit
On 2010/04/08 13:35:43, Erik Corry wrote:
> Add full stop to comment.

Done.

http://codereview.chromium.org/1556019/diff/7001/8001#newcode240
src/regexp.js:240: cache.answerSaved = saveAnswer;
On 2010/04/08 13:35:43, Erik Corry wrote:
> Is this safe?

Could be. With the latest patch it is for sure.

[Erik Corry, 2010-04-08 14:31:23]

LGTM!

[Lasse Reichstein, 2010-04-08 14:50:52]

Too late-comments :(

http://codereview.chromium.org/1556019/diff/14001/15001
File src/regexp.js (right):

http://codereview.chromium.org/1556019/diff/14001/15001#newcode170
src/regexp.js:170: cache.answerSaved = false;
No need to set answerSaved here. Set it, unconditionally, to saveAnswer when
updating the cache values below.

Due to the call to ToString below, we can't trust the value set here to stick
anyway.

http://codereview.chromium.org/1556019/diff/14001/15001#newcode241
src/regexp.js:241: cache.answerSaved = true;
I.e., always set answerSaved to saveAnswer here.