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

- Add WeakMapPrototypeDelete and WeakSetPrototypeDelete TFJ builtins
  - Fast paths when it's not necessary to shrink the table
- Add WeakCollectionDelete TFS

Some quick benchmarks shows 1.4x - 2.15x gains in performance.
https://github.com/peterwmwong/v8-perf/blob/master/weakcollection-delete/README.md

Bug: v8:5049, v8:6604
Change-Id: I14036df153f3a0242f9083d751658b868b16660a
Reviewed-on: https://chromium-review.googlesource.com/743864Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#49076}

src/bootstrapper.cc

@@ -3292,6 +3292,8 @@ void Genesis::InitializeGlobal(Handle<JSGlobalObject> global_object,
     // Setup %WeakMapPrototype%.
     Handle<JSObject> prototype(JSObject::cast(cons->instance_prototype()));
 
+    SimpleInstallFunction(prototype, "delete",
+                          Builtins::kWeakMapPrototypeDelete, 1, true);
     SimpleInstallFunction(prototype, "get", Builtins::kWeakMapGet, 1, true);
     SimpleInstallFunction(prototype, "has", Builtins::kWeakMapHas, 1, true);
     SimpleInstallFunction(prototype, "set", Builtins::kWeakMapPrototypeSet, 2,
@@ -3312,6 +3314,8 @@ void Genesis::InitializeGlobal(Handle<JSGlobalObject> global_object,
     // Setup %WeakSetPrototype%.
     Handle<JSObject> prototype(JSObject::cast(cons->instance_prototype()));
 
+    SimpleInstallFunction(prototype, "delete",
+                          Builtins::kWeakSetPrototypeDelete, 1, true);
     SimpleInstallFunction(prototype, "has", Builtins::kWeakSetHas, 1, true);
     SimpleInstallFunction(prototype, "add", Builtins::kWeakSetPrototypeAdd, 1,
                           true);

src/builtins/builtins-collections-gen.cc

@@ -1901,6 +1901,22 @@ TNode<Word32T> WeakCollectionsBuiltinsAssembler::InsufficientCapacityToAdd(
                         capacity));
 }
 
+void WeakCollectionsBuiltinsAssembler::RemoveEntry(
+    TNode<Object> table, TNode<IntPtrT> key_index,
+    TNode<IntPtrT> number_of_elements) {
+  // See ObjectHashTable::RemoveEntry().
+  TNode<IntPtrT> value_index = ValueIndexFromKeyIndex(key_index);
+  StoreFixedArrayElement(table, key_index, TheHoleConstant());
+  StoreFixedArrayElement(table, value_index, TheHoleConstant());
+
+  // See HashTableBase::ElementRemoved().
+  TNode<IntPtrT> number_of_deleted = LoadNumberOfDeleted(table, 1);
+  StoreFixedArrayElement(table, ObjectHashTable::kNumberOfElementsIndex,
+                         SmiFromWord(number_of_elements), SKIP_WRITE_BARRIER);
+  StoreFixedArrayElement(table, ObjectHashTable::kNumberOfDeletedElementsIndex,
+                         SmiFromWord(number_of_deleted), SKIP_WRITE_BARRIER);
+}
+
 TNode<BoolT> WeakCollectionsBuiltinsAssembler::ShouldRehash(
     TNode<IntPtrT> number_of_elements, TNode<IntPtrT> number_of_deleted) {
   // Rehash if more than 33% of the entries are deleted.
@@ -1908,6 +1924,22 @@ TNode<BoolT> WeakCollectionsBuiltinsAssembler::ShouldRehash(
                                   number_of_elements);
 }
 
+TNode<Word32T> WeakCollectionsBuiltinsAssembler::ShouldShrink(
+    TNode<IntPtrT> capacity, TNode<IntPtrT> number_of_elements) {
+  // See HashTable::Shrink().
+  TNode<IntPtrT> quarter_capacity = WordShr(capacity, 2);
+  return Word32And(
+      // Shrink to fit the number of elements if only a quarter of the
+      // capacity is filled with elements.
+      IntPtrLessThanOrEqual(number_of_elements, quarter_capacity),
+
+      // Allocate a new dictionary with room for at least the current
+      // number of elements. The allocation method will make sure that
+      // there is extra room in the dictionary for additions. Don't go
+      // lower than room for 16 elements.
+      IntPtrGreaterThanOrEqual(number_of_elements, IntPtrConstant(16)));
+}
+
 TNode<IntPtrT> WeakCollectionsBuiltinsAssembler::ValueIndexFromKeyIndex(
     TNode<IntPtrT> key_index) {
   return IntPtrAdd(key_index,
@@ -1978,6 +2010,37 @@ TF_BUILTIN(WeakMapHas, WeakCollectionsBuiltinsAssembler) {
   Return(FalseConstant());
 }
 
+// Helper that removes the entry with a given key from the backing store
+// (ObjectHashTable) of a WeakMap or WeakSet.
+TF_BUILTIN(WeakCollectionDelete, WeakCollectionsBuiltinsAssembler) {
+  TNode<Context> context = CAST(Parameter(Descriptor::kContext));
+  TNode<Object> collection = CAST(Parameter(Descriptor::kCollection));
+  TNode<Object> key = CAST(Parameter(Descriptor::kKey));
+
+  Label call_runtime(this), if_not_found(this);
+
+  GotoIf(TaggedIsSmi(key), &if_not_found);
+  GotoIfNot(IsJSReceiver(key), &if_not_found);
+
+  TNode<IntPtrT> hash = LoadJSReceiverIdentityHash(key, &if_not_found);
+  TNode<Object> table = LoadTable(collection);
+  TNode<IntPtrT> capacity = LoadTableCapacity(table);
+  TNode<IntPtrT> key_index =
+      FindKeyIndexForKey(table, key, hash, EntryMask(capacity), &if_not_found);
+  TNode<IntPtrT> number_of_elements = LoadNumberOfElements(table, -1);
+  GotoIf(ShouldShrink(capacity, number_of_elements), &call_runtime);
+
+  RemoveEntry(table, key_index, number_of_elements);
+  Return(TrueConstant());
+
+  BIND(&if_not_found);
+  Return(FalseConstant());
+
+  BIND(&call_runtime);
+  Return(CallRuntime(Runtime::kWeakCollectionDelete, context, collection, key,
+                     SmiTag(hash)));
+}
+
 // Helper that sets the key and value to the backing store (ObjectHashTable) of
 // a WeakMap or WeakSet.
 TF_BUILTIN(WeakCollectionSet, WeakCollectionsBuiltinsAssembler) {
@@ -2030,6 +2093,17 @@ TF_BUILTIN(WeakCollectionSet, WeakCollectionsBuiltinsAssembler) {
   }
 }
 
+TF_BUILTIN(WeakMapPrototypeDelete, CodeStubAssembler) {
+  TNode<Context> context = CAST(Parameter(Descriptor::kContext));
+  TNode<Object> receiver = CAST(Parameter(Descriptor::kReceiver));
+  TNode<Object> key = CAST(Parameter(Descriptor::kKey));
+
+  ThrowIfNotInstanceType(context, receiver, JS_WEAK_MAP_TYPE,
+                         "WeakMap.prototype.delete");
+
+  Return(CallBuiltin(Builtins::kWeakCollectionDelete, context, receiver, key));
+}
+
 TF_BUILTIN(WeakMapPrototypeSet, CodeStubAssembler) {
   TNode<Context> context = CAST(Parameter(Descriptor::kContext));
   TNode<Object> receiver = CAST(Parameter(Descriptor::kReceiver));
@@ -2069,6 +2143,18 @@ TF_BUILTIN(WeakSetPrototypeAdd, CodeStubAssembler) {
   ThrowTypeError(context, MessageTemplate::kInvalidWeakSetValue, value);
 }
 
+TF_BUILTIN(WeakSetPrototypeDelete, CodeStubAssembler) {
+  TNode<Context> context = CAST(Parameter(Descriptor::kContext));
+  TNode<Object> receiver = CAST(Parameter(Descriptor::kReceiver));
+  TNode<Object> value = CAST(Parameter(Descriptor::kValue));
+
+  ThrowIfNotInstanceType(context, receiver, JS_WEAK_SET_TYPE,
+                         "WeakSet.prototype.delete");
+
+  Return(
+      CallBuiltin(Builtins::kWeakCollectionDelete, context, receiver, value));
+}
+
 TF_BUILTIN(WeakSetHas, WeakCollectionsBuiltinsAssembler) {
   Node* const receiver = Parameter(Descriptor::kReceiver);
   Node* const key = Parameter(Descriptor::kKey);

src/builtins/builtins-definitions.h

@@ -1090,12 +1090,15 @@ namespace internal {
   TFJ(WeakMapGet, 1, kKey)                                                     \
   TFJ(WeakMapHas, 1, kKey)                                                     \
   TFJ(WeakMapPrototypeSet, 2, kKey, kValue)                                    \
+  TFJ(WeakMapPrototypeDelete, 1, kKey)                                         \
                                                                                \
   /* WeakSet */                                                                \
   TFJ(WeakSetHas, 1, kKey)                                                     \
   TFJ(WeakSetPrototypeAdd, 1, kValue)                                          \
+  TFJ(WeakSetPrototypeDelete, 1, kValue)                                       \
                                                                                \
   /* WeakSet / WeakMap Helpers */                                              \
+  TFS(WeakCollectionDelete, kCollection, kKey)                                 \
   TFS(WeakCollectionSet, kCollection, kKey, kValue)                            \
                                                                                \
   /* AsyncGenerator */                                                         \

src/js/weak-collection.js

@@ -40,23 +40,6 @@ function WeakMapConstructor(iterable) {
 }
 
 
-// Set up the non-enumerable functions on the WeakMap prototype object.
-DEFINE_METHODS(
-  GlobalWeakMap.prototype,
-  {
-    delete(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);
-    }
-  }
-);
-
 // -------------------------------------------------------------------
 
 %SetCode(GlobalWeakMap, WeakMapConstructor);
@@ -84,23 +67,6 @@ function WeakSetConstructor(iterable) {
 }
 
 
-// Set up the non-enumerable functions on the WeakSet prototype object.
-DEFINE_METHODS(
-  GlobalWeakSet.prototype,
-  {
-    delete(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);

src/runtime/runtime-collections.cc

@@ -116,10 +116,18 @@ RUNTIME_FUNCTION(Runtime_WeakCollectionDelete) {
   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());
+
+#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 shrinking the table is necessary. See
+  // HashTable::Shrink().
+  DCHECK(table->NumberOfElements() - 1 <= (table->Capacity() >> 2) &&
+         table->NumberOfElements() - 1 >= 16);
+#endif
+
   bool was_present = JSWeakCollection::Delete(weak_collection, key, hash);
   return isolate->heap()->ToBoolean(was_present);
 }

test/mjsunit/messages.js

@@ -180,11 +180,21 @@ test(function() {
 }, "Method WeakSet.prototype.add called on incompatible receiver [object Array]",
 TypeError);
 
+test(function() {
+  WeakSet.prototype.delete.call([]);
+}, "Method WeakSet.prototype.delete called on incompatible receiver [object Array]",
+TypeError);
+
 test(function() {
   WeakMap.prototype.set.call([]);
 }, "Method WeakMap.prototype.set called on incompatible receiver [object Array]",
 TypeError);
 
+test(function() {
+  WeakMap.prototype.delete.call([]);
+}, "Method WeakMap.prototype.delete called on incompatible receiver [object Array]",
+TypeError);
+
 // kNonCallableInInstanceOfCheck
 test(function() {
   1 instanceof {};