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.
http://codereview.chromium.org/10837290/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
