From ee7ed39ac8327124e74dd7ad5f1de0dede988cb7 Mon Sep 17 00:00:00 2001 From: yurys Date: Wed, 4 Feb 2015 04:52:53 -0800 Subject: [PATCH] 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 Review URL: https://codereview.chromium.org/891473005 Cr-Commit-Position: refs/heads/master@{#26425} --- include/v8.h | 26 +++++++ src/api.cc | 113 +++++++++++++++++++++++++++++ src/factory.cc | 9 +++ src/factory.h | 2 + src/runtime/runtime-collections.cc | 58 +++++++++------ src/runtime/runtime.h | 7 ++ test/cctest/test-api.cc | 73 +++++++++++++++++++ test/cctest/test-weakmaps.cc | 11 +-- test/cctest/test-weaksets.cc | 6 +- 9 files changed, 270 insertions(+), 35 deletions(-) diff --git a/include/v8.h b/include/v8.h index e8ca3df18f..a799fceaed 100644 --- a/include/v8.h +++ b/include/v8.h @@ -1536,6 +1536,32 @@ 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 key, Handle value); + Local Get(Handle key); + bool Has(Handle key); + bool Delete(Handle key); + + private: + NativeWeakMap(Isolate* isolate, Handle weak_map); + + Isolate* isolate_; + UniquePersistent map_; + + // Disallow copying and assigning. + NativeWeakMap(NativeWeakMap&); + void operator=(NativeWeakMap&); +}; + + // --- Value --- diff --git a/src/api.cc b/src/api.cc index d282607bee..a6d950e93c 100644 --- a/src/api.cc +++ b/src/api.cc @@ -2249,6 +2249,119 @@ 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(v8_isolate); + ENTER_V8(isolate); + i::HandleScope scope(isolate); + i::Handle weakmap = isolate->factory()->NewJSWeakMap(); + i::Runtime::WeakCollectionInitialize(isolate, weakmap); + Local v8_obj = Utils::ToLocal(i::Handle::cast(weakmap)); + return new NativeWeakMap(v8_isolate, v8_obj); +} + + +NativeWeakMap::NativeWeakMap(Isolate* isolate, Handle weak_map) + : isolate_(isolate), map_(isolate, weak_map) {} + + +NativeWeakMap::~NativeWeakMap() {} + + +void NativeWeakMap::Set(Handle v8_key, Handle v8_value) { + v8::HandleScope handleScope(isolate_); + Local map_handle = Local::New(isolate_, map_); + i::Isolate* isolate = reinterpret_cast(isolate_); + ENTER_V8(isolate); + i::Handle key = Utils::OpenHandle(*v8_key); + i::Handle value = Utils::OpenHandle(*v8_value); + i::Handle weak_collection = + i::Handle::cast(Utils::OpenHandle(*map_handle)); + if (!key->IsJSReceiver() && !key->IsSymbol()) { + DCHECK(false); + return; + } + i::Handle table( + i::ObjectHashTable::cast(weak_collection->table())); + if (!table->IsKey(*key)) { + DCHECK(false); + return; + } + i::Runtime::WeakCollectionSet(weak_collection, key, value); +} + + +Local NativeWeakMap::Get(Handle v8_key) { + v8::EscapableHandleScope handleScope(isolate_); + Local map_handle = Local::New(isolate_, map_); + i::Isolate* isolate = reinterpret_cast(isolate_); + ENTER_V8(isolate); + i::Handle key = Utils::OpenHandle(*v8_key); + i::Handle weak_collection = + i::Handle::cast(Utils::OpenHandle(*map_handle)); + if (!key->IsJSReceiver() && !key->IsSymbol()) { + DCHECK(false); + return Undefined(isolate_); + } + i::Handle table( + i::ObjectHashTable::cast(weak_collection->table())); + if (!table->IsKey(*key)) { + DCHECK(false); + return Undefined(isolate_); + } + i::Handle lookup(table->Lookup(key), isolate); + if (lookup->IsTheHole()) return Undefined(isolate_); + Local result = Utils::ToLocal(lookup); + return handleScope.Escape(result); +} + + +bool NativeWeakMap::Has(Handle v8_key) { + v8::HandleScope handleScope(isolate_); + Local map_handle = Local::New(isolate_, map_); + i::Isolate* isolate = reinterpret_cast(isolate_); + ENTER_V8(isolate); + i::Handle key = Utils::OpenHandle(*v8_key); + i::Handle weak_collection = + i::Handle::cast(Utils::OpenHandle(*map_handle)); + if (!key->IsJSReceiver() && !key->IsSymbol()) { + DCHECK(false); + return false; + } + i::Handle table( + i::ObjectHashTable::cast(weak_collection->table())); + if (!table->IsKey(*key)) { + DCHECK(false); + return false; + } + i::Handle lookup(table->Lookup(key), isolate); + return !lookup->IsTheHole(); +} + + +bool NativeWeakMap::Delete(Handle v8_key) { + v8::HandleScope handleScope(isolate_); + Local map_handle = Local::New(isolate_, map_); + i::Isolate* isolate = reinterpret_cast(isolate_); + ENTER_V8(isolate); + i::Handle key = Utils::OpenHandle(*v8_key); + i::Handle weak_collection = + i::Handle::cast(Utils::OpenHandle(*map_handle)); + if (!key->IsJSReceiver() && !key->IsSymbol()) { + DCHECK(false); + return false; + } + i::Handle 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 JSON::Parse(Local json_string) { diff --git a/src/factory.cc b/src/factory.cc index 7f6b811158..13ef723b01 100644 --- a/src/factory.cc +++ b/src/factory.cc @@ -2237,6 +2237,15 @@ Handle Factory::NewArgumentsObject(Handle callee, } +Handle 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 = NewMap(JS_WEAK_MAP_TYPE, JSWeakMap::kSize); + return Handle::cast(NewJSObjectFromMap(map)); +} + + Handle Factory::CreateApiFunction( Handle obj, Handle prototype, diff --git a/src/factory.h b/src/factory.h index 141fc5e66e..5be5c124eb 100644 --- a/src/factory.h +++ b/src/factory.h @@ -359,6 +359,8 @@ class Factory FINAL { return NewJSObjectFromMap(neander_map()); } + Handle NewJSWeakMap(); + Handle NewArgumentsObject(Handle callee, int length); // JS objects are pretenured when allocated by the bootstrapper and diff --git a/src/runtime/runtime-collections.cc b/src/runtime/runtime-collections.cc index abdd056998..ffffbdd2f2 100644 --- a/src/runtime/runtime-collections.cc +++ b/src/runtime/runtime-collections.cc @@ -296,12 +296,11 @@ RUNTIME_FUNCTION(Runtime_MapIteratorNext) { } -static Handle WeakCollectionInitialize( +void Runtime::WeakCollectionInitialize( Isolate* isolate, Handle weak_collection) { DCHECK(weak_collection->map()->inobject_properties() == 0); Handle table = ObjectHashTable::New(isolate, 0); weak_collection->set_table(*table); - return weak_collection; } @@ -309,7 +308,8 @@ RUNTIME_FUNCTION(Runtime_WeakCollectionInitialize) { HandleScope scope(isolate); DCHECK(args.length() == 1); CONVERT_ARG_HANDLE_CHECKED(JSWeakCollection, weak_collection, 0); - return *WeakCollectionInitialize(isolate, weak_collection); + Runtime::WeakCollectionInitialize(isolate, weak_collection); + return *weak_collection; } @@ -341,6 +341,24 @@ RUNTIME_FUNCTION(Runtime_WeakCollectionHas) { } +bool Runtime::WeakCollectionDelete(Handle weak_collection, + Handle key) { + DCHECK(key->IsJSReceiver() || key->IsSymbol()); + Handle table( + ObjectHashTable::cast(weak_collection->table())); + DCHECK(table->IsKey(*key)); + bool was_present = false; + Handle 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); @@ -350,15 +368,23 @@ RUNTIME_FUNCTION(Runtime_WeakCollectionDelete) { Handle table( ObjectHashTable::cast(weak_collection->table())); RUNTIME_ASSERT(table->IsKey(*key)); - bool was_present = false; - Handle new_table = - ObjectHashTable::Remove(table, key, &was_present); + bool was_present = Runtime::WeakCollectionDelete(weak_collection, key); + return isolate->heap()->ToBoolean(was_present); +} + + +void Runtime::WeakCollectionSet(Handle weak_collection, + Handle key, Handle value) { + DCHECK(key->IsJSReceiver() || key->IsSymbol()); + Handle table( + ObjectHashTable::cast(weak_collection->table())); + DCHECK(table->IsKey(*key)); + Handle 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 isolate->heap()->ToBoolean(was_present); } @@ -372,12 +398,7 @@ RUNTIME_FUNCTION(Runtime_WeakCollectionSet) { Handle table( ObjectHashTable::cast(weak_collection->table())); RUNTIME_ASSERT(table->IsKey(*key)); - Handle 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()); - } + Runtime::WeakCollectionSet(weak_collection, key, value); return *weak_collection; } @@ -414,14 +435,9 @@ RUNTIME_FUNCTION(Runtime_GetWeakSetValues) { RUNTIME_FUNCTION(Runtime_ObservationWeakMapCreate) { HandleScope scope(isolate); DCHECK(args.length() == 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 = - isolate->factory()->NewMap(JS_WEAK_MAP_TYPE, JSWeakMap::kSize); - Handle weakmap = - Handle::cast(isolate->factory()->NewJSObjectFromMap(map)); - return *WeakCollectionInitialize(isolate, weakmap); + Handle weakmap = isolate->factory()->NewJSWeakMap(); + Runtime::WeakCollectionInitialize(isolate, weakmap); + return *weakmap; } } } // namespace v8::internal diff --git a/src/runtime/runtime.h b/src/runtime/runtime.h index 89ccd50767..734f0a411d 100644 --- a/src/runtime/runtime.h +++ b/src/runtime/runtime.h @@ -879,6 +879,13 @@ class Runtime : public AllStatic { MUST_USE_RESULT static MaybeHandle CreateArrayLiteralBoilerplate( Isolate* isolate, Handle literals, Handle elements); + + static void WeakCollectionInitialize( + Isolate* isolate, Handle weak_collection); + static void WeakCollectionSet(Handle weak_collection, + Handle key, Handle value); + static bool WeakCollectionDelete(Handle weak_collection, + Handle key); }; diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index dd5358c6c3..5b3311a622 100644 --- a/test/cctest/test-api.cc +++ b/test/cctest/test-api.cc @@ -46,6 +46,7 @@ #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" @@ -4737,6 +4738,78 @@ TEST(MessageHandler5) { } +TEST(NativeWeakMap) { + v8::Isolate* isolate = CcTest::isolate(); + i::SmartPointer weak_map(v8::NativeWeakMap::New(isolate)); + CHECK(!weak_map.is_empty()); + + LocalContext env; + HandleScope scope(isolate); + + Local value = v8::Object::New(isolate); + + Local 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 o1(&counter); + WeakCallCounterAndPersistent o2(&counter); + WeakCallCounterAndPersistent s1(&counter); + { + HandleScope scope(isolate); + Local obj1 = v8::Object::New(isolate); + Local obj2 = v8::Object::New(isolate); + Local 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::New(isolate, o1.handle)))); + CHECK(value->Equals(weak_map->Get(Local::New(isolate, o2.handle)))); + CHECK(value->Equals(weak_map->Get(Local::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(); diff --git a/test/cctest/test-weakmaps.cc b/test/cctest/test-weakmaps.cc index 6c19cb8e47..c3b0c2efd1 100644 --- a/test/cctest/test-weakmaps.cc +++ b/test/cctest/test-weakmaps.cc @@ -42,10 +42,7 @@ static Isolate* GetIsolateFrom(LocalContext* context) { static Handle AllocateJSWeakMap(Isolate* isolate) { - Factory* factory = isolate->factory(); - Handle map = factory->NewMap(JS_WEAK_MAP_TYPE, JSWeakMap::kSize); - Handle weakmap_obj = factory->NewJSObjectFromMap(map); - Handle weakmap(JSWeakMap::cast(*weakmap_obj)); + Handle weakmap = isolate->factory()->NewJSWeakMap(); // Do not leak handles for the hash table, it would make entries strong. { HandleScope scope(isolate); @@ -58,11 +55,7 @@ static Handle AllocateJSWeakMap(Isolate* isolate) { static void PutIntoWeakMap(Handle weakmap, Handle key, Handle value) { - Handle table = ObjectHashTable::Put( - Handle(ObjectHashTable::cast(weakmap->table())), - Handle(JSObject::cast(*key)), - value); - weakmap->set_table(*table); + Runtime::WeakCollectionSet(weakmap, key, value); } static int NumberOfWeakCalls = 0; diff --git a/test/cctest/test-weaksets.cc b/test/cctest/test-weaksets.cc index 299cc92e9b..60f2d45dc3 100644 --- a/test/cctest/test-weaksets.cc +++ b/test/cctest/test-weaksets.cc @@ -58,11 +58,7 @@ static Handle AllocateJSWeakSet(Isolate* isolate) { static void PutIntoWeakSet(Handle weakset, Handle key, Handle value) { - Handle table = ObjectHashTable::Put( - Handle(ObjectHashTable::cast(weakset->table())), - Handle(JSObject::cast(*key)), - value); - weakset->set_table(*table); + Runtime::WeakCollectionSet(weakset, key, value); } static int NumberOfWeakCalls = 0;