[js weak refs] Add WeakFactory.prototype.cleanupSome

BUG=v8:8179

Change-Id: I7dc024fe4880a787cadac8b79bca6da87e2d36de
Reviewed-on: https://chromium-review.googlesource.com/c/1328926
Commit-Queue: Marja Hölttä <marja@chromium.org>
Reviewed-by: Sathya Gunasekaran <gsathya@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57476}

src/bootstrapper.cc

@@ -4582,6 +4582,9 @@ void Genesis::InitializeGlobal_harmony_weak_refs() {
 
     SimpleInstallFunction(isolate(), weak_factory_prototype, "makeRef",
                           Builtins::kWeakFactoryMakeRef, 2, false);
+
+    SimpleInstallFunction(isolate(), weak_factory_prototype, "cleanupSome",
+                          Builtins::kWeakFactoryCleanupSome, 0, false);
   }
   {
     // Create %WeakCellPrototype%

src/builtins/builtins-definitions.h

@@ -1334,6 +1334,7 @@ namespace internal {
   CPP(WeakCellClear)                                                           \
   CPP(WeakCellHoldingsGetter)                                                  \
   CPP(WeakFactoryCleanupIteratorNext)                                          \
+  CPP(WeakFactoryCleanupSome)                                                  \
   CPP(WeakFactoryConstructor)                                                  \
   CPP(WeakFactoryMakeCell)                                                     \
   CPP(WeakFactoryMakeRef)                                                      \

src/builtins/builtins-weak-refs.cc

@@ -115,6 +115,19 @@ BUILTIN(WeakFactoryMakeRef) {
   return *weak_ref;
 }
 
+BUILTIN(WeakFactoryCleanupSome) {
+  HandleScope scope(isolate);
+  const char* method_name = "WeakFactory.prototype.cleanupSome";
+
+  CHECK_RECEIVER(JSWeakFactory, weak_factory, method_name);
+
+  // Don't do set_scheduled_for_cleanup(false); we still have the microtask
+  // scheduled and don't want to schedule another one in case the user never
+  // executes microtasks.
+  JSWeakFactory::Cleanup(weak_factory, isolate);
+  return ReadOnlyRoots(isolate).undefined_value();
+}
+
 BUILTIN(WeakFactoryCleanupIteratorNext) {
   HandleScope scope(isolate);
   CHECK_RECEIVER(JSWeakFactoryCleanupIterator, iterator, "next");

src/objects.cc

@@ -18986,5 +18986,44 @@ template void
 BaseNameDictionary<NameDictionary, NameDictionaryShape>::CollectKeysTo(
     Handle<NameDictionary> dictionary, KeyAccumulator* keys);
 
+void JSWeakFactory::Cleanup(Handle<JSWeakFactory> weak_factory,
+                            Isolate* isolate) {
+  // It's possible that the cleared_cells list is empty, since
+  // WeakCell.clear() was called on all its elements before this task ran. In
+  // that case, don't call the cleanup function.
+  if (!weak_factory->cleared_cells()->IsUndefined(isolate)) {
+    // Construct the iterator.
+    Handle<JSWeakFactoryCleanupIterator> iterator;
+    {
+      Handle<Map> cleanup_iterator_map(
+          isolate->native_context()->js_weak_factory_cleanup_iterator_map(),
+          isolate);
+      iterator = Handle<JSWeakFactoryCleanupIterator>::cast(
+          isolate->factory()->NewJSObjectFromMap(
+              cleanup_iterator_map, NOT_TENURED,
+              Handle<AllocationSite>::null()));
+      iterator->set_factory(*weak_factory);
+    }
+    Handle<Object> cleanup(weak_factory->cleanup(), isolate);
+
+    v8::TryCatch try_catch(reinterpret_cast<v8::Isolate*>(isolate));
+    v8::Local<v8::Value> result;
+    MaybeHandle<Object> exception;
+    Handle<Object> args[] = {iterator};
+    bool has_pending_exception = !ToLocal<Value>(
+        Execution::TryCall(
+            isolate, cleanup,
+            handle(ReadOnlyRoots(isolate).undefined_value(), isolate), 1, args,
+            Execution::MessageHandling::kReport, &exception,
+            Execution::Target::kCallable),
+        &result);
+    // TODO(marja): (spec): What if there's an exception?
+    USE(has_pending_exception);
+
+    // TODO(marja): (spec): Should the iterator be invalidated after the
+    // function returns?
+  }
+}
+
 }  // namespace internal
 }  // namespace v8

src/objects/js-weak-refs.h

@@ -47,6 +47,10 @@ class JSWeakFactory : public JSObject {
   // list. (Assumes there is one.)
   inline JSWeakCell* PopClearedCell(Isolate* isolate);
 
+  // Constructs an iterator for the WeakCells in the cleared_cells list and
+  // calls the user's cleanup function.
+  static void Cleanup(Handle<JSWeakFactory> weak_factory, Isolate* isolate);
+
   static const int kNativeContextOffset = JSObject::kHeaderSize;
   static const int kCleanupOffset = kNativeContextOffset + kPointerSize;
   static const int kActiveCellsOffset = kCleanupOffset + kPointerSize;

src/runtime/runtime-weak-refs.cc

@@ -20,41 +20,7 @@ RUNTIME_FUNCTION(Runtime_WeakFactoryCleanupJob) {
   CONVERT_ARG_HANDLE_CHECKED(JSWeakFactory, weak_factory, 0);
   weak_factory->set_scheduled_for_cleanup(false);
 
-  // It's possible that the cleared_cells list is empty, since
-  // WeakCell.clear() was called on all its elements before this task ran. In
-  // that case, don't call the cleanup function.
-  if (!weak_factory->cleared_cells()->IsUndefined(isolate)) {
-    // Construct the iterator.
-    Handle<JSWeakFactoryCleanupIterator> iterator;
-    {
-      Handle<Map> cleanup_iterator_map(
-          isolate->native_context()->js_weak_factory_cleanup_iterator_map(),
-          isolate);
-      iterator = Handle<JSWeakFactoryCleanupIterator>::cast(
-          isolate->factory()->NewJSObjectFromMap(
-              cleanup_iterator_map, NOT_TENURED,
-              Handle<AllocationSite>::null()));
-      iterator->set_factory(*weak_factory);
-    }
-    Handle<Object> cleanup(weak_factory->cleanup(), isolate);
-
-    v8::TryCatch try_catch(reinterpret_cast<v8::Isolate*>(isolate));
-    v8::Local<v8::Value> result;
-    MaybeHandle<Object> exception;
-    Handle<Object> args[] = {iterator};
-    bool has_pending_exception = !ToLocal<Value>(
-        Execution::TryCall(
-            isolate, cleanup,
-            handle(ReadOnlyRoots(isolate).undefined_value(), isolate), 1, args,
-            Execution::MessageHandling::kReport, &exception,
-            Execution::Target::kCallable),
-        &result);
-    // TODO(marja): (spec): What if there's an exception?
-    USE(has_pending_exception);
-
-    // TODO(marja): (spec): Should the iterator be invalidated after the
-    // function returns?
-  }
+  JSWeakFactory::Cleanup(weak_factory, isolate);
   return ReadOnlyRoots(isolate).undefined_value();
 }
 

test/mjsunit/harmony/weakrefs/basics.js

@@ -232,6 +232,14 @@
   WeakFactory.prototype.makeRef.call(wf, {});
 })();
 
+(function TestCleanupSomeWithoutWeakFactory() {
+  assertThrows(() => WeakFactory.prototype.cleanupSome.call({}), TypeError);
+  // Does not throw:
+  let wf = new WeakFactory(() => {});
+  let rv = WeakFactory.prototype.cleanupSome.call(wf);
+  assertEquals(undefined, rv);
+})();
+
 (function TestDerefWithoutWeakRef() {
   let wf = new WeakFactory(() => {});
   let wc = wf.makeCell({});

test/mjsunit/harmony/weakrefs/cleanupsome-cleared-weakcell.js

+// Copyright 2018 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.
+
+// Flags: --harmony-weak-refs --expose-gc --noincremental-marking
+
+let cleanup_count = 0;
+let cleanup_cells = [];
+let cleanup = function(iter) {
+  for (wc of iter) {
+    cleanup_cells.push(wc);
+  }
+  ++cleanup_count;
+}
+
+let wf = new WeakFactory(cleanup);
+let weak_cell;
+(function() {
+  let o = {};
+  weak_cell = wf.makeCell(o);
+
+  // cleanupSome won't do anything since there are no dirty WeakCells.
+  wf.cleanupSome();
+  assertEquals(0, cleanup_count);
+})();
+
+// GC will detect the WeakCell as dirty.
+gc();
+
+// Clear the WeakCell just before we would've called cleanupSome.
+weak_cell.clear();
+
+wf.cleanupSome();
+
+assertEquals(0, cleanup_count);

test/mjsunit/harmony/weakrefs/cleanupsome-dereffed-and-cleared-weakref.js

+// Copyright 2018 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.
+
+// Flags: --harmony-weak-refs --expose-gc --noincremental-marking --allow-natives-syntax
+
+let cleanup_count = 0;
+let cleanup_cells = [];
+let cleanup = function(iter) {
+  for (wc of iter) {
+    cleanup_cells.push(wc);
+  }
+  ++cleanup_count;
+}
+
+let o = {};
+let wf = new WeakFactory(cleanup);
+let weak_ref;
+(function() {
+  weak_ref = wf.makeRef(o);
+
+  // cleanupSome won't do anything since there are no dirty WeakCells.
+  wf.cleanupSome();
+  assertEquals(0, cleanup_count);
+})();
+
+// Clear the KeepDuringJob set.
+%RunMicrotasks();
+
+weak_ref.deref();
+o = null;
+
+// The WeakRef is not detected as dirty, since the KeepDuringJob set keeps the
+// target object alive.
+gc();
+
+wf.cleanupSome();
+assertEquals(0, cleanup_count);
+
+%RunMicrotasks();
+// Next turn.
+
+// This GC detects the WeakRef as dirty.
+gc();
+
+// Clear the WeakRef just before we would've called cleanupSome.
+weak_ref.clear();
+
+wf.cleanupSome();
+
+assertEquals(0, cleanup_count);

test/mjsunit/harmony/weakrefs/cleanupsome-dereffed-weakref.js

+// Copyright 2018 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.
+
+// Flags: --harmony-weak-refs --expose-gc --noincremental-marking --allow-natives-syntax
+
+let cleanup_count = 0;
+let cleanup_cells = [];
+let cleanup = function(iter) {
+  for (wc of iter) {
+    cleanup_cells.push(wc);
+  }
+  ++cleanup_count;
+}
+
+let o = {};
+let wf = new WeakFactory(cleanup);
+let weak_ref;
+(function() {
+  weak_ref = wf.makeRef(o);
+
+  // cleanupSome won't do anything since there are no dirty WeakCells.
+  wf.cleanupSome();
+  assertEquals(0, cleanup_count);
+})();
+
+// Clear the KeepDuringJob set.
+%RunMicrotasks();
+
+weak_ref.deref();
+o = null;
+
+// The WeakRef is not detected as dirty, since the KeepDuringJob set keeps the
+// target object alive.
+gc();
+
+wf.cleanupSome();
+assertEquals(0, cleanup_count);
+
+%RunMicrotasks();
+// Next turn.
+
+// Now the WeakRef can be cleared.
+gc();
+wf.cleanupSome();
+
+assertEquals(1, cleanup_count);
+assertEquals(1, cleanup_cells.length);
+assertEquals(weak_ref, cleanup_cells[0]);
+
+// The cleanup task is not executed again since all WeakCells have been
+// processed.
+
+%RunMicrotasks();
+// Next turn.
+
+assertEquals(1, cleanup_count);

test/mjsunit/harmony/weakrefs/cleanupsome-weakcell.js

+// Copyright 2018 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.
+
+// Flags: --harmony-weak-refs --expose-gc --noincremental-marking
+
+let cleanup_count = 0;
+let cleanup_cells = [];
+let cleanup = function(iter) {
+  for (wc of iter) {
+    cleanup_cells.push(wc);
+  }
+  ++cleanup_count;
+}
+
+let wf = new WeakFactory(cleanup);
+let weak_cell;
+(function() {
+  let o = {};
+  weak_cell = wf.makeCell(o);
+
+  // cleanupSome won't do anything since there are no dirty WeakCells.
+  wf.cleanupSome();
+  assertEquals(0, cleanup_count);
+})();
+
+// GC will detect the WeakCell as dirty.
+gc();
+
+wf.cleanupSome();
+assertEquals(1, cleanup_count);
+assertEquals(1, cleanup_cells.length);
+assertEquals(weak_cell, cleanup_cells[0]);

test/mjsunit/harmony/weakrefs/cleanupsome-weakref.js

+// Copyright 2018 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.
+
+// Flags: --harmony-weak-refs --expose-gc --noincremental-marking --allow-natives-syntax
+
+let cleanup_count = 0;
+let cleanup_cells = [];
+let cleanup = function(iter) {
+  for (wc of iter) {
+    cleanup_cells.push(wc);
+  }
+  ++cleanup_count;
+}
+
+let wf = new WeakFactory(cleanup);
+let weak_ref;
+(function() {
+  let o = {};
+  weak_ref = wf.makeRef(o);
+
+  // cleanupSome won't do anything since there are no dirty WeakCells.
+  wf.cleanupSome();
+  assertEquals(0, cleanup_count);
+})();
+
+// The WeakRef is not detected as dirty, since the KeepDuringJob set keeps the
+// target object alive.
+gc();
+
+wf.cleanupSome();
+assertEquals(0, cleanup_count);
+
+%RunMicrotasks();
+// Next turn.
+
+// Now the WeakRef can be cleared.
+gc();
+wf.cleanupSome();
+
+assertEquals(1, cleanup_count);
+assertEquals(1, cleanup_cells.length);
+assertEquals(weak_ref, cleanup_cells[0]);