Commit 7ae0a2f9 authored by peterwmwong's avatar peterwmwong Committed by Commit Bot

[builtins] Port WeakMap.p.set and WeakSet.p.add to CSA from JS

- Add WeakMapPrototypeSet and WeakSetPrototypeAdd TFJ builtins
  - Fast paths for...
    1) existing key
    2) new key when ObjectHashTable has a "sufficient capacity"
- Create WeakCollectionsBuiltinsAssembler to consolidate common WeakMap/WeakSet code generation
- Convert existing WeakMapLookupHashIndex to use WeakCollectionsBuiltinsAssembler

Some quick benchmarks shows performance gains of...
- 1.56x - 1.98x for WeakMap constructor
- 1.66x - 2.06x for WeakSet constructor
- 1.50x - 2.11x for WeakMap.p.set
- 1.54x - 2.26x for WeakSet.p.add

https: //github.com/peterwmwong/v8-perf/blob/master/weakcollection-set/README.md
Bug: v8:5049, v8:6604
Change-Id: I3499d46be6b2b3b1d8d46720ebe86cc5142ee542
Reviewed-on: https://chromium-review.googlesource.com/737935
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: 's avatarJakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#49036}
parent 574d51d5
......@@ -1391,6 +1391,13 @@ ExternalReference ExternalReference::get_or_create_hash_raw(Isolate* isolate) {
return ExternalReference(Redirect(isolate, FUNCTION_ADDR(f)));
}
ExternalReference ExternalReference::jsreceiver_create_identity_hash(
Isolate* isolate) {
typedef Smi* (*CreateIdentityHash)(Isolate * isolate, JSReceiver * key);
CreateIdentityHash f = JSReceiver::CreateIdentityHash;
return ExternalReference(Redirect(isolate, FUNCTION_ADDR(f)));
}
ExternalReference ExternalReference::try_internalize_string_function(
Isolate* isolate) {
return ExternalReference(Redirect(
......
......@@ -972,6 +972,7 @@ class ExternalReference BASE_EMBEDDED {
static ExternalReference orderedhashmap_gethash_raw(Isolate* isolate);
static ExternalReference get_or_create_hash_raw(Isolate* isolate);
static ExternalReference jsreceiver_create_identity_hash(Isolate* isolate);
static ExternalReference page_flags(Page* page);
......
......@@ -3293,6 +3293,8 @@ void Genesis::InitializeGlobal(Handle<JSGlobalObject> global_object,
SimpleInstallFunction(prototype, "get", Builtins::kWeakMapGet, 1, true);
SimpleInstallFunction(prototype, "has", Builtins::kWeakMapHas, 1, true);
SimpleInstallFunction(prototype, "set", Builtins::kWeakMapPrototypeSet, 2,
true);
JSObject::AddProperty(
prototype, factory->to_string_tag_symbol(),
......@@ -3310,6 +3312,8 @@ void Genesis::InitializeGlobal(Handle<JSGlobalObject> global_object,
Handle<JSObject> prototype(JSObject::cast(cons->instance_prototype()));
SimpleInstallFunction(prototype, "has", Builtins::kWeakSetHas, 1, true);
SimpleInstallFunction(prototype, "add", Builtins::kWeakSetPrototypeAdd, 1,
true);
JSObject::AddProperty(
prototype, factory->to_string_tag_symbol(),
......
This diff is collapsed.
......@@ -1089,9 +1089,14 @@ namespace internal {
TFS(WeakMapLookupHashIndex, kTable, kKey) \
TFJ(WeakMapGet, 1, kKey) \
TFJ(WeakMapHas, 1, kKey) \
TFJ(WeakMapPrototypeSet, 2, kKey, kValue) \
\
/* WeakSet */ \
TFJ(WeakSetHas, 1, kKey) \
TFJ(WeakSetPrototypeAdd, 1, kValue) \
\
/* WeakSet / WeakMap Helpers */ \
TFS(WeakCollectionSet, kCollection, kKey, kValue) \
\
/* AsyncGenerator */ \
\
......
......@@ -1382,6 +1382,63 @@ Node* CodeStubAssembler::LoadMapEnumLength(SloppyTNode<Map> map) {
return DecodeWordFromWord32<Map::EnumLengthBits>(bit_field3);
}
TNode<IntPtrT> CodeStubAssembler::LoadJSReceiverIdentityHash(
SloppyTNode<Object> receiver, Label* if_no_hash) {
TVARIABLE(IntPtrT, var_hash);
Label done(this), if_smi(this), if_property_array(this),
if_property_dictionary(this), if_fixed_array(this);
TNode<Object> properties_or_hash =
LoadObjectField(TNode<HeapObject>::UncheckedCast(receiver),
JSReceiver::kPropertiesOrHashOffset);
GotoIf(TaggedIsSmi(properties_or_hash), &if_smi);
TNode<HeapObject> properties =
TNode<HeapObject>::UncheckedCast(properties_or_hash);
TNode<Int32T> properties_instance_type = LoadInstanceType(properties);
GotoIf(InstanceTypeEqual(properties_instance_type, PROPERTY_ARRAY_TYPE),
&if_property_array);
Branch(InstanceTypeEqual(properties_instance_type, HASH_TABLE_TYPE),
&if_property_dictionary, &if_fixed_array);
BIND(&if_fixed_array);
{
var_hash = IntPtrConstant(PropertyArray::kNoHashSentinel);
Goto(&done);
}
BIND(&if_smi);
{
var_hash = SmiUntag(TNode<Smi>::UncheckedCast(properties_or_hash));
Goto(&done);
}
BIND(&if_property_array);
{
TNode<IntPtrT> length_and_hash = LoadAndUntagObjectField(
properties, PropertyArray::kLengthAndHashOffset);
var_hash = TNode<IntPtrT>::UncheckedCast(
DecodeWord<PropertyArray::HashField>(length_and_hash));
Goto(&done);
}
BIND(&if_property_dictionary);
{
var_hash = SmiUntag(
LoadFixedArrayElement(properties, NameDictionary::kObjectHashIndex));
Goto(&done);
}
BIND(&done);
if (if_no_hash != nullptr) {
GotoIf(
IntPtrEqual(var_hash, IntPtrConstant(PropertyArray::kNoHashSentinel)),
if_no_hash);
}
return var_hash;
}
TNode<Uint32T> CodeStubAssembler::LoadNameHashField(SloppyTNode<Name> name) {
CSA_ASSERT(this, IsName(name));
return LoadObjectField<Uint32T>(name, Name::kHashFieldOffset);
......
......@@ -512,6 +512,9 @@ class V8_EXPORT_PRIVATE CodeStubAssembler : public compiler::CodeAssembler {
TNode<Object> LoadMapConstructor(SloppyTNode<Map> map);
// Load the EnumLength of a Map.
Node* LoadMapEnumLength(SloppyTNode<Map> map);
// Load the identity hash of a JSRececiver.
TNode<IntPtrT> LoadJSReceiverIdentityHash(SloppyTNode<Object> receiver,
Label* if_no_hash = nullptr);
// This is only used on a newly allocated PropertyArray which
// doesn't have an existing hash.
......
......@@ -269,6 +269,8 @@ void ExternalReferenceTable::AddReferences(Isolate* isolate) {
"orderedhashmap_gethash_raw");
Add(ExternalReference::get_or_create_hash_raw(isolate).address(),
"get_or_create_hash_raw");
Add(ExternalReference::jsreceiver_create_identity_hash(isolate).address(),
"jsreceiver_create_identity_hash");
Add(ExternalReference::log_enter_external_function(isolate).address(),
"Logger::EnterExternal");
Add(ExternalReference::log_leave_external_function(isolate).address(),
......
......@@ -44,15 +44,6 @@ function WeakMapConstructor(iterable) {
DEFINE_METHODS(
GlobalWeakMap.prototype,
{
set(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, %GenericHash(key));
}
delete(key) {
if (!IS_WEAKMAP(this)) {
throw %make_type_error(kIncompatibleMethodReceiver,
......@@ -97,15 +88,6 @@ function WeakSetConstructor(iterable) {
DEFINE_METHODS(
GlobalWeakSet.prototype,
{
add(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, %GenericHash(value));
}
delete(value) {
if (!IS_WEAKSET(this)) {
throw %make_type_error(kIncompatibleMethodReceiver,
......
......@@ -6479,6 +6479,16 @@ Object* JSReceiver::GetIdentityHash(Isolate* isolate) {
return Smi::FromInt(hash);
}
// static
Smi* JSReceiver::CreateIdentityHash(Isolate* isolate, JSReceiver* key) {
DisallowHeapAllocation no_gc;
int hash = isolate->GenerateIdentityHash(PropertyArray::HashField::kMax);
DCHECK_NE(PropertyArray::kNoHashSentinel, hash);
key->SetIdentityHash(hash);
return Smi::FromInt(hash);
}
Smi* JSReceiver::GetOrCreateIdentityHash(Isolate* isolate) {
DisallowHeapAllocation no_gc;
......@@ -6487,11 +6497,7 @@ Smi* JSReceiver::GetOrCreateIdentityHash(Isolate* isolate) {
return Smi::cast(hash_obj);
}
int hash = isolate->GenerateIdentityHash(PropertyArray::HashField::kMax);
DCHECK_NE(PropertyArray::kNoHashSentinel, hash);
SetIdentityHash(hash);
return Smi::FromInt(hash);
return JSReceiver::CreateIdentityHash(isolate, this);
}
Maybe<bool> JSObject::DeletePropertyWithInterceptor(LookupIterator* it,
......
......@@ -2162,6 +2162,7 @@ class JSReceiver: public HeapObject {
// Retrieves a permanent object identity hash code. May create and store a
// hash code if needed and none exists.
static Smi* CreateIdentityHash(Isolate* isolate, JSReceiver* key);
Smi* GetOrCreateIdentityHash(Isolate* isolate);
// Stores the hash code. The hash passed in must be masked with
......
......@@ -130,12 +130,20 @@ RUNTIME_FUNCTION(Runtime_WeakCollectionSet) {
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)
#ifdef DEBUG
DCHECK(key->IsJSReceiver());
DCHECK(ObjectHashTableShape::IsLive(isolate, *key));
Handle<ObjectHashTable> table(
ObjectHashTable::cast(weak_collection->table()));
CHECK(table->IsKey(isolate, *key));
// Should only be called when rehashing or resizing the table is necessary.
// See ObjectHashTable::Put() and HashTable::HasSufficientCapacityToAdd().
DCHECK((table->NumberOfDeletedElements() << 1) > table->NumberOfElements() ||
!table->HasSufficientCapacityToAdd(1));
#endif
JSWeakCollection::Set(weak_collection, key, value, hash);
return *weak_collection;
}
......
......@@ -175,6 +175,16 @@ test(function() {
}, "Method Set.prototype.add called on incompatible receiver [object Array]",
TypeError);
test(function() {
WeakSet.prototype.add.call([]);
}, "Method WeakSet.prototype.add called on incompatible receiver [object Array]",
TypeError);
test(function() {
WeakMap.prototype.set.call([]);
}, "Method WeakMap.prototype.set called on incompatible receiver [object Array]",
TypeError);
// kNonCallableInInstanceOfCheck
test(function() {
1 instanceof {};
......@@ -198,6 +208,24 @@ test(function() {
1 in 1;
}, "Cannot use 'in' operator to search for '1' in 1", TypeError);
// kInvalidWeakMapKey
test(function() {
new WeakMap([[1, 1]]);
}, "Invalid value used as weak map key", TypeError);
test(function() {
new WeakMap().set(1, 1);
}, "Invalid value used as weak map key", TypeError);
// kInvalidWeakSetValue
test(function() {
new WeakSet([1]);
}, "Invalid value used in weak set", TypeError);
test(function() {
new WeakSet().add(1);
}, "Invalid value used in weak set", TypeError);
// kIteratorResultNotAnObject
test(function() {
var obj = {};
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment