Revision: 13591
Author:   [email protected]
Date:     Mon Feb  4 12:24:11 2013
Log: 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
http://code.google.com/p/v8/source/detail?r=13591

Modified:
 /branches/bleeding_edge/src/object-observe.js
 /branches/bleeding_edge/src/runtime.cc
 /branches/bleeding_edge/src/runtime.h
 /branches/bleeding_edge/test/cctest/test-object-observe.cc

=======================================
--- /branches/bleeding_edge/src/object-observe.js       Wed Dec 19 02:28:36 2012
+++ /branches/bleeding_edge/src/object-observe.js       Mon Feb  4 12:24:11 2013
@@ -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 = {
=======================================
--- /branches/bleeding_edge/src/runtime.cc      Mon Feb  4 04:01:59 2013
+++ /branches/bleeding_edge/src/runtime.cc      Mon Feb  4 12:24:11 2013
@@ -863,16 +863,22 @@
 }


-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);
   weakmap->set_next(Smi::FromInt(0));
   return *weakmap;
 }
+
+
+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) {
@@ -13414,37 +13420,28 @@
 }


-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();
+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();
   }
-  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;
-  }
-  Handle<Object> value = args.at<Object>(2);
-  return *PutIntoObjectHashTable(table, key, value);
+  return object;
 }


=======================================
--- /branches/bleeding_edge/src/runtime.h       Mon Feb  4 04:01:59 2013
+++ /branches/bleeding_edge/src/runtime.h       Mon Feb  4 12:24:11 2013
@@ -333,9 +333,8 @@
   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) \
=======================================
--- /branches/bleeding_edge/test/cctest/test-object-observe.cc Fri Dec 21 09:40:09 2012 +++ /branches/bleeding_edge/test/cctest/test-object-observe.cc Mon Feb 4 12:24:11 2013
@@ -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 @@
   };
   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));
+}

--
--
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.


Reply via email to