Revert of Add WeakKeyMap to v8.h (patchset #2 id:20001 of https://codereview.chromium...
authoryurys <yurys@chromium.org>
Wed, 4 Feb 2015 15:12:37 +0000 (07:12 -0800)
committerCommit bot <commit-bot@chromium.org>
Wed, 4 Feb 2015 15:12:52 +0000 (15:12 +0000)
Reason for revert:
Revert this patch due to shared win build compilation failure

http://build.chromium.org/p/client.v8/builders/V8%20Win32%20-%20nosnap%20-%20shared/builds/5030/steps/compile/logs/stdio

Original issue's description:
> Add WeakKeyMap to v8.h
>
> A new map wich references its keys weakly is added to v8.h. Internally it uses the same storage as JSWeakMap but doesn't depend on the JavaScript part of WeakMap implementation in weak-collection.js, hence it can be instantiated without entering any context.
>
> BUG=chromium:437416
> LOG=Y
>
> Committed: https://crrev.com/ee7ed39ac8327124e74dd7ad5f1de0dede988cb7
> Cr-Commit-Position: refs/heads/master@{#26425}

TBR=jochen@chromium.org,mstarzinger@chromium.org,rossberg@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=chromium:437416

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

Cr-Commit-Position: refs/heads/master@{#26430}

include/v8.h
src/api.cc
src/factory.cc
src/factory.h
src/runtime/runtime-collections.cc
src/runtime/runtime.h
test/cctest/test-api.cc
test/cctest/test-weakmaps.cc
test/cctest/test-weaksets.cc

index 8e62c93..eb7a8c3 100644 (file)
@@ -1536,32 +1536,6 @@ class V8_EXPORT JSON {
 };
 
 
-/**
- * A map whose keys are referenced weakly. It is similar to JavaScript WeakMap
- * but can be created without entering a v8::Context and hence shouldn't
- * escape to JavaScript.
- */
-class V8_EXPORT NativeWeakMap {
- public:
-  static NativeWeakMap* New(Isolate* isolate);
-  ~NativeWeakMap();
-  void Set(Handle<Value> key, Handle<Value> value);
-  Local<Value> Get(Handle<Value> key);
-  bool Has(Handle<Value> key);
-  bool Delete(Handle<Value> key);
-
- private:
-  NativeWeakMap(Isolate* isolate, Handle<Object> weak_map);
-
-  Isolate* isolate_;
-  UniquePersistent<Object> map_;
-
-  // Disallow copying and assigning.
-  NativeWeakMap(NativeWeakMap&);
-  void operator=(NativeWeakMap&);
-};
-
-
 // --- Value ---
 
 
index d6c0475..7c40994 100644 (file)
@@ -2250,119 +2250,6 @@ bool StackFrame::IsConstructor() const {
 }
 
 
-// --- N a t i v e W e a k M a p ---
-
-NativeWeakMap* NativeWeakMap::New(Isolate* v8_isolate) {
-  i::Isolate* isolate = reinterpret_cast<i::Isolate*>(v8_isolate);
-  ENTER_V8(isolate);
-  i::HandleScope scope(isolate);
-  i::Handle<i::JSWeakMap> weakmap = isolate->factory()->NewJSWeakMap();
-  i::Runtime::WeakCollectionInitialize(isolate, weakmap);
-  Local<Object> v8_obj = Utils::ToLocal(i::Handle<i::JSObject>::cast(weakmap));
-  return new NativeWeakMap(v8_isolate, v8_obj);
-}
-
-
-NativeWeakMap::NativeWeakMap(Isolate* isolate, Handle<Object> weak_map)
-    : isolate_(isolate), map_(isolate, weak_map) {}
-
-
-NativeWeakMap::~NativeWeakMap() {}
-
-
-void NativeWeakMap::Set(Handle<Value> v8_key, Handle<Value> v8_value) {
-  v8::HandleScope handleScope(isolate_);
-  Local<Object> map_handle = Local<Object>::New(isolate_, map_);
-  i::Isolate* isolate = reinterpret_cast<i::Isolate*>(isolate_);
-  ENTER_V8(isolate);
-  i::Handle<i::Object> key = Utils::OpenHandle(*v8_key);
-  i::Handle<i::Object> value = Utils::OpenHandle(*v8_value);
-  i::Handle<i::JSWeakMap> weak_collection =
-      i::Handle<i::JSWeakMap>::cast(Utils::OpenHandle(*map_handle));
-  if (!key->IsJSReceiver() && !key->IsSymbol()) {
-    DCHECK(false);
-    return;
-  }
-  i::Handle<i::ObjectHashTable> table(
-      i::ObjectHashTable::cast(weak_collection->table()));
-  if (!table->IsKey(*key)) {
-    DCHECK(false);
-    return;
-  }
-  i::Runtime::WeakCollectionSet(weak_collection, key, value);
-}
-
-
-Local<Value> NativeWeakMap::Get(Handle<Value> v8_key) {
-  v8::EscapableHandleScope handleScope(isolate_);
-  Local<Object> map_handle = Local<Object>::New(isolate_, map_);
-  i::Isolate* isolate = reinterpret_cast<i::Isolate*>(isolate_);
-  ENTER_V8(isolate);
-  i::Handle<i::Object> key = Utils::OpenHandle(*v8_key);
-  i::Handle<i::JSWeakMap> weak_collection =
-      i::Handle<i::JSWeakMap>::cast(Utils::OpenHandle(*map_handle));
-  if (!key->IsJSReceiver() && !key->IsSymbol()) {
-    DCHECK(false);
-    return Undefined(isolate_);
-  }
-  i::Handle<i::ObjectHashTable> table(
-      i::ObjectHashTable::cast(weak_collection->table()));
-  if (!table->IsKey(*key)) {
-    DCHECK(false);
-    return Undefined(isolate_);
-  }
-  i::Handle<i::Object> lookup(table->Lookup(key), isolate);
-  if (lookup->IsTheHole()) return Undefined(isolate_);
-  Local<Value> result = Utils::ToLocal(lookup);
-  return handleScope.Escape(result);
-}
-
-
-bool NativeWeakMap::Has(Handle<Value> v8_key) {
-  v8::HandleScope handleScope(isolate_);
-  Local<Object> map_handle = Local<Object>::New(isolate_, map_);
-  i::Isolate* isolate = reinterpret_cast<i::Isolate*>(isolate_);
-  ENTER_V8(isolate);
-  i::Handle<i::Object> key = Utils::OpenHandle(*v8_key);
-  i::Handle<i::JSWeakMap> weak_collection =
-      i::Handle<i::JSWeakMap>::cast(Utils::OpenHandle(*map_handle));
-  if (!key->IsJSReceiver() && !key->IsSymbol()) {
-    DCHECK(false);
-    return false;
-  }
-  i::Handle<i::ObjectHashTable> table(
-      i::ObjectHashTable::cast(weak_collection->table()));
-  if (!table->IsKey(*key)) {
-    DCHECK(false);
-    return false;
-  }
-  i::Handle<i::Object> lookup(table->Lookup(key), isolate);
-  return !lookup->IsTheHole();
-}
-
-
-bool NativeWeakMap::Delete(Handle<Value> v8_key) {
-  v8::HandleScope handleScope(isolate_);
-  Local<Object> map_handle = Local<Object>::New(isolate_, map_);
-  i::Isolate* isolate = reinterpret_cast<i::Isolate*>(isolate_);
-  ENTER_V8(isolate);
-  i::Handle<i::Object> key = Utils::OpenHandle(*v8_key);
-  i::Handle<i::JSWeakMap> weak_collection =
-      i::Handle<i::JSWeakMap>::cast(Utils::OpenHandle(*map_handle));
-  if (!key->IsJSReceiver() && !key->IsSymbol()) {
-    DCHECK(false);
-    return false;
-  }
-  i::Handle<i::ObjectHashTable> table(
-      i::ObjectHashTable::cast(weak_collection->table()));
-  if (!table->IsKey(*key)) {
-    DCHECK(false);
-    return false;
-  }
-  return i::Runtime::WeakCollectionDelete(weak_collection, key);
-}
-
-
 // --- J S O N ---
 
 Local<Value> JSON::Parse(Local<String> json_string) {
index d29c856..3c3021c 100644 (file)
@@ -2237,15 +2237,6 @@ Handle<JSObject> Factory::NewArgumentsObject(Handle<JSFunction> callee,
 }
 
 
-Handle<JSWeakMap> Factory::NewJSWeakMap() {
-  // TODO(adamk): Currently the map is only created three times per
-  // isolate. If it's created more often, the map should be moved into the
-  // strong root list.
-  Handle<Map> map = NewMap(JS_WEAK_MAP_TYPE, JSWeakMap::kSize);
-  return Handle<JSWeakMap>::cast(NewJSObjectFromMap(map));
-}
-
-
 Handle<Map> Factory::ObjectLiteralMapFromCache(Handle<Context> context,
                                                int number_of_properties,
                                                bool* is_result_from_cache) {
index 4dfd98c..24ffe79 100644 (file)
@@ -359,8 +359,6 @@ class Factory FINAL {
     return NewJSObjectFromMap(neander_map());
   }
 
-  Handle<JSWeakMap> NewJSWeakMap();
-
   Handle<JSObject> NewArgumentsObject(Handle<JSFunction> callee, int length);
 
   // JS objects are pretenured when allocated by the bootstrapper and
index ffffbdd..abdd056 100644 (file)
@@ -296,11 +296,12 @@ RUNTIME_FUNCTION(Runtime_MapIteratorNext) {
 }
 
 
-void Runtime::WeakCollectionInitialize(
+static Handle<JSWeakCollection> WeakCollectionInitialize(
     Isolate* isolate, Handle<JSWeakCollection> weak_collection) {
   DCHECK(weak_collection->map()->inobject_properties() == 0);
   Handle<ObjectHashTable> table = ObjectHashTable::New(isolate, 0);
   weak_collection->set_table(*table);
+  return weak_collection;
 }
 
 
@@ -308,8 +309,7 @@ RUNTIME_FUNCTION(Runtime_WeakCollectionInitialize) {
   HandleScope scope(isolate);
   DCHECK(args.length() == 1);
   CONVERT_ARG_HANDLE_CHECKED(JSWeakCollection, weak_collection, 0);
-  Runtime::WeakCollectionInitialize(isolate, weak_collection);
-  return *weak_collection;
+  return *WeakCollectionInitialize(isolate, weak_collection);
 }
 
 
@@ -341,24 +341,6 @@ RUNTIME_FUNCTION(Runtime_WeakCollectionHas) {
 }
 
 
-bool Runtime::WeakCollectionDelete(Handle<JSWeakCollection> weak_collection,
-                                   Handle<Object> key) {
-  DCHECK(key->IsJSReceiver() || key->IsSymbol());
-  Handle<ObjectHashTable> table(
-      ObjectHashTable::cast(weak_collection->table()));
-  DCHECK(table->IsKey(*key));
-  bool was_present = false;
-  Handle<ObjectHashTable> new_table =
-      ObjectHashTable::Remove(table, key, &was_present);
-  weak_collection->set_table(*new_table);
-  if (*table != *new_table) {
-    // Zap the old table since we didn't record slots for its elements.
-    table->FillWithHoles(0, table->length());
-  }
-  return was_present;
-}
-
-
 RUNTIME_FUNCTION(Runtime_WeakCollectionDelete) {
   HandleScope scope(isolate);
   DCHECK(args.length() == 2);
@@ -368,23 +350,15 @@ RUNTIME_FUNCTION(Runtime_WeakCollectionDelete) {
   Handle<ObjectHashTable> table(
       ObjectHashTable::cast(weak_collection->table()));
   RUNTIME_ASSERT(table->IsKey(*key));
-  bool was_present = Runtime::WeakCollectionDelete(weak_collection, key);
-  return isolate->heap()->ToBoolean(was_present);
-}
-
-
-void Runtime::WeakCollectionSet(Handle<JSWeakCollection> weak_collection,
-                                Handle<Object> key, Handle<Object> value) {
-  DCHECK(key->IsJSReceiver() || key->IsSymbol());
-  Handle<ObjectHashTable> table(
-      ObjectHashTable::cast(weak_collection->table()));
-  DCHECK(table->IsKey(*key));
-  Handle<ObjectHashTable> new_table = ObjectHashTable::Put(table, key, value);
+  bool was_present = false;
+  Handle<ObjectHashTable> new_table =
+      ObjectHashTable::Remove(table, key, &was_present);
   weak_collection->set_table(*new_table);
   if (*table != *new_table) {
     // Zap the old table since we didn't record slots for its elements.
     table->FillWithHoles(0, table->length());
   }
+  return isolate->heap()->ToBoolean(was_present);
 }
 
 
@@ -398,7 +372,12 @@ RUNTIME_FUNCTION(Runtime_WeakCollectionSet) {
   Handle<ObjectHashTable> table(
       ObjectHashTable::cast(weak_collection->table()));
   RUNTIME_ASSERT(table->IsKey(*key));
-  Runtime::WeakCollectionSet(weak_collection, key, value);
+  Handle<ObjectHashTable> new_table = ObjectHashTable::Put(table, key, value);
+  weak_collection->set_table(*new_table);
+  if (*table != *new_table) {
+    // Zap the old table since we didn't record slots for its elements.
+    table->FillWithHoles(0, table->length());
+  }
   return *weak_collection;
 }
 
@@ -435,9 +414,14 @@ RUNTIME_FUNCTION(Runtime_GetWeakSetValues) {
 RUNTIME_FUNCTION(Runtime_ObservationWeakMapCreate) {
   HandleScope scope(isolate);
   DCHECK(args.length() == 0);
-  Handle<JSWeakMap> weakmap = isolate->factory()->NewJSWeakMap();
-  Runtime::WeakCollectionInitialize(isolate, weakmap);
-  return *weakmap;
+  // 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 *WeakCollectionInitialize(isolate, weakmap);
 }
 }
 }  // namespace v8::internal
index 86ea420..521a91e 100644 (file)
@@ -874,13 +874,6 @@ class Runtime : public AllStatic {
   MUST_USE_RESULT static MaybeHandle<Object> CreateArrayLiteralBoilerplate(
       Isolate* isolate, Handle<FixedArray> literals,
       Handle<FixedArray> elements);
-
-  static void WeakCollectionInitialize(
-      Isolate* isolate, Handle<JSWeakCollection> weak_collection);
-  static void WeakCollectionSet(Handle<JSWeakCollection> weak_collection,
-                                Handle<Object> key, Handle<Object> value);
-  static bool WeakCollectionDelete(Handle<JSWeakCollection> weak_collection,
-                                   Handle<Object> key);
 };
 
 
index b66d117..9d39547 100644 (file)
@@ -46,7 +46,6 @@
 #include "src/isolate.h"
 #include "src/objects.h"
 #include "src/parser.h"
-#include "src/smart-pointers.h"
 #include "src/snapshot.h"
 #include "src/unicode-inl.h"
 #include "src/utils.h"
@@ -4738,78 +4737,6 @@ TEST(MessageHandler5) {
 }
 
 
-TEST(NativeWeakMap) {
-  v8::Isolate* isolate = CcTest::isolate();
-  i::SmartPointer<v8::NativeWeakMap> weak_map(v8::NativeWeakMap::New(isolate));
-  CHECK(!weak_map.is_empty());
-
-  LocalContext env;
-  HandleScope scope(isolate);
-
-  Local<Object> value = v8::Object::New(isolate);
-
-  Local<Object> local1 = v8::Object::New(isolate);
-  CHECK(!weak_map->Has(local1));
-  CHECK(weak_map->Get(local1)->IsUndefined());
-  weak_map->Set(local1, value);
-  CHECK(weak_map->Has(local1));
-  CHECK(value->Equals(weak_map->Get(local1)));
-
-  WeakCallCounter counter(1234);
-  WeakCallCounterAndPersistent<Value> o1(&counter);
-  WeakCallCounterAndPersistent<Value> o2(&counter);
-  WeakCallCounterAndPersistent<Value> s1(&counter);
-  {
-    HandleScope scope(isolate);
-    Local<v8::Object> obj1 = v8::Object::New(isolate);
-    Local<v8::Object> obj2 = v8::Object::New(isolate);
-    Local<v8::Symbol> sym1 = v8::Symbol::New(isolate);
-
-    weak_map->Set(obj1, value);
-    weak_map->Set(obj2, value);
-    weak_map->Set(sym1, value);
-
-    o1.handle.Reset(isolate, obj1);
-    o2.handle.Reset(isolate, obj2);
-    s1.handle.Reset(isolate, sym1);
-
-    CHECK(weak_map->Has(local1));
-    CHECK(weak_map->Has(obj1));
-    CHECK(weak_map->Has(obj2));
-    CHECK(weak_map->Has(sym1));
-
-    CHECK(value->Equals(weak_map->Get(local1)));
-    CHECK(value->Equals(weak_map->Get(obj1)));
-    CHECK(value->Equals(weak_map->Get(obj2)));
-    CHECK(value->Equals(weak_map->Get(sym1)));
-  }
-  CcTest::heap()->CollectAllGarbage(TestHeap::Heap::kNoGCFlags);
-  {
-    HandleScope scope(isolate);
-    CHECK(value->Equals(weak_map->Get(local1)));
-    CHECK(value->Equals(weak_map->Get(Local<Value>::New(isolate, o1.handle))));
-    CHECK(value->Equals(weak_map->Get(Local<Value>::New(isolate, o2.handle))));
-    CHECK(value->Equals(weak_map->Get(Local<Value>::New(isolate, s1.handle))));
-  }
-
-  o1.handle.SetWeak(&o1, &WeakPointerCallback);
-  o2.handle.SetWeak(&o2, &WeakPointerCallback);
-  s1.handle.SetWeak(&s1, &WeakPointerCallback);
-
-  CcTest::heap()->CollectAllGarbage(TestHeap::Heap::kNoGCFlags);
-  CHECK_EQ(3, counter.NumberOfWeakCalls());
-
-  CHECK(o1.handle.IsEmpty());
-  CHECK(o2.handle.IsEmpty());
-  CHECK(s1.handle.IsEmpty());
-
-  CHECK(value->Equals(weak_map->Get(local1)));
-  CHECK(weak_map->Delete(local1));
-  CHECK(!weak_map->Has(local1));
-  CHECK(weak_map->Get(local1)->IsUndefined());
-}
-
-
 THREADED_TEST(GetSetProperty) {
   LocalContext context;
   v8::Isolate* isolate = context->GetIsolate();
index c3b0c2e..6c19cb8 100644 (file)
@@ -42,7 +42,10 @@ static Isolate* GetIsolateFrom(LocalContext* context) {
 
 
 static Handle<JSWeakMap> AllocateJSWeakMap(Isolate* isolate) {
-  Handle<JSWeakMap> weakmap = isolate->factory()->NewJSWeakMap();
+  Factory* factory = isolate->factory();
+  Handle<Map> map = factory->NewMap(JS_WEAK_MAP_TYPE, JSWeakMap::kSize);
+  Handle<JSObject> weakmap_obj = factory->NewJSObjectFromMap(map);
+  Handle<JSWeakMap> weakmap(JSWeakMap::cast(*weakmap_obj));
   // Do not leak handles for the hash table, it would make entries strong.
   {
     HandleScope scope(isolate);
@@ -55,7 +58,11 @@ static Handle<JSWeakMap> AllocateJSWeakMap(Isolate* isolate) {
 static void PutIntoWeakMap(Handle<JSWeakMap> weakmap,
                            Handle<JSObject> key,
                            Handle<Object> value) {
-  Runtime::WeakCollectionSet(weakmap, key, value);
+  Handle<ObjectHashTable> table = ObjectHashTable::Put(
+      Handle<ObjectHashTable>(ObjectHashTable::cast(weakmap->table())),
+      Handle<JSObject>(JSObject::cast(*key)),
+      value);
+  weakmap->set_table(*table);
 }
 
 static int NumberOfWeakCalls = 0;
index 60f2d45..299cc92 100644 (file)
@@ -58,7 +58,11 @@ static Handle<JSWeakSet> AllocateJSWeakSet(Isolate* isolate) {
 static void PutIntoWeakSet(Handle<JSWeakSet> weakset,
                            Handle<JSObject> key,
                            Handle<Object> value) {
-  Runtime::WeakCollectionSet(weakset, key, value);
+  Handle<ObjectHashTable> table = ObjectHashTable::Put(
+      Handle<ObjectHashTable>(ObjectHashTable::cast(weakset->table())),
+      Handle<JSObject>(JSObject::cast(*key)),
+      value);
+  weakset->set_table(*table);
 }
 
 static int NumberOfWeakCalls = 0;