From 431abca0caed834fc23fe146c98eee46efa9d0e9 Mon Sep 17 00:00:00 2001 From: Adam Klein Date: Thu, 15 Jun 2017 21:35:42 +0000 Subject: [PATCH] Revert "[builtins] Move most WeakMap/WeakSet code from JS to C++ builtins" This reverts commit 8196e102654935eb780c8fa868b97d3409b7c0e6. Reason for revert: Performance regression due to hashcode lookup. Original change's description: > [builtins] Move most WeakMap/WeakSet code from JS to C++ builtins > > They were already implemented mostly in C++ (only error/negative > cases were handled in script), so this is mostly just a cleanup. > Only the constructors remain in script after this CL. > > Bug: v8:6354 > Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng > Change-Id: I5b3579337a8e33dc30d49c2da5cfd42baec697bb > Reviewed-on: https://chromium-review.googlesource.com/531670 > Reviewed-by: Camillo Bruni > Reviewed-by: Sathya Gunasekaran > Commit-Queue: Adam Klein > Cr-Commit-Position: refs/heads/master@{#45924} TBR=adamk@chromium.org,cbruni@chromium.org,gsathya@chromium.org Bug: v8:6354, chromium:733238 Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng Change-Id: Ia5a741b9587886298f3ca057f6a6adeba556b8e0 Reviewed-on: https://chromium-review.googlesource.com/537207 Reviewed-by: Adam Klein Commit-Queue: Adam Klein Cr-Commit-Position: refs/heads/master@{#45966} --- BUILD.gn | 1 - src/api.cc | 6 +- src/bootstrapper.cc | 35 ++----- src/builtins/builtins-definitions.h | 11 -- src/builtins/builtins-weak-collection.cc | 107 ------------------- src/js/weak-collection.js | 126 ++++++++++++++++++++++- src/objects.cc | 12 ++- src/objects.h | 5 +- src/runtime/runtime-collections.cc | 62 +++++++++++ src/runtime/runtime.h | 4 + src/v8.gyp | 1 - test/cctest/test-weakmaps.cc | 15 ++- test/cctest/test-weaksets.cc | 12 ++- 13 files changed, 227 insertions(+), 170 deletions(-) delete mode 100644 src/builtins/builtins-weak-collection.cc diff --git a/BUILD.gn b/BUILD.gn index 851b91f2d0..0cee354264 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -1201,7 +1201,6 @@ v8_source_set("v8_base") { "src/builtins/builtins-symbol.cc", "src/builtins/builtins-typedarray.cc", "src/builtins/builtins-utils.h", - "src/builtins/builtins-weak-collection.cc", "src/builtins/builtins.cc", "src/builtins/builtins.h", "src/cached-powers.cc", diff --git a/src/api.cc b/src/api.cc index 50bf23224b..e920a156a2 100644 --- a/src/api.cc +++ b/src/api.cc @@ -3091,7 +3091,8 @@ void NativeWeakMap::Set(Local v8_key, Local v8_value) { DCHECK(false); return; } - i::JSWeakCollection::Set(weak_collection, key, value); + int32_t hash = i::Object::GetOrCreateHash(isolate, key)->value(); + i::JSWeakCollection::Set(weak_collection, key, value, hash); } Local NativeWeakMap::Get(Local v8_key) const { @@ -3153,7 +3154,8 @@ bool NativeWeakMap::Delete(Local v8_key) { DCHECK(false); return false; } - return i::JSWeakCollection::Delete(weak_collection, key); + int32_t hash = i::Object::GetOrCreateHash(isolate, key)->value(); + return i::JSWeakCollection::Delete(weak_collection, key, hash); } diff --git a/src/bootstrapper.cc b/src/bootstrapper.cc index e6051a19a6..74a1cd69e0 100644 --- a/src/bootstrapper.cc +++ b/src/bootstrapper.cc @@ -3098,42 +3098,19 @@ void Genesis::InitializeGlobal(Handle global_object, } { // -- W e a k M a p - Handle prototype = - factory->NewJSObject(isolate->object_function(), TENURED); - Handle js_weak_map_fun = - InstallFunction(global, "WeakMap", JS_WEAK_MAP_TYPE, JSWeakMap::kSize, - prototype, Builtins::kIllegal); + Handle js_weak_map_fun = InstallFunction( + global, "WeakMap", JS_WEAK_MAP_TYPE, JSWeakMap::kSize, + isolate->initial_object_prototype(), Builtins::kIllegal); InstallWithIntrinsicDefaultProto(isolate, js_weak_map_fun, Context::JS_WEAK_MAP_FUN_INDEX); - JSObject::AddProperty(prototype, factory->constructor_string(), - js_weak_map_fun, DONT_ENUM); - JSObject::AddProperty( - prototype, factory->to_string_tag_symbol(), factory->WeakMap_string(), - static_cast(DONT_ENUM | READ_ONLY)); - SimpleInstallFunction(prototype, "get", Builtins::kWeakMapGet, 1, true); - SimpleInstallFunction(prototype, "has", Builtins::kWeakMapHas, 1, true); - SimpleInstallFunction(prototype, "delete", Builtins::kWeakMapDelete, 1, - true); - SimpleInstallFunction(prototype, "set", Builtins::kWeakMapSet, 2, true); } { // -- W e a k S e t - Handle prototype = - factory->NewJSObject(isolate->object_function(), TENURED); - Handle js_weak_set_fun = - InstallFunction(global, "WeakSet", JS_WEAK_SET_TYPE, JSWeakSet::kSize, - prototype, Builtins::kIllegal); + Handle js_weak_set_fun = InstallFunction( + global, "WeakSet", JS_WEAK_SET_TYPE, JSWeakSet::kSize, + isolate->initial_object_prototype(), Builtins::kIllegal); InstallWithIntrinsicDefaultProto(isolate, js_weak_set_fun, Context::JS_WEAK_SET_FUN_INDEX); - JSObject::AddProperty(prototype, factory->constructor_string(), - js_weak_set_fun, DONT_ENUM); - JSObject::AddProperty( - prototype, factory->to_string_tag_symbol(), factory->WeakSet_string(), - static_cast(DONT_ENUM | READ_ONLY)); - SimpleInstallFunction(prototype, "add", Builtins::kWeakSetAdd, 1, true); - SimpleInstallFunction(prototype, "delete", Builtins::kWeakSetDelete, 1, - true); - SimpleInstallFunction(prototype, "has", Builtins::kWeakSetHas, 1, true); } { // -- P r o x y diff --git a/src/builtins/builtins-definitions.h b/src/builtins/builtins-definitions.h index 1d0f7c66e3..4bba5003ba 100644 --- a/src/builtins/builtins-definitions.h +++ b/src/builtins/builtins-definitions.h @@ -583,17 +583,6 @@ namespace internal { CPP(MapClear) \ CPP(MapForEach) \ \ - /* WeakMap */ \ - CPP(WeakMapGet) \ - CPP(WeakMapSet) \ - CPP(WeakMapDelete) \ - CPP(WeakMapHas) \ - \ - /* WeakSet */ \ - CPP(WeakSetAdd) \ - CPP(WeakSetDelete) \ - CPP(WeakSetHas) \ - \ /* Math */ \ /* ES6 #sec-math.abs */ \ TFJ(MathAbs, 1, kX) \ diff --git a/src/builtins/builtins-weak-collection.cc b/src/builtins/builtins-weak-collection.cc deleted file mode 100644 index dd0de5c5e1..0000000000 --- a/src/builtins/builtins-weak-collection.cc +++ /dev/null @@ -1,107 +0,0 @@ -// Copyright 2017 the V8 project authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "src/builtins/builtins-utils.h" -#include "src/builtins/builtins.h" - -#include "src/objects-inl.h" - -namespace v8 { -namespace internal { - -namespace { - -// Returns the value keyed by |key|, or the hole if |key| is not present. -Object* WeakCollectionLookup(Handle collection, - Handle key) { - return ObjectHashTable::cast(collection->table())->Lookup(key); -} - -Object* WeakCollectionHas(Isolate* isolate, Handle collection, - Handle key) { - return isolate->heap()->ToBoolean( - !WeakCollectionLookup(collection, key)->IsTheHole(isolate)); -} - -} // anonymous namespace - -BUILTIN(WeakMapGet) { - HandleScope scope(isolate); - CHECK_RECEIVER(JSWeakMap, map, "WeakMap.prototype.get"); - if (!args.atOrUndefined(isolate, 1)->IsJSReceiver()) { - return isolate->heap()->undefined_value(); - } - Handle key = args.at(1); - Object* lookup = WeakCollectionLookup(map, key); - return lookup->IsTheHole(isolate) ? isolate->heap()->undefined_value() - : lookup; -} - -BUILTIN(WeakMapHas) { - HandleScope scope(isolate); - CHECK_RECEIVER(JSWeakMap, map, "WeakMap.prototype.has"); - if (!args.atOrUndefined(isolate, 1)->IsJSReceiver()) { - return isolate->heap()->false_value(); - } - Handle key = args.at(1); - return WeakCollectionHas(isolate, map, key); -} - -BUILTIN(WeakMapDelete) { - HandleScope scope(isolate); - CHECK_RECEIVER(JSWeakMap, map, "WeakMap.prototype.delete"); - if (!args.atOrUndefined(isolate, 1)->IsJSReceiver()) { - return isolate->heap()->false_value(); - } - Handle key = args.at(1); - return isolate->heap()->ToBoolean(JSWeakCollection::Delete(map, key)); -} - -BUILTIN(WeakMapSet) { - HandleScope scope(isolate); - CHECK_RECEIVER(JSWeakMap, map, "WeakMap.prototype.set"); - if (!args.atOrUndefined(isolate, 1)->IsJSReceiver()) { - THROW_NEW_ERROR_RETURN_FAILURE( - isolate, NewTypeError(MessageTemplate::kInvalidWeakMapKey)); - } - Handle key = args.at(1); - Handle value = args.atOrUndefined(isolate, 2); - JSWeakCollection::Set(map, key, value); - return *map; -} - -BUILTIN(WeakSetAdd) { - HandleScope scope(isolate); - CHECK_RECEIVER(JSWeakSet, set, "WeakSet.prototype.set"); - if (!args.atOrUndefined(isolate, 1)->IsJSReceiver()) { - THROW_NEW_ERROR_RETURN_FAILURE( - isolate, NewTypeError(MessageTemplate::kInvalidWeakSetValue)); - } - Handle key = args.at(1); - JSWeakCollection::Set(set, key, isolate->factory()->true_value()); - return *set; -} - -BUILTIN(WeakSetDelete) { - HandleScope scope(isolate); - CHECK_RECEIVER(JSWeakSet, set, "WeakSet.prototype.delete"); - if (!args.atOrUndefined(isolate, 1)->IsJSReceiver()) { - return isolate->heap()->false_value(); - } - Handle key = args.at(1); - return isolate->heap()->ToBoolean(JSWeakCollection::Delete(set, key)); -} - -BUILTIN(WeakSetHas) { - HandleScope scope(isolate); - CHECK_RECEIVER(JSWeakSet, set, "WeakSet.prototype.has"); - if (!args.atOrUndefined(isolate, 1)->IsJSReceiver()) { - return isolate->heap()->false_value(); - } - Handle key = args.at(1); - return WeakCollectionHas(isolate, set, key); -} - -} // namespace internal -} // namespace v8 diff --git a/src/js/weak-collection.js b/src/js/weak-collection.js index f4522c490c..f5092d29f5 100644 --- a/src/js/weak-collection.js +++ b/src/js/weak-collection.js @@ -11,11 +11,20 @@ // ------------------------------------------------------------------- // Imports +var GetExistingHash; +var GetHash; +var GlobalObject = global.Object; var GlobalWeakMap = global.WeakMap; var GlobalWeakSet = global.WeakSet; +var toStringTagSymbol = utils.ImportNow("to_string_tag_symbol"); + +utils.Import(function(from) { + GetExistingHash = from.GetExistingHash; + GetHash = from.GetHash; +}); // ------------------------------------------------------------------- -// WeakMap +// Harmony WeakMap function WeakMapConstructor(iterable) { if (IS_UNDEFINED(new.target)) { @@ -38,11 +47,73 @@ function WeakMapConstructor(iterable) { } } -%SetCode(GlobalWeakMap, WeakMapConstructor); -%FunctionSetLength(GlobalWeakMap, 0); + +function WeakMapGet(key) { + if (!IS_WEAKMAP(this)) { + throw %make_type_error(kIncompatibleMethodReceiver, + 'WeakMap.prototype.get', this); + } + if (!IS_RECEIVER(key)) return UNDEFINED; + var hash = GetExistingHash(key); + if (IS_UNDEFINED(hash)) return UNDEFINED; + return %WeakCollectionGet(this, key, hash); +} + + +function WeakMapSet(key, value) { + if (!IS_WEAKMAP(this)) { + throw %make_type_error(kIncompatibleMethodReceiver, + 'WeakMap.prototype.set', this); + } + if (!IS_RECEIVER(key)) throw %make_type_error(kInvalidWeakMapKey); + return %WeakCollectionSet(this, key, value, GetHash(key)); +} + + +function WeakMapHas(key) { + if (!IS_WEAKMAP(this)) { + throw %make_type_error(kIncompatibleMethodReceiver, + 'WeakMap.prototype.has', this); + } + if (!IS_RECEIVER(key)) return false; + var hash = GetExistingHash(key); + if (IS_UNDEFINED(hash)) return false; + return %WeakCollectionHas(this, key, hash); +} + + +function WeakMapDelete(key) { + if (!IS_WEAKMAP(this)) { + throw %make_type_error(kIncompatibleMethodReceiver, + 'WeakMap.prototype.delete', this); + } + if (!IS_RECEIVER(key)) return false; + var hash = GetExistingHash(key); + if (IS_UNDEFINED(hash)) return false; + return %WeakCollectionDelete(this, key, hash); +} + // ------------------------------------------------------------------- -// WeakSet + +%SetCode(GlobalWeakMap, WeakMapConstructor); +%FunctionSetLength(GlobalWeakMap, 0); +%FunctionSetPrototype(GlobalWeakMap, new GlobalObject()); +%AddNamedProperty(GlobalWeakMap.prototype, "constructor", GlobalWeakMap, + DONT_ENUM); +%AddNamedProperty(GlobalWeakMap.prototype, toStringTagSymbol, "WeakMap", + DONT_ENUM | READ_ONLY); + +// Set up the non-enumerable functions on the WeakMap prototype object. +utils.InstallFunctions(GlobalWeakMap.prototype, DONT_ENUM, [ + "get", WeakMapGet, + "set", WeakMapSet, + "has", WeakMapHas, + "delete", WeakMapDelete +]); + +// ------------------------------------------------------------------- +// Harmony WeakSet function WeakSetConstructor(iterable) { if (IS_UNDEFINED(new.target)) { @@ -62,9 +133,56 @@ function WeakSetConstructor(iterable) { } } + +function WeakSetAdd(value) { + if (!IS_WEAKSET(this)) { + throw %make_type_error(kIncompatibleMethodReceiver, + 'WeakSet.prototype.add', this); + } + if (!IS_RECEIVER(value)) throw %make_type_error(kInvalidWeakSetValue); + return %WeakCollectionSet(this, value, true, GetHash(value)); +} + + +function WeakSetHas(value) { + if (!IS_WEAKSET(this)) { + throw %make_type_error(kIncompatibleMethodReceiver, + 'WeakSet.prototype.has', this); + } + if (!IS_RECEIVER(value)) return false; + var hash = GetExistingHash(value); + if (IS_UNDEFINED(hash)) return false; + return %WeakCollectionHas(this, value, hash); +} + + +function WeakSetDelete(value) { + if (!IS_WEAKSET(this)) { + throw %make_type_error(kIncompatibleMethodReceiver, + 'WeakSet.prototype.delete', this); + } + if (!IS_RECEIVER(value)) return false; + var hash = GetExistingHash(value); + if (IS_UNDEFINED(hash)) return false; + return %WeakCollectionDelete(this, value, hash); +} + + // ------------------------------------------------------------------- %SetCode(GlobalWeakSet, WeakSetConstructor); %FunctionSetLength(GlobalWeakSet, 0); +%FunctionSetPrototype(GlobalWeakSet, new GlobalObject()); +%AddNamedProperty(GlobalWeakSet.prototype, "constructor", GlobalWeakSet, + DONT_ENUM); +%AddNamedProperty(GlobalWeakSet.prototype, toStringTagSymbol, "WeakSet", + DONT_ENUM | READ_ONLY); + +// Set up the non-enumerable functions on the WeakSet prototype object. +utils.InstallFunctions(GlobalWeakSet.prototype, DONT_ENUM, [ + "add", WeakSetAdd, + "has", WeakSetHas, + "delete", WeakSetDelete +]); }) diff --git a/src/objects.cc b/src/objects.cc index 9e612a4a04..3fb75a4573 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -18897,13 +18897,16 @@ void JSWeakCollection::Initialize(Handle weak_collection, weak_collection->set_table(*table); } + void JSWeakCollection::Set(Handle weak_collection, - Handle key, Handle value) { + Handle key, Handle value, + int32_t hash) { DCHECK(key->IsJSReceiver() || key->IsSymbol()); Handle table( ObjectHashTable::cast(weak_collection->table())); DCHECK(table->IsKey(table->GetIsolate(), *key)); - Handle new_table = ObjectHashTable::Put(table, key, value); + Handle new_table = + ObjectHashTable::Put(table, key, value, hash); weak_collection->set_table(*new_table); if (*table != *new_table) { // Zap the old table since we didn't record slots for its elements. @@ -18911,15 +18914,16 @@ void JSWeakCollection::Set(Handle weak_collection, } } + bool JSWeakCollection::Delete(Handle weak_collection, - Handle key) { + Handle key, int32_t hash) { DCHECK(key->IsJSReceiver() || key->IsSymbol()); Handle table( ObjectHashTable::cast(weak_collection->table())); DCHECK(table->IsKey(table->GetIsolate(), *key)); bool was_present = false; Handle new_table = - ObjectHashTable::Remove(table, key, &was_present); + ObjectHashTable::Remove(table, key, &was_present, hash); weak_collection->set_table(*new_table); if (*table != *new_table) { // Zap the old table since we didn't record slots for its elements. diff --git a/src/objects.h b/src/objects.h index 2e6a1e5b40..85125c5259 100644 --- a/src/objects.h +++ b/src/objects.h @@ -6351,8 +6351,9 @@ class JSWeakCollection: public JSObject { static void Initialize(Handle collection, Isolate* isolate); static void Set(Handle collection, Handle key, - Handle value); - static bool Delete(Handle collection, Handle key); + Handle value, int32_t hash); + static bool Delete(Handle collection, Handle key, + int32_t hash); static Handle GetEntries(Handle holder, int max_entries); diff --git a/src/runtime/runtime-collections.cc b/src/runtime/runtime-collections.cc index a6eb29a70a..ec1e3f0786 100644 --- a/src/runtime/runtime-collections.cc +++ b/src/runtime/runtime-collections.cc @@ -255,6 +255,68 @@ RUNTIME_FUNCTION(Runtime_WeakCollectionInitialize) { } +RUNTIME_FUNCTION(Runtime_WeakCollectionGet) { + HandleScope scope(isolate); + DCHECK_EQ(3, args.length()); + CONVERT_ARG_HANDLE_CHECKED(JSWeakCollection, weak_collection, 0); + CONVERT_ARG_HANDLE_CHECKED(Object, key, 1); + CONVERT_SMI_ARG_CHECKED(hash, 2) + CHECK(key->IsJSReceiver() || key->IsSymbol()); + Handle table( + ObjectHashTable::cast(weak_collection->table())); + CHECK(table->IsKey(isolate, *key)); + Handle lookup(table->Lookup(key, hash), isolate); + return lookup->IsTheHole(isolate) ? isolate->heap()->undefined_value() + : *lookup; +} + + +RUNTIME_FUNCTION(Runtime_WeakCollectionHas) { + HandleScope scope(isolate); + DCHECK_EQ(3, args.length()); + CONVERT_ARG_HANDLE_CHECKED(JSWeakCollection, weak_collection, 0); + CONVERT_ARG_HANDLE_CHECKED(Object, key, 1); + CONVERT_SMI_ARG_CHECKED(hash, 2) + CHECK(key->IsJSReceiver() || key->IsSymbol()); + Handle table( + ObjectHashTable::cast(weak_collection->table())); + CHECK(table->IsKey(isolate, *key)); + Handle lookup(table->Lookup(key, hash), isolate); + return isolate->heap()->ToBoolean(!lookup->IsTheHole(isolate)); +} + + +RUNTIME_FUNCTION(Runtime_WeakCollectionDelete) { + HandleScope scope(isolate); + DCHECK_EQ(3, args.length()); + CONVERT_ARG_HANDLE_CHECKED(JSWeakCollection, weak_collection, 0); + CONVERT_ARG_HANDLE_CHECKED(Object, key, 1); + CONVERT_SMI_ARG_CHECKED(hash, 2) + CHECK(key->IsJSReceiver() || key->IsSymbol()); + Handle table( + ObjectHashTable::cast(weak_collection->table())); + CHECK(table->IsKey(isolate, *key)); + bool was_present = JSWeakCollection::Delete(weak_collection, key, hash); + return isolate->heap()->ToBoolean(was_present); +} + + +RUNTIME_FUNCTION(Runtime_WeakCollectionSet) { + HandleScope scope(isolate); + DCHECK_EQ(4, args.length()); + CONVERT_ARG_HANDLE_CHECKED(JSWeakCollection, weak_collection, 0); + CONVERT_ARG_HANDLE_CHECKED(Object, key, 1); + CHECK(key->IsJSReceiver() || key->IsSymbol()); + CONVERT_ARG_HANDLE_CHECKED(Object, value, 2); + CONVERT_SMI_ARG_CHECKED(hash, 3) + Handle table( + ObjectHashTable::cast(weak_collection->table())); + CHECK(table->IsKey(isolate, *key)); + JSWeakCollection::Set(weak_collection, key, value, hash); + return *weak_collection; +} + + RUNTIME_FUNCTION(Runtime_GetWeakSetValues) { HandleScope scope(isolate); DCHECK_EQ(2, args.length()); diff --git a/src/runtime/runtime.h b/src/runtime/runtime.h index 338ee93686..b5cdbdc0a5 100644 --- a/src/runtime/runtime.h +++ b/src/runtime/runtime.h @@ -114,6 +114,10 @@ namespace internal { F(GetWeakMapEntries, 2, 1) \ F(MapIteratorNext, 2, 1) \ F(WeakCollectionInitialize, 1, 1) \ + F(WeakCollectionGet, 3, 1) \ + F(WeakCollectionHas, 3, 1) \ + F(WeakCollectionDelete, 3, 1) \ + F(WeakCollectionSet, 4, 1) \ F(GetWeakSetValues, 2, 1) \ F(IsJSMap, 1, 1) \ F(IsJSSet, 1, 1) \ diff --git a/src/v8.gyp b/src/v8.gyp index f0fbee2617..4736893504 100644 --- a/src/v8.gyp +++ b/src/v8.gyp @@ -658,7 +658,6 @@ 'builtins/builtins-symbol.cc', 'builtins/builtins-typedarray.cc', 'builtins/builtins-utils.h', - 'builtins/builtins-weak-collection.cc', 'builtins/builtins.cc', 'builtins/builtins.h', 'cached-powers.cc', diff --git a/test/cctest/test-weakmaps.cc b/test/cctest/test-weakmaps.cc index 6b8f72d309..9c88456eb5 100644 --- a/test/cctest/test-weakmaps.cc +++ b/test/cctest/test-weakmaps.cc @@ -96,8 +96,10 @@ TEST(Weakness) { Handle map = factory->NewMap(JS_OBJECT_TYPE, JSObject::kHeaderSize); Handle object = factory->NewJSObjectFromMap(map); Handle smi(Smi::FromInt(23), isolate); - JSWeakCollection::Set(weakmap, key, object); - JSWeakCollection::Set(weakmap, object, smi); + int32_t hash = Object::GetOrCreateHash(isolate, key)->value(); + JSWeakCollection::Set(weakmap, key, object, hash); + int32_t object_hash = Object::GetOrCreateHash(isolate, object)->value(); + JSWeakCollection::Set(weakmap, object, smi, object_hash); } CHECK_EQ(2, ObjectHashTable::cast(weakmap->table())->NumberOfElements()); @@ -140,7 +142,8 @@ TEST(Shrinking) { for (int i = 0; i < 32; i++) { Handle object = factory->NewJSObjectFromMap(map); Handle smi(Smi::FromInt(i), isolate); - JSWeakCollection::Set(weakmap, object, smi); + int32_t object_hash = Object::GetOrCreateHash(isolate, object)->value(); + JSWeakCollection::Set(weakmap, object, smi, object_hash); } } @@ -187,7 +190,8 @@ TEST(Regress2060a) { Handle object = factory->NewJSObject(function, TENURED); CHECK(!heap->InNewSpace(*object)); CHECK(!first_page->Contains(object->address())); - JSWeakCollection::Set(weakmap, key, object); + int32_t hash = Object::GetOrCreateHash(isolate, key)->value(); + JSWeakCollection::Set(weakmap, key, object, hash); } } @@ -228,7 +232,8 @@ TEST(Regress2060b) { Handle weakmap = AllocateJSWeakMap(isolate); for (int i = 0; i < 32; i++) { Handle smi(Smi::FromInt(i), isolate); - JSWeakCollection::Set(weakmap, keys[i], smi); + int32_t hash = Object::GetOrCreateHash(isolate, keys[i])->value(); + JSWeakCollection::Set(weakmap, keys[i], smi, hash); } // Force compacting garbage collection. The subsequent collections are used diff --git a/test/cctest/test-weaksets.cc b/test/cctest/test-weaksets.cc index 656fac284f..f0d3354f4f 100644 --- a/test/cctest/test-weaksets.cc +++ b/test/cctest/test-weaksets.cc @@ -97,7 +97,8 @@ TEST(WeakSet_Weakness) { { HandleScope scope(isolate); Handle smi(Smi::FromInt(23), isolate); - JSWeakCollection::Set(weakset, key, smi); + int32_t hash = Object::GetOrCreateHash(isolate, key)->value(); + JSWeakCollection::Set(weakset, key, smi, hash); } CHECK_EQ(1, ObjectHashTable::cast(weakset->table())->NumberOfElements()); @@ -140,7 +141,8 @@ TEST(WeakSet_Shrinking) { for (int i = 0; i < 32; i++) { Handle object = factory->NewJSObjectFromMap(map); Handle smi(Smi::FromInt(i), isolate); - JSWeakCollection::Set(weakset, object, smi); + int32_t hash = Object::GetOrCreateHash(isolate, object)->value(); + JSWeakCollection::Set(weakset, object, smi, hash); } } @@ -187,7 +189,8 @@ TEST(WeakSet_Regress2060a) { Handle object = factory->NewJSObject(function, TENURED); CHECK(!heap->InNewSpace(*object)); CHECK(!first_page->Contains(object->address())); - JSWeakCollection::Set(weakset, key, object); + int32_t hash = Object::GetOrCreateHash(isolate, key)->value(); + JSWeakCollection::Set(weakset, key, object, hash); } } @@ -228,7 +231,8 @@ TEST(WeakSet_Regress2060b) { Handle weakset = AllocateJSWeakSet(isolate); for (int i = 0; i < 32; i++) { Handle smi(Smi::FromInt(i), isolate); - JSWeakCollection::Set(weakset, keys[i], smi); + int32_t hash = Object::GetOrCreateHash(isolate, keys[i])->value(); + JSWeakCollection::Set(weakset, keys[i], smi, hash); } // Force compacting garbage collection. The subsequent collections are used