Revision: 13592
Author: [email protected]
Date: Mon Feb 4 13:03:08 2013
Log: Object.observe: change array truncation logic to efficiently
handle large sparse arrays
Review URL: https://codereview.chromium.org/12041084
http://code.google.com/p/v8/source/detail?r=13592
Modified:
/branches/bleeding_edge/src/objects.cc
/branches/bleeding_edge/test/mjsunit/harmony/object-observe.js
=======================================
--- /branches/bleeding_edge/src/objects.cc Mon Feb 4 02:56:50 2013
+++ /branches/bleeding_edge/src/objects.cc Mon Feb 4 13:03:08 2013
@@ -9342,6 +9342,25 @@
GetIsolate()->factory()->SetElementsCapacityAndLength(
Handle<JSArray>(this), required_size, required_size);
}
+
+
+// Returns false if the passed-in index is marked non-configurable,
+// which will cause the ES5 truncation operation to halt, and thus
+// no further old values need be collected.
+static bool GetOldValue(Isolate* isolate,
+ Handle<JSObject> object,
+ uint32_t index,
+ List<Handle<Object> >* old_values,
+ List<Handle<String> >* indices) {
+ PropertyAttributes attributes = object->GetLocalElementAttribute(index);
+ ASSERT(attributes != ABSENT);
+ if (attributes == DONT_DELETE) return false;
+ old_values->Add(object->GetLocalElementAccessorPair(index) == NULL
+ ? Object::GetElement(object, index)
+ : Handle<Object>::cast(isolate->factory()->the_hole_value()));
+ indices->Add(isolate->factory()->Uint32ToString(index));
+ return true;
+}
MaybeObject* JSArray::SetElementsLength(Object* len) {
@@ -9363,19 +9382,28 @@
if (!new_length_handle->ToArrayIndex(&new_length))
return Failure::InternalError();
- // TODO(adamk): This loop can be very slow for arrays in dictionary mode.
- // Find another way to iterate over arrays with dictionary elements.
- for (uint32_t i = old_length - 1; i + 1 > new_length; --i) {
- PropertyAttributes attributes = self->GetLocalElementAttribute(i);
- if (attributes == ABSENT) continue;
- // A non-configurable property will cause the truncation operation to
- // stop at this index.
- if (attributes == DONT_DELETE) break;
- old_values.Add(
- self->GetLocalElementAccessorPair(i) == NULL
- ? Object::GetElement(self, i)
- : Handle<Object>::cast(isolate->factory()->the_hole_value()));
- indices.Add(isolate->factory()->Uint32ToString(i));
+ // Observed arrays should always be in dictionary mode;
+ // if they were in fast mode, the below is slower than necessary
+ // as it iterates over the array backing store multiple times.
+ ASSERT(self->HasDictionaryElements());
+ static const PropertyAttributes kNoAttrFilter = NONE;
+ int num_elements = self->NumberOfLocalElements(kNoAttrFilter);
+ if (num_elements > 0) {
+ if (old_length == static_cast<uint32_t>(num_elements)) {
+ // Simple case for arrays without holes.
+ for (uint32_t i = old_length - 1; i + 1 > new_length; --i) {
+ if (!GetOldValue(isolate, self, i, &old_values, &indices)) break;
+ }
+ } else {
+ // For sparse arrays, only iterate over existing elements.
+ Handle<FixedArray> keys =
isolate->factory()->NewFixedArray(num_elements);
+ self->GetLocalElementKeys(*keys, kNoAttrFilter);
+ while (num_elements-- > 0) {
+ uint32_t index = NumberToUint32(keys->get(num_elements));
+ if (index < new_length) break;
+ if (!GetOldValue(isolate, self, index, &old_values, &indices))
break;
+ }
+ }
}
MaybeObject* result =
=======================================
--- /branches/bleeding_edge/test/mjsunit/harmony/object-observe.js Wed Jan
30 13:07:28 2013
+++ /branches/bleeding_edge/test/mjsunit/harmony/object-observe.js Mon Feb
4 13:03:08 2013
@@ -626,16 +626,15 @@
var arr3 = ['hello'];
arr3[2] = 'goodbye';
arr3.length = 6;
-// TODO(adamk): Enable this test case when it can run in a reasonable
-// amount of time.
-//var slow_arr = new Array(1000000000);
-//slow_arr[500000000] = 'hello';
+var slow_arr = new Array(1000000000);
+slow_arr[500000000] = 'hello';
Object.defineProperty(arr, '0', {configurable: false});
Object.defineProperty(arr, '2', {get: function(){}});
Object.defineProperty(arr2, '0', {get: function(){}, configurable: false});
Object.observe(arr, observer.callback);
Object.observe(arr2, observer.callback);
Object.observe(arr3, observer.callback);
+Object.observe(slow_arr, observer.callback);
arr.length = 2;
arr.length = 0;
arr.length = 10;
@@ -649,6 +648,7 @@
arr3.length /= 2;
Object.defineProperty(arr3, 'length', {value: 5});
Object.defineProperty(arr3, 'length', {value: 10, writable: false});
+slow_arr.length = 100;
Object.deliverChangeRecords(observer.callback);
observer.assertCallbackRecords([
{ object: arr, name: '3', type: 'deleted', oldValue: 'd' },
@@ -669,6 +669,8 @@
{ object: arr3, name: 'length', type: 'updated', oldValue: 2 },
{ object: arr3, name: 'length', type: 'updated', oldValue: 1 },
{ object: arr3, name: 'length', type: 'reconfigured', oldValue: 5 },
+ { object: slow_arr, name: '500000000', type: 'deleted',
oldValue: 'hello' },
+ { object: slow_arr, name: 'length', type: 'updated', oldValue:
1000000000 },
]);
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
