Object.observe: use JSWeakMaps instead of raw ObjectHashTables in observation state

object-observe.js uses weak maps to add "hidden" properties to
objects. Previously, the hash tables it was using weren't actually
weak. This patch changes the existing runtime functions to create
instances of JSWeakMap instead of exposing ObjectHashTable directly.

Review URL: https://codereview.chromium.org/12092079

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@13591 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/object-observe.js

@@ -29,33 +29,38 @@
 
 var observationState = %GetObservationState();
 if (IS_UNDEFINED(observationState.observerInfoMap)) {
-  observationState.observerInfoMap = %CreateObjectHashTable();
-  observationState.objectInfoMap = %CreateObjectHashTable();
-  observationState.notifierTargetMap = %CreateObjectHashTable();
+  observationState.observerInfoMap = %ObservationWeakMapCreate();
+  observationState.objectInfoMap = %ObservationWeakMapCreate();
+  observationState.notifierTargetMap = %ObservationWeakMapCreate();
   observationState.pendingObservers = new InternalArray;
   observationState.observerPriority = 0;
 }
 
-function InternalObjectHashTable(tableName) {
-  this.tableName = tableName;
+function ObservationWeakMap(map) {
+  this.map_ = map;
 }
 
-InternalObjectHashTable.prototype = {
+ObservationWeakMap.prototype = {
   get: function(key) {
-    return %ObjectHashTableGet(observationState[this.tableName], key);
+    key = %UnwrapGlobalProxy(key);
+    if (!IS_SPEC_OBJECT(key)) return void 0;
+    return %WeakMapGet(this.map_, key);
   },
   set: function(key, value) {
-    observationState[this.tableName] =
-        %ObjectHashTableSet(observationState[this.tableName], key, value);
+    key = %UnwrapGlobalProxy(key);
+    if (!IS_SPEC_OBJECT(key)) return void 0;
+    %WeakMapSet(this.map_, key, value);
   },
   has: function(key) {
     return !IS_UNDEFINED(this.get(key));
   }
 };
 
-var observerInfoMap = new InternalObjectHashTable('observerInfoMap');
-var objectInfoMap = new InternalObjectHashTable('objectInfoMap');
-var notifierTargetMap = new InternalObjectHashTable('notifierTargetMap');
+var observerInfoMap =
+    new ObservationWeakMap(observationState.observerInfoMap);
+var objectInfoMap = new ObservationWeakMap(observationState.objectInfoMap);
+var notifierTargetMap =
+    new ObservationWeakMap(observationState.notifierTargetMap);
 
 function CreateObjectInfo(object) {
   var info = {

src/runtime.cc

@@ -863,10 +863,8 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_MapGetSize) {
 }
 
 
-RUNTIME_FUNCTION(MaybeObject*, Runtime_WeakMapInitialize) {
-  HandleScope scope(isolate);
-  ASSERT(args.length() == 1);
-  CONVERT_ARG_HANDLE_CHECKED(JSWeakMap, weakmap, 0);
+static JSWeakMap* WeakMapInitialize(Isolate* isolate,
+                                    Handle<JSWeakMap> weakmap) {
   ASSERT(weakmap->map()->inobject_properties() == 0);
   Handle<ObjectHashTable> table = isolate->factory()->NewObjectHashTable(0);
   weakmap->set_table(*table);
@@ -875,6 +873,14 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_WeakMapInitialize) {
 }
 
 
+RUNTIME_FUNCTION(MaybeObject*, Runtime_WeakMapInitialize) {
+  HandleScope scope(isolate);
+  ASSERT(args.length() == 1);
+  CONVERT_ARG_HANDLE_CHECKED(JSWeakMap, weakmap, 0);
+  return WeakMapInitialize(isolate, weakmap);
+}
+
+
 RUNTIME_FUNCTION(MaybeObject*, Runtime_WeakMapGet) {
   HandleScope scope(isolate);
   ASSERT(args.length() == 2);
@@ -13414,37 +13420,28 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_GetObservationState) {
 }
 
 
-RUNTIME_FUNCTION(MaybeObject*, Runtime_CreateObjectHashTable) {
+RUNTIME_FUNCTION(MaybeObject*, Runtime_ObservationWeakMapCreate) {
+  HandleScope scope(isolate);
   ASSERT(args.length() == 0);
-  return ObjectHashTable::Allocate(0);
+  // TODO(adamk): Currently this runtime function is only called three times per
+  // isolate. If it's called more often, the map should be moved into the
+  // strong root list.
+  Handle<Map> map =
+      isolate->factory()->NewMap(JS_WEAK_MAP_TYPE, JSWeakMap::kSize);
+  Handle<JSWeakMap> weakmap =
+      Handle<JSWeakMap>::cast(isolate->factory()->NewJSObjectFromMap(map));
+  return WeakMapInitialize(isolate, weakmap);
 }
 
 
-RUNTIME_FUNCTION(MaybeObject*, Runtime_ObjectHashTableGet) {
-  NoHandleAllocation ha;
-  ASSERT(args.length() == 2);
-  CONVERT_ARG_CHECKED(ObjectHashTable, table, 0);
-  Object* key = args[1];
-  if (key->IsJSGlobalProxy()) {
-    key = key->GetPrototype();
-    if (key->IsNull()) return isolate->heap()->undefined_value();
-  }
-  Object* lookup = table->Lookup(key);
-  return lookup->IsTheHole() ? isolate->heap()->undefined_value() : lookup;
-}
-
-
-RUNTIME_FUNCTION(MaybeObject*, Runtime_ObjectHashTableSet) {
-  HandleScope scope(isolate);
-  ASSERT(args.length() == 3);
-  CONVERT_ARG_HANDLE_CHECKED(ObjectHashTable, table, 0);
-  Handle<Object> key = args.at<Object>(1);
-  if (key->IsJSGlobalProxy()) {
-    key = handle(key->GetPrototype(), isolate);
-    if (key->IsNull()) return *table;
+RUNTIME_FUNCTION(MaybeObject*, Runtime_UnwrapGlobalProxy) {
+  ASSERT(args.length() == 1);
+  Object* object = args[0];
+  if (object->IsJSGlobalProxy()) {
+    object = object->GetPrototype();
+    if (object->IsNull()) return isolate->heap()->undefined_value();
   }
-  Handle<Object> value = args.at<Object>(2);
-  return *PutIntoObjectHashTable(table, key, value);
+  return object;
 }
 
 

src/runtime.h

@@ -333,9 +333,8 @@ namespace internal {
   F(SetIsObserved, 2, 1) \
   F(SetObserverDeliveryPending, 0, 1) \
   F(GetObservationState, 0, 1) \
-  F(CreateObjectHashTable, 0, 1) \
-  F(ObjectHashTableGet, 2, 1) \
-  F(ObjectHashTableSet, 3, 1) \
+  F(ObservationWeakMapCreate, 0, 1) \
+  F(UnwrapGlobalProxy, 1, 1) \
   \
   /* Statements */ \
   F(NewClosure, 3, 1) \

test/cctest/test-object-observe.cc

@@ -30,6 +30,7 @@
 #include "cctest.h"
 
 using namespace v8;
+namespace i = v8::internal;
 
 namespace {
 // Need to create a new isolate when FLAG_harmony_observation is on.
@@ -397,3 +398,37 @@ TEST(HiddenPrototypeObservation) {
   };
   EXPECT_RECORDS(CompileRun("records"), expected_records3);
 }
+
+
+static int NumberOfElements(i::Handle<i::JSWeakMap> map) {
+  return i::ObjectHashTable::cast(map->table())->NumberOfElements();
+}
+
+
+TEST(ObservationWeakMap) {
+  HarmonyIsolate isolate;
+  HandleScope scope;
+  LocalContext context;
+  CompileRun(
+      "var obj = {};"
+      "Object.observe(obj, function(){});"
+      "Object.getNotifier(obj);"
+      "obj = null;");
+  i::Handle<i::JSObject> observation_state = FACTORY->observation_state();
+  i::Handle<i::JSWeakMap> observerInfoMap =
+      i::Handle<i::JSWeakMap>::cast(
+          i::GetProperty(observation_state, "observerInfoMap"));
+  i::Handle<i::JSWeakMap> objectInfoMap =
+      i::Handle<i::JSWeakMap>::cast(
+          i::GetProperty(observation_state, "objectInfoMap"));
+  i::Handle<i::JSWeakMap> notifierTargetMap =
+      i::Handle<i::JSWeakMap>::cast(
+          i::GetProperty(observation_state, "notifierTargetMap"));
+  CHECK_EQ(1, NumberOfElements(observerInfoMap));
+  CHECK_EQ(1, NumberOfElements(objectInfoMap));
+  CHECK_EQ(1, NumberOfElements(notifierTargetMap));
+  HEAP->CollectAllGarbage(i::Heap::kNoGCFlags);
+  CHECK_EQ(0, NumberOfElements(observerInfoMap));
+  CHECK_EQ(0, NumberOfElements(objectInfoMap));
+  CHECK_EQ(0, NumberOfElements(notifierTargetMap));
+}