Cache results in SearchRegExpMultiple.

Cache results in SearchRegExpMultiple.


BUG=

Committed: https://code.google.com/p/v8/source/detail?r=12416

[Yang, 2012-08-16 18:19:43]

PTAL.

[ulan, 2012-08-28 08:44:03]

Few comments:

http://codereview.chromium.org/10837290/diff/6001/src/heap.h
File src/heap.h (right):

http://codereview.chromium.org/10837290/diff/6001/src/heap.h#newcode2575
src/heap.h:2575: class RegExpResultsCache {
Can we introduce a parameter for this class or its functions that indicates
whether we are using this cache for string split or regex replace? This would
allow more precise checks and help with documentation.

http://codereview.chromium.org/10837290/diff/6001/src/heap.h#newcode2578
src/heap.h:2578: static void Enter(Heap* heap,
Can we add high level comments for these functions describing pre/post
conditions? Maybe also choose more descriptive name than "array"?

http://codereview.chromium.org/10837290/diff/6001/src/runtime.cc
File src/runtime.cc (right):

http://codereview.chromium.org/10837290/diff/6001/src/runtime.cc#newcode2422
src/runtime.cc:2422: static int GetFinalLength(Handle<FixedArray> array) {
The function name is a bit obscure. Maybe just inline the expression at the
call-site?

http://codereview.chromium.org/10837290/diff/6001/src/runtime.cc#newcode3688
src/runtime.cc:3688: isolate->factory()->SetContent(result_array,
cached_fixed_array);
A comment that the cached_fixed_array is COW would be helpful here.

[Yang, 2012-08-28 13:16:23]

On 2012/08/28 08:44:03, ulan wrote:
> Few comments:
> 
> http://codereview.chromium.org/10837290/diff/6001/src/heap.h
> File src/heap.h (right):
> 
> http://codereview.chromium.org/10837290/diff/6001/src/heap.h#newcode2575
> src/heap.h:2575: class RegExpResultsCache {
> Can we introduce a parameter for this class or its functions that indicates
> whether we are using this cache for string split or regex replace? This would
> allow more precise checks and help with documentation.
> 
> http://codereview.chromium.org/10837290/diff/6001/src/heap.h#newcode2578
> src/heap.h:2578: static void Enter(Heap* heap,
> Can we add high level comments for these functions describing pre/post
> conditions? Maybe also choose more descriptive name than "array"?
> 
> http://codereview.chromium.org/10837290/diff/6001/src/runtime.cc
> File src/runtime.cc (right):
> 
> http://codereview.chromium.org/10837290/diff/6001/src/runtime.cc#newcode2422
> src/runtime.cc:2422: static int GetFinalLength(Handle<FixedArray> array) {
> The function name is a bit obscure. Maybe just inline the expression at the
> call-site?
> 
> http://codereview.chromium.org/10837290/diff/6001/src/runtime.cc#newcode3688
> src/runtime.cc:3688: isolate->factory()->SetContent(result_array,
> cached_fixed_array);
> A comment that the cached_fixed_array is COW would be helpful here.

Addressed comments. Please take another look.

[ulan, 2012-08-31 08:52:51]

LGTM

http://codereview.chromium.org/10837290/diff/10002/src/runtime.cc
File src/runtime.cc (right):

http://codereview.chromium.org/10837290/diff/10002/src/runtime.cc#newcode2422
src/runtime.cc:2422: static int GetFinalLength(Handle<FixedArray> array) {
It can be removed now.