Skip to content

Commit 1f7e3b3

Browse files
yury-sCommit bot
yury-s
authored and
Commit bot
committed
Revert of Add WeakKeyMap to v8.h (patchset v8#2 id:20001 of https://codereview.chromium.org/891473005/)
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}
1 parent e225675 commit 1f7e3b3

9 files changed

Lines changed: 35 additions & 270 deletions

File tree

‎include/v8.h‎

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1536,32 +1536,6 @@ class V8_EXPORT JSON {
15361536
};
15371537

15381538

1539-
/**
1540-
* A map whose keys are referenced weakly. It is similar to JavaScript WeakMap
1541-
* but can be created without entering a v8::Context and hence shouldn't
1542-
* escape to JavaScript.
1543-
*/
1544-
class V8_EXPORT NativeWeakMap {
1545-
public:
1546-
static NativeWeakMap* New(Isolate* isolate);
1547-
~NativeWeakMap();
1548-
void Set(Handle<Value> key, Handle<Value> value);
1549-
Local<Value> Get(Handle<Value> key);
1550-
bool Has(Handle<Value> key);
1551-
bool Delete(Handle<Value> key);
1552-
1553-
private:
1554-
NativeWeakMap(Isolate* isolate, Handle<Object> weak_map);
1555-
1556-
Isolate* isolate_;
1557-
UniquePersistent<Object> map_;
1558-
1559-
// Disallow copying and assigning.
1560-
NativeWeakMap(NativeWeakMap&);
1561-
void operator=(NativeWeakMap&);
1562-
};
1563-
1564-
15651539
// --- Value ---
15661540

15671541

‎src/api.cc‎

Lines changed: 0 additions & 113 deletions
Original file line numberDiff line numberDiff line change
@@ -2250,119 +2250,6 @@ bool StackFrame::IsConstructor() const {
22502250
}
22512251

22522252

2253-
// --- N a t i v e W e a k M a p ---
2254-
2255-
NativeWeakMap* NativeWeakMap::New(Isolate* v8_isolate) {
2256-
i::Isolate* isolate = reinterpret_cast<i::Isolate*>(v8_isolate);
2257-
ENTER_V8(isolate);
2258-
i::HandleScope scope(isolate);
2259-
i::Handle<i::JSWeakMap> weakmap = isolate->factory()->NewJSWeakMap();
2260-
i::Runtime::WeakCollectionInitialize(isolate, weakmap);
2261-
Local<Object> v8_obj = Utils::ToLocal(i::Handle<i::JSObject>::cast(weakmap));
2262-
return new NativeWeakMap(v8_isolate, v8_obj);
2263-
}
2264-
2265-
2266-
NativeWeakMap::NativeWeakMap(Isolate* isolate, Handle<Object> weak_map)
2267-
: isolate_(isolate), map_(isolate, weak_map) {}
2268-
2269-
2270-
NativeWeakMap::~NativeWeakMap() {}
2271-
2272-
2273-
void NativeWeakMap::Set(Handle<Value> v8_key, Handle<Value> v8_value) {
2274-
v8::HandleScope handleScope(isolate_);
2275-
Local<Object> map_handle = Local<Object>::New(isolate_, map_);
2276-
i::Isolate* isolate = reinterpret_cast<i::Isolate*>(isolate_);
2277-
ENTER_V8(isolate);
2278-
i::Handle<i::Object> key = Utils::OpenHandle(*v8_key);
2279-
i::Handle<i::Object> value = Utils::OpenHandle(*v8_value);
2280-
i::Handle<i::JSWeakMap> weak_collection =
2281-
i::Handle<i::JSWeakMap>::cast(Utils::OpenHandle(*map_handle));
2282-
if (!key->IsJSReceiver() && !key->IsSymbol()) {
2283-
DCHECK(false);
2284-
return;
2285-
}
2286-
i::Handle<i::ObjectHashTable> table(
2287-
i::ObjectHashTable::cast(weak_collection->table()));
2288-
if (!table->IsKey(*key)) {
2289-
DCHECK(false);
2290-
return;
2291-
}
2292-
i::Runtime::WeakCollectionSet(weak_collection, key, value);
2293-
}
2294-
2295-
2296-
Local<Value> NativeWeakMap::Get(Handle<Value> v8_key) {
2297-
v8::EscapableHandleScope handleScope(isolate_);
2298-
Local<Object> map_handle = Local<Object>::New(isolate_, map_);
2299-
i::Isolate* isolate = reinterpret_cast<i::Isolate*>(isolate_);
2300-
ENTER_V8(isolate);
2301-
i::Handle<i::Object> key = Utils::OpenHandle(*v8_key);
2302-
i::Handle<i::JSWeakMap> weak_collection =
2303-
i::Handle<i::JSWeakMap>::cast(Utils::OpenHandle(*map_handle));
2304-
if (!key->IsJSReceiver() && !key->IsSymbol()) {
2305-
DCHECK(false);
2306-
return Undefined(isolate_);
2307-
}
2308-
i::Handle<i::ObjectHashTable> table(
2309-
i::ObjectHashTable::cast(weak_collection->table()));
2310-
if (!table->IsKey(*key)) {
2311-
DCHECK(false);
2312-
return Undefined(isolate_);
2313-
}
2314-
i::Handle<i::Object> lookup(table->Lookup(key), isolate);
2315-
if (lookup->IsTheHole()) return Undefined(isolate_);
2316-
Local<Value> result = Utils::ToLocal(lookup);
2317-
return handleScope.Escape(result);
2318-
}
2319-
2320-
2321-
bool NativeWeakMap::Has(Handle<Value> v8_key) {
2322-
v8::HandleScope handleScope(isolate_);
2323-
Local<Object> map_handle = Local<Object>::New(isolate_, map_);
2324-
i::Isolate* isolate = reinterpret_cast<i::Isolate*>(isolate_);
2325-
ENTER_V8(isolate);
2326-
i::Handle<i::Object> key = Utils::OpenHandle(*v8_key);
2327-
i::Handle<i::JSWeakMap> weak_collection =
2328-
i::Handle<i::JSWeakMap>::cast(Utils::OpenHandle(*map_handle));
2329-
if (!key->IsJSReceiver() && !key->IsSymbol()) {
2330-
DCHECK(false);
2331-
return false;
2332-
}
2333-
i::Handle<i::ObjectHashTable> table(
2334-
i::ObjectHashTable::cast(weak_collection->table()));
2335-
if (!table->IsKey(*key)) {
2336-
DCHECK(false);
2337-
return false;
2338-
}
2339-
i::Handle<i::Object> lookup(table->Lookup(key), isolate);
2340-
return !lookup->IsTheHole();
2341-
}
2342-
2343-
2344-
bool NativeWeakMap::Delete(Handle<Value> v8_key) {
2345-
v8::HandleScope handleScope(isolate_);
2346-
Local<Object> map_handle = Local<Object>::New(isolate_, map_);
2347-
i::Isolate* isolate = reinterpret_cast<i::Isolate*>(isolate_);
2348-
ENTER_V8(isolate);
2349-
i::Handle<i::Object> key = Utils::OpenHandle(*v8_key);
2350-
i::Handle<i::JSWeakMap> weak_collection =
2351-
i::Handle<i::JSWeakMap>::cast(Utils::OpenHandle(*map_handle));
2352-
if (!key->IsJSReceiver() && !key->IsSymbol()) {
2353-
DCHECK(false);
2354-
return false;
2355-
}
2356-
i::Handle<i::ObjectHashTable> table(
2357-
i::ObjectHashTable::cast(weak_collection->table()));
2358-
if (!table->IsKey(*key)) {
2359-
DCHECK(false);
2360-
return false;
2361-
}
2362-
return i::Runtime::WeakCollectionDelete(weak_collection, key);
2363-
}
2364-
2365-
23662253
// --- J S O N ---
23672254

23682255
Local<Value> JSON::Parse(Local<String> json_string) {

‎src/factory.cc‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2237,15 +2237,6 @@ Handle<JSObject> Factory::NewArgumentsObject(Handle<JSFunction> callee,
22372237
}
22382238

22392239

2240-
Handle<JSWeakMap> Factory::NewJSWeakMap() {
2241-
// TODO(adamk): Currently the map is only created three times per
2242-
// isolate. If it's created more often, the map should be moved into the
2243-
// strong root list.
2244-
Handle<Map> map = NewMap(JS_WEAK_MAP_TYPE, JSWeakMap::kSize);
2245-
return Handle<JSWeakMap>::cast(NewJSObjectFromMap(map));
2246-
}
2247-
2248-
22492240
Handle<Map> Factory::ObjectLiteralMapFromCache(Handle<Context> context,
22502241
int number_of_properties,
22512242
bool* is_result_from_cache) {

‎src/factory.h‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -359,8 +359,6 @@ class Factory FINAL {
359359
return NewJSObjectFromMap(neander_map());
360360
}
361361

362-
Handle<JSWeakMap> NewJSWeakMap();
363-
364362
Handle<JSObject> NewArgumentsObject(Handle<JSFunction> callee, int length);
365363

366364
// JS objects are pretenured when allocated by the bootstrapper and

‎src/runtime/runtime-collections.cc‎

Lines changed: 21 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -296,20 +296,20 @@ RUNTIME_FUNCTION(Runtime_MapIteratorNext) {
296296
}
297297

298298

299-
void Runtime::WeakCollectionInitialize(
299+
static Handle<JSWeakCollection> WeakCollectionInitialize(
300300
Isolate* isolate, Handle<JSWeakCollection> weak_collection) {
301301
DCHECK(weak_collection->map()->inobject_properties() == 0);
302302
Handle<ObjectHashTable> table = ObjectHashTable::New(isolate, 0);
303303
weak_collection->set_table(*table);
304+
return weak_collection;
304305
}
305306

306307

307308
RUNTIME_FUNCTION(Runtime_WeakCollectionInitialize) {
308309
HandleScope scope(isolate);
309310
DCHECK(args.length() == 1);
310311
CONVERT_ARG_HANDLE_CHECKED(JSWeakCollection, weak_collection, 0);
311-
Runtime::WeakCollectionInitialize(isolate, weak_collection);
312-
return *weak_collection;
312+
return *WeakCollectionInitialize(isolate, weak_collection);
313313
}
314314

315315

@@ -341,24 +341,6 @@ RUNTIME_FUNCTION(Runtime_WeakCollectionHas) {
341341
}
342342

343343

344-
bool Runtime::WeakCollectionDelete(Handle<JSWeakCollection> weak_collection,
345-
Handle<Object> key) {
346-
DCHECK(key->IsJSReceiver() || key->IsSymbol());
347-
Handle<ObjectHashTable> table(
348-
ObjectHashTable::cast(weak_collection->table()));
349-
DCHECK(table->IsKey(*key));
350-
bool was_present = false;
351-
Handle<ObjectHashTable> new_table =
352-
ObjectHashTable::Remove(table, key, &was_present);
353-
weak_collection->set_table(*new_table);
354-
if (*table != *new_table) {
355-
// Zap the old table since we didn't record slots for its elements.
356-
table->FillWithHoles(0, table->length());
357-
}
358-
return was_present;
359-
}
360-
361-
362344
RUNTIME_FUNCTION(Runtime_WeakCollectionDelete) {
363345
HandleScope scope(isolate);
364346
DCHECK(args.length() == 2);
@@ -368,23 +350,15 @@ RUNTIME_FUNCTION(Runtime_WeakCollectionDelete) {
368350
Handle<ObjectHashTable> table(
369351
ObjectHashTable::cast(weak_collection->table()));
370352
RUNTIME_ASSERT(table->IsKey(*key));
371-
bool was_present = Runtime::WeakCollectionDelete(weak_collection, key);
372-
return isolate->heap()->ToBoolean(was_present);
373-
}
374-
375-
376-
void Runtime::WeakCollectionSet(Handle<JSWeakCollection> weak_collection,
377-
Handle<Object> key, Handle<Object> value) {
378-
DCHECK(key->IsJSReceiver() || key->IsSymbol());
379-
Handle<ObjectHashTable> table(
380-
ObjectHashTable::cast(weak_collection->table()));
381-
DCHECK(table->IsKey(*key));
382-
Handle<ObjectHashTable> new_table = ObjectHashTable::Put(table, key, value);
353+
bool was_present = false;
354+
Handle<ObjectHashTable> new_table =
355+
ObjectHashTable::Remove(table, key, &was_present);
383356
weak_collection->set_table(*new_table);
384357
if (*table != *new_table) {
385358
// Zap the old table since we didn't record slots for its elements.
386359
table->FillWithHoles(0, table->length());
387360
}
361+
return isolate->heap()->ToBoolean(was_present);
388362
}
389363

390364

@@ -398,7 +372,12 @@ RUNTIME_FUNCTION(Runtime_WeakCollectionSet) {
398372
Handle<ObjectHashTable> table(
399373
ObjectHashTable::cast(weak_collection->table()));
400374
RUNTIME_ASSERT(table->IsKey(*key));
401-
Runtime::WeakCollectionSet(weak_collection, key, value);
375+
Handle<ObjectHashTable> new_table = ObjectHashTable::Put(table, key, value);
376+
weak_collection->set_table(*new_table);
377+
if (*table != *new_table) {
378+
// Zap the old table since we didn't record slots for its elements.
379+
table->FillWithHoles(0, table->length());
380+
}
402381
return *weak_collection;
403382
}
404383

@@ -435,9 +414,14 @@ RUNTIME_FUNCTION(Runtime_GetWeakSetValues) {
435414
RUNTIME_FUNCTION(Runtime_ObservationWeakMapCreate) {
436415
HandleScope scope(isolate);
437416
DCHECK(args.length() == 0);
438-
Handle<JSWeakMap> weakmap = isolate->factory()->NewJSWeakMap();
439-
Runtime::WeakCollectionInitialize(isolate, weakmap);
440-
return *weakmap;
417+
// TODO(adamk): Currently this runtime function is only called three times per
418+
// isolate. If it's called more often, the map should be moved into the
419+
// strong root list.
420+
Handle<Map> map =
421+
isolate->factory()->NewMap(JS_WEAK_MAP_TYPE, JSWeakMap::kSize);
422+
Handle<JSWeakMap> weakmap =
423+
Handle<JSWeakMap>::cast(isolate->factory()->NewJSObjectFromMap(map));
424+
return *WeakCollectionInitialize(isolate, weakmap);
441425
}
442426
}
443427
} // namespace v8::internal

‎src/runtime/runtime.h‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -874,13 +874,6 @@ class Runtime : public AllStatic {
874874
MUST_USE_RESULT static MaybeHandle<Object> CreateArrayLiteralBoilerplate(
875875
Isolate* isolate, Handle<FixedArray> literals,
876876
Handle<FixedArray> elements);
877-
878-
static void WeakCollectionInitialize(
879-
Isolate* isolate, Handle<JSWeakCollection> weak_collection);
880-
static void WeakCollectionSet(Handle<JSWeakCollection> weak_collection,
881-
Handle<Object> key, Handle<Object> value);
882-
static bool WeakCollectionDelete(Handle<JSWeakCollection> weak_collection,
883-
Handle<Object> key);
884877
};
885878

886879

‎test/cctest/test-api.cc‎

Lines changed: 0 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@
4646
#include "src/isolate.h"
4747
#include "src/objects.h"
4848
#include "src/parser.h"
49-
#include "src/smart-pointers.h"
5049
#include "src/snapshot.h"
5150
#include "src/unicode-inl.h"
5251
#include "src/utils.h"
@@ -4738,78 +4737,6 @@ TEST(MessageHandler5) {
47384737
}
47394738

47404739

4741-
TEST(NativeWeakMap) {
4742-
v8::Isolate* isolate = CcTest::isolate();
4743-
i::SmartPointer<v8::NativeWeakMap> weak_map(v8::NativeWeakMap::New(isolate));
4744-
CHECK(!weak_map.is_empty());
4745-
4746-
LocalContext env;
4747-
HandleScope scope(isolate);
4748-
4749-
Local<Object> value = v8::Object::New(isolate);
4750-
4751-
Local<Object> local1 = v8::Object::New(isolate);
4752-
CHECK(!weak_map->Has(local1));
4753-
CHECK(weak_map->Get(local1)->IsUndefined());
4754-
weak_map->Set(local1, value);
4755-
CHECK(weak_map->Has(local1));
4756-
CHECK(value->Equals(weak_map->Get(local1)));
4757-
4758-
WeakCallCounter counter(1234);
4759-
WeakCallCounterAndPersistent<Value> o1(&counter);
4760-
WeakCallCounterAndPersistent<Value> o2(&counter);
4761-
WeakCallCounterAndPersistent<Value> s1(&counter);
4762-
{
4763-
HandleScope scope(isolate);
4764-
Local<v8::Object> obj1 = v8::Object::New(isolate);
4765-
Local<v8::Object> obj2 = v8::Object::New(isolate);
4766-
Local<v8::Symbol> sym1 = v8::Symbol::New(isolate);
4767-
4768-
weak_map->Set(obj1, value);
4769-
weak_map->Set(obj2, value);
4770-
weak_map->Set(sym1, value);
4771-
4772-
o1.handle.Reset(isolate, obj1);
4773-
o2.handle.Reset(isolate, obj2);
4774-
s1.handle.Reset(isolate, sym1);
4775-
4776-
CHECK(weak_map->Has(local1));
4777-
CHECK(weak_map->Has(obj1));
4778-
CHECK(weak_map->Has(obj2));
4779-
CHECK(weak_map->Has(sym1));
4780-
4781-
CHECK(value->Equals(weak_map->Get(local1)));
4782-
CHECK(value->Equals(weak_map->Get(obj1)));
4783-
CHECK(value->Equals(weak_map->Get(obj2)));
4784-
CHECK(value->Equals(weak_map->Get(sym1)));
4785-
}
4786-
CcTest::heap()->CollectAllGarbage(TestHeap::Heap::kNoGCFlags);
4787-
{
4788-
HandleScope scope(isolate);
4789-
CHECK(value->Equals(weak_map->Get(local1)));
4790-
CHECK(value->Equals(weak_map->Get(Local<Value>::New(isolate, o1.handle))));
4791-
CHECK(value->Equals(weak_map->Get(Local<Value>::New(isolate, o2.handle))));
4792-
CHECK(value->Equals(weak_map->Get(Local<Value>::New(isolate, s1.handle))));
4793-
}
4794-
4795-
o1.handle.SetWeak(&o1, &WeakPointerCallback);
4796-
o2.handle.SetWeak(&o2, &WeakPointerCallback);
4797-
s1.handle.SetWeak(&s1, &WeakPointerCallback);
4798-
4799-
CcTest::heap()->CollectAllGarbage(TestHeap::Heap::kNoGCFlags);
4800-
CHECK_EQ(3, counter.NumberOfWeakCalls());
4801-
4802-
CHECK(o1.handle.IsEmpty());
4803-
CHECK(o2.handle.IsEmpty());
4804-
CHECK(s1.handle.IsEmpty());
4805-
4806-
CHECK(value->Equals(weak_map->Get(local1)));
4807-
CHECK(weak_map->Delete(local1));
4808-
CHECK(!weak_map->Has(local1));
4809-
CHECK(weak_map->Get(local1)->IsUndefined());
4810-
}
4811-
4812-
48134740
THREADED_TEST(GetSetProperty) {
48144741
LocalContext context;
48154742
v8::Isolate* isolate = context->GetIsolate();

0 commit comments

Comments
 (0)