Reviewers: rossberg,
Description:
Object.observe: change array truncation logic to efficiently handle large
sparse
arrays
Please review this at https://codereview.chromium.org/12041084/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files:
M src/objects.cc
M test/mjsunit/harmony/object-observe.js
Index: src/objects.cc
diff --git a/src/objects.cc b/src/objects.cc
index
0825b64c3367e5d2d92bcd5a8374b3ec616eef6e..e5a0fea562b7187d2e8989bb1b5d6b687a836c3b
100644
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -9370,19 +9370,31 @@ MaybeObject* JSArray::SetElementsLength(Object*
len) {
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) {
+ Handle<FixedArray> keys =
isolate->factory()->NewFixedArray(num_elements);
+ self->GetLocalElementKeys(*keys, kNoAttrFilter);
+ while (num_elements-- > 0) {
+ Handle<Object> key(keys->get(num_elements));
+ uint32_t index = 0;
+ CHECK(key->ToArrayIndex(&index));
+ if (index < new_length) break;
+ PropertyAttributes attributes =
self->GetLocalElementAttribute(index);
+ ASSERT(attributes != ABSENT);
+ // A non-configurable property will cause the truncation operation to
+ // stop at this index.
+ if (attributes == DONT_DELETE) break;
+ old_values.Add(
+ self->GetLocalElementAccessorPair(index) == NULL
+ ? Object::GetElement(self, index)
+ : Handle<Object>::cast(isolate->factory()->the_hole_value()));
+ indices.Add(isolate->factory()->NumberToString(key));
+ }
}
MaybeObject* result =
Index: test/mjsunit/harmony/object-observe.js
diff --git a/test/mjsunit/harmony/object-observe.js
b/test/mjsunit/harmony/object-observe.js
index
4836eaa453d9190af081c1cf55a0e17e71d433f8..cef00e6a192256322ce6879ad52aaecd8d98f14f
100644
--- a/test/mjsunit/harmony/object-observe.js
+++ b/test/mjsunit/harmony/object-observe.js
@@ -626,16 +626,15 @@ var arr2 = ['alpha', 'beta'];
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++;
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 @@ observer.assertCallbackRecords([
{ 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
