Erik, I'd like you to do a code review. To review this change, run
gvn review --project https://v8.googlecode.com/svn [EMAIL PROTECTED]/[EMAIL PROTECTED] Alternatively, to review the latest snapshot of this change branch, run gvn --project https://v8.googlecode.com/svn review [EMAIL PROTECTED]/faster-contains-check to review the following change: [EMAIL PROTECTED]/[EMAIL PROTECTED] | [EMAIL PROTECTED] | 2008-09-09 08:06:57 +-100 (Tue, 09 Sep 2008) Description: Move the contains check in array join from javascript to C++. Affected Paths: M //branches/bleeding_edge/src/array.js M //branches/bleeding_edge/src/runtime.cc M //branches/bleeding_edge/src/runtime.h This is a semiautomated message from "gvn mail". See <http://code.google.com/p/gvn/> to learn more. Index: src/array.js =================================================================== --- src/array.js (^/branches/bleeding_edge/src/[EMAIL PROTECTED]) +++ src/array.js (^/changes/[EMAIL PROTECTED]/faster-contains-check/bleeding_edge/src/[EMAIL PROTECTED]) @@ -111,8 +111,7 @@ function Join(array, length, separator, convert) { if (is_array) { // If the array is cyclic, return the empty string for already // visited arrays. - if (Contains(visited_arrays, array)) return ''; - visited_arrays[visited_arrays.length] = array; + if (!%PushIfAbsent(visited_arrays, array)) return ''; } // Attempt to convert the elements. @@ -702,17 +701,17 @@ function ArraySort(comparefn) { while (true) { var child_index = ((parent_index + 1) << 1) - 1; if (child_index >= i) break; - var child1_value = this[child_index]; + var child1_value = this[child_index]; var child2_value = this[child_index + 1]; var parent_value = this[parent_index]; if (child_index + 1 >= i || Compare(child1_value, child2_value) > 0) { if (Compare(parent_value, child1_value) > 0) break; - this[child_index] = parent_value; + this[child_index] = parent_value; this[parent_index] = child1_value; parent_index = child_index; } else { if (Compare(parent_value, child2_value) > 0) break; - this[child_index + 1] = parent_value; + this[child_index + 1] = parent_value; this[parent_index] = child2_value; parent_index = child_index + 1; } Index: src/runtime.cc =================================================================== --- src/runtime.cc (^/branches/bleeding_edge/src/[EMAIL PROTECTED]) +++ src/runtime.cc (^/changes/[EMAIL PROTECTED]/faster-contains-check/bleeding_edge/src/[EMAIL PROTECTED]) @@ -3443,6 +3443,25 @@ static Object* Runtime_SetNewFunctionAttributes(Ar } +// Push an array unto an array of arrays if it is not already in the +// array. Returns true if the element was pushed on the stack and +// false otherwise. +static Object* Runtime_PushIfAbsent(Arguments args) { + ASSERT(args.length == 2); + CONVERT_CHECKED(JSArray, array, args[0]); + CONVERT_CHECKED(JSArray, element, args[1]); + ASSERT(array->HasFastElements()); + int length = Smi::cast(array->length())->value(); + FixedArray* elements = FixedArray::cast(array->elements()); + for (int i = 0; i < length; i++) { + if (elements->get(i) == element) return Heap::false_value(); + } + Object* obj = array->SetFastElement(length, element); + if (obj->IsFailure()) return obj; + return Heap::true_value(); +} + + // This will not allocate (flatten the string), but it may run // very slowly for very deeply nested ConsStrings. For debugging use only. static Object* Runtime_GlobalPrint(Arguments args) { Index: src/runtime.h =================================================================== --- src/runtime.h (^/branches/bleeding_edge/src/[EMAIL PROTECTED]) +++ src/runtime.h (^/changes/[EMAIL PROTECTED]/faster-contains-check/bleeding_edge/src/[EMAIL PROTECTED]) @@ -61,6 +61,9 @@ namespace v8 { namespace internal { F(LazyCompile, 1) \ F(SetNewFunctionAttributes, 1) \ \ + /* Array join support */ \ + F(PushIfAbsent, 2) \ + \ /* ConsStrings */ \ F(ConsStringFst, 1) \ F(ConsStringSnd, 1) \ --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
