Revision: 13205
Author: [email protected]
Date: Wed Dec 12 03:38:24 2012
Log: Object.observe: prevent observed objects from using fast elements.
This is necessary because polymorphic stores generally
do not perform a map check but only an instance type check,
which misses out on changes in the observation status.
Unfortunately, there currently is no efficient way in V8
to maintain that optimisation in the presence of Object.observe.
[email protected]
BUG=v8:2409
Review URL: https://codereview.chromium.org/11477006
http://code.google.com/p/v8/source/detail?r=13205
Modified:
/branches/bleeding_edge/src/objects-inl.h
/branches/bleeding_edge/src/objects.cc
/branches/bleeding_edge/src/runtime.cc
/branches/bleeding_edge/test/mjsunit/harmony/object-observe.js
/branches/bleeding_edge/test/mjsunit/regress/regress-observe-empty-double-array.js
=======================================
--- /branches/bleeding_edge/src/objects-inl.h Tue Dec 11 09:28:40 2012
+++ /branches/bleeding_edge/src/objects-inl.h Wed Dec 12 03:38:24 2012
@@ -1426,6 +1426,13 @@
if (!maybe_obj->ToObject(&obj)) return maybe_obj;
set_map(Map::cast(obj));
initialize_elements();
+ if (FLAG_harmony_observation && map()->is_observed()) {
+ // Maintain invariant that observed elements are always in dictionary
mode.
+ // For this to work on arrays, we have to make sure to reset length
first.
+ if (IsJSArray()) JSArray::cast(this)->set_length(Smi::FromInt(0));
+ maybe_obj = NormalizeElements();
+ if (maybe_obj->IsFailure()) return maybe_obj;
+ }
return this;
}
=======================================
--- /branches/bleeding_edge/src/objects.cc Tue Dec 11 09:28:40 2012
+++ /branches/bleeding_edge/src/objects.cc Wed Dec 12 03:38:24 2012
@@ -9424,6 +9424,7 @@
Heap* heap = GetHeap();
// We should never end in here with a pixel or external array.
ASSERT(!HasExternalArrayElements());
+ ASSERT(!map()->is_observed());
// Allocate a new fast elements backing store.
FixedArray* new_elements;
@@ -9488,6 +9489,7 @@
Heap* heap = GetHeap();
// We should never end in here with a pixel or external array.
ASSERT(!HasExternalArrayElements());
+ ASSERT(!map()->is_observed());
FixedArrayBase* elems;
{ MaybeObject* maybe_obj =
@@ -10631,6 +10633,7 @@
MaybeObject* JSObject::TransitionElementsKind(ElementsKind to_kind) {
+ ASSERT(!map()->is_observed());
ElementsKind from_kind = map()->elements_kind();
if (IsFastHoleyElementsKind(from_kind)) {
@@ -10887,6 +10890,9 @@
// An object requiring access checks is never allowed to have fast
// elements. If it had fast elements we would skip security checks.
if (IsAccessCheckNeeded()) return false;
+ // Observed objects may not go to fast mode because they rely on map
checks,
+ // and for fast element accesses we sometimes check element kinds only.
+ if (FLAG_harmony_observation && map()->is_observed()) return false;
FixedArray* elements = FixedArray::cast(this->elements());
SeededNumberDictionary* dictionary = NULL;
=======================================
--- /branches/bleeding_edge/src/runtime.cc Tue Dec 11 15:27:38 2012
+++ /branches/bleeding_edge/src/runtime.cc Wed Dec 12 03:38:24 2012
@@ -13493,6 +13493,13 @@
if (!maybe->To(&map)) return maybe;
map->set_is_observed(is_observed);
obj->set_map(map);
+ if (is_observed && obj->IsJSObject() &&
+ !JSObject::cast(obj)->HasExternalArrayElements()) {
+ // Go to dictionary mode, so that we don't skip map checks.
+ maybe = JSObject::cast(obj)->NormalizeElements();
+ if (maybe->IsFailure()) return maybe;
+ ASSERT(!JSObject::cast(obj)->HasFastElements());
+ }
}
return isolate->heap()->undefined_value();
}
=======================================
--- /branches/bleeding_edge/test/mjsunit/harmony/object-observe.js Mon Dec
10 02:53:57 2012
+++ /branches/bleeding_edge/test/mjsunit/harmony/object-observe.js Wed Dec
12 03:38:24 2012
@@ -26,6 +26,7 @@
// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
// Flags: --harmony-observation --harmony-proxies --harmony-collections
+// Flags: --allow-natives-syntax
var allObservers = [];
function reset() {
@@ -871,3 +872,80 @@
obj.prototype = 7;
Object.deliverChangeRecords(observer.callback);
observer.assertNotCalled();
+
+
+// Check that changes in observation status are detected in all IC states
and
+// in optimized code, especially in cases usually using fast elements.
+function TestFastElements(prepopulate, polymorphic, optimize) {
+ var setElement = eval(
+ "(function setElement(a, i, v) { a[i] = v " +
+ "/* " + prepopulate + " " + polymorphic + " " + optimize + " */" +
+ "})"
+ );
+ print("TestFastElements:", setElement);
+
+ var arr = prepopulate ? [1, 2, 3, 4, 5] : [0];
+ setElement(arr, 1, 210);
+ setElement(arr, 1, 211);
+ if (polymorphic) setElement(["M", "i", "l", "n", "e", "r"], 0, "m");
+ if (optimize) %OptimizeFunctionOnNextCall(setElement);
+ setElement(arr, 1, 212);
+
+ reset();
+ Object.observe(arr, observer.callback);
+ setElement(arr, 1, 989898);
+ Object.deliverChangeRecords(observer.callback);
+ observer.assertCallbackRecords([
+ { object: arr, name: '1', type: 'updated', oldValue: 212 }
+ ]);
+}
+
+for (var b1 = 0; b1 < 2; ++b1)
+ for (var b2 = 0; b2 < 2; ++b2)
+ for (var b3 = 0; b3 < 2; ++b3)
+ TestFastElements(b1 != 0, b2 != 0, b3 != 0);
+
+
+function TestFastElementsLength(polymorphic, optimize, oldSize, newSize) {
+ var setLength = eval(
+ "(function setLength(a, n) { a.length = n " +
+ "/* " + polymorphic + " " + optimize + " " + oldSize + " " + newSize
+ " */"
+ + "})"
+ );
+ print("TestFastElementsLength:", setLength);
+
+ function array(n) {
+ var arr = new Array(n);
+ for (var i = 0; i < n; ++i) arr[i] = i;
+ return arr;
+ }
+
+ setLength(array(oldSize), newSize);
+ setLength(array(oldSize), newSize);
+ if (polymorphic) setLength(array(oldSize).map(isNaN), newSize);
+ if (optimize) %OptimizeFunctionOnNextCall(setLength);
+ setLength(array(oldSize), newSize);
+
+ reset();
+ var arr = array(oldSize);
+ Object.observe(arr, observer.callback);
+ setLength(arr, newSize);
+ Object.deliverChangeRecords(observer.callback);
+ if (oldSize === newSize) {
+ observer.assertNotCalled();
+ } else {
+ var count = oldSize > newSize ? oldSize - newSize : 0;
+ observer.assertRecordCount(count + 1);
+ var lengthRecord = observer.records[count];
+ assertSame(arr, lengthRecord.object);
+ assertEquals('length', lengthRecord.name);
+ assertEquals('updated', lengthRecord.type);
+ assertSame(oldSize, lengthRecord.oldValue);
+ }
+}
+
+for (var b1 = 0; b1 < 2; ++b1)
+ for (var b2 = 0; b2 < 2; ++b2)
+ for (var n1 = 0; n1 < 3; ++n1)
+ for (var n2 = 0; n2 < 3; ++n2)
+ TestFastElementsLength(b1 != 0, b2 != 0, 20*n1, 20*n2);
=======================================
---
/branches/bleeding_edge/test/mjsunit/regress/regress-observe-empty-double-array.js
Wed Nov 28 00:41:45 2012
+++
/branches/bleeding_edge/test/mjsunit/regress/regress-observe-empty-double-array.js
Wed Dec 12 03:38:24 2012
@@ -32,6 +32,7 @@
arr = [1.1];
Object.observe(arr, function(){});
arr.length = 0;
-assertTrue(%HasFastDoubleElements(arr));
+// TODO(observe): we currently disallow fast elements for observed object.
+// assertTrue(%HasFastDoubleElements(arr));
// Should not crash
arr.push(1.1);
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
