go/lem: http://compute3.aar:9013/golem/r4129-v8-antonm-array-concat-generic-bis-vs-4129-v8.html
So if anything, it looks slightly better than previous variant. yours, anton. On Tue, Mar 16, 2010 at 7:20 AM, <[email protected]> wrote: > Reviewers: Lasse Reichstein, Mads Ager, > > Message: > Lasse, Mads, > > here comes promised more generic version of Array.concat. > > Lasse, this patch should obsolete http://codereview.chromium.org/971001 if > submitted first > > Description: > More generic version of Array.concat builtin. > > Please review this at http://codereview.chromium.org/1036002 > > Affected files: > M src/builtins.cc > M src/v8-counters.h > > > Index: src/builtins.cc > diff --git a/src/builtins.cc b/src/builtins.cc > index > 4166b7121d269c5d38091cccf1b753425f306b00..38f23bdc55798eb14dca47252c61043d054b4e6f > 100644 > --- a/src/builtins.cc > +++ b/src/builtins.cc > @@ -728,36 +728,29 @@ BUILTIN(ArraySplice) { > > > BUILTIN(ArrayConcat) { > - Counters::array_concat_builtin_total.Increment(); > - if (args.length() != 2) { > - // Fast case only for concating two arrays. > - return CallJsBuiltin("ArrayConcat", args); > - } > - Counters::array_concat_builtin_two_args.Increment(); > - > - Object* receiver_obj = *args.receiver(); > - FixedArray* receiver_elms = NULL; > - Object* arg_obj = args[1]; > - FixedArray* arg_elms = NULL; > - if (!IsJSArrayWithFastElements(receiver_obj, &receiver_elms) > - || !IsJSArrayWithFastElements(arg_obj, &arg_elms) > - || !ArrayPrototypeHasNoElements()) { > + if (!ArrayPrototypeHasNoElements()) { > return CallJsBuiltin("ArrayConcat", args); > } > > - JSArray* receiver_array = JSArray::cast(receiver_obj); > - ASSERT(receiver_array->HasFastElements()); > - JSArray* arg_array = JSArray::cast(arg_obj); > - ASSERT(arg_array->HasFastElements()); > - > - int receiver_len = Smi::cast(receiver_array->length())->value(); > - int arg_len = Smi::cast(arg_array->length())->value(); > - ASSERT(receiver_len <= (Smi::kMaxValue - arg_len)); > + // Iterate through all the arguments performing checks > + // and calculating total length. > + int n_arguments = args.length(); > + int result_len = 0; > + for (int i = 0; i < n_arguments; i++) { > + Object* arg = args[i]; > + FixedArray* elms = NULL; > + if (!IsJSArrayWithFastElements(arg, &elms)) { > + return CallJsBuiltin("ArrayConcat", args); > + } > + ASSERT(JSArray::cast(arg)->HasFastElements()); > > - int result_len = receiver_len + arg_len; > - if (result_len > FixedArray::kMaxSize) { > - return CallJsBuiltin("ArrayConcat", args); > + int len = Smi::cast(JSArray::cast(arg)->length())->value(); > + result_len += len; > + if (result_len > FixedArray::kMaxLength) { > + return CallJsBuiltin("ArrayConcat", args); > + } > } > + > if (result_len == 0) { > return AllocateEmptyJSArray(); > } > @@ -773,8 +766,15 @@ BUILTIN(ArrayConcat) { > > // Copy data. > AssertNoAllocation no_gc; > - CopyElements(&no_gc, result_elms, 0, receiver_elms, 0, receiver_len); > - CopyElements(&no_gc, result_elms, receiver_len, arg_elms, 0, arg_len); > + int start_pos = 0; > + for (int i = 0; i < n_arguments; i++) { > + JSArray* array = JSArray::cast(args[i]); > + FixedArray* elms = FixedArray::cast(array->elements()); > + int len = Smi::cast(array->length())->value(); > + CopyElements(&no_gc, result_elms, start_pos, elms, 0, len); > + start_pos += len; > + } > + ASSERT(start_pos == result_len); > > // Set the length and elements. > result_array->set_length(Smi::FromInt(result_len)); > Index: src/v8-counters.h > diff --git a/src/v8-counters.h b/src/v8-counters.h > index > c1400ae4047f1b091c32d549a03499581cc740eb..b595cd4922c2739b6b537c68a33507891d97f955 > 100644 > --- a/src/v8-counters.h > +++ b/src/v8-counters.h > @@ -151,8 +151,6 @@ namespace internal { > SC(constructed_objects_stub, V8.ConstructedObjectsStub) \ > SC(array_function_runtime, V8.ArrayFunctionRuntime) \ > SC(array_function_native, V8.ArrayFunctionNative) \ > - SC(array_concat_builtin_total, V8.ArrayConcatTotal) \ > - SC(array_concat_builtin_two_args, V8.ArrayConcatTwoArgs) \ > SC(for_in, V8.ForIn) \ > SC(enum_cache_hits, V8.EnumCacheHits) \ > SC(enum_cache_misses, V8.EnumCacheMisses) \ > > > -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
