[js weak refs] Migrate the WeakRef parts into the new API

New API is here: tc39/proposal-weakrefs#55 The WeakCell parts stay in the old API, resulting in temporary code duplication in some parts. Those parts will go away once the WeakCell-related parts are migrated to the new API (but the spec needs some work first). BUG=v8:8179 Change-Id: I81ca824a14d830e3c5fa515d5ad7e5f78c10e19d Reviewed-on: https://chromium-review.googlesource.com/c/1378171 Commit-Queue: Marja Hölttä <[email protected]> Reviewed-by: Ulan Degenbaev <[email protected]> Reviewed-by: Sathya Gunasekaran <[email protected]> Cr-Commit-Position: refs/heads/master@{#58264}
guybedford · Dec 17, 2018 · fef6f8e · fef6f8e
1 parent 9b42c4c
commit fef6f8e
Show file tree

Hide file tree

Showing 27 changed files with 225 additions and 422 deletions.
diff --git a/src/bootstrapper.cc b/src/bootstrapper.cc
@@ -4549,9 +4549,6 @@ void Genesis::InitializeGlobal_harmony_weak_refs() {
     SimpleInstallFunction(isolate(), weak_factory_prototype, "makeCell",
                           Builtins::kWeakFactoryMakeCell, 2, false);
 
-    SimpleInstallFunction(isolate(), weak_factory_prototype, "makeRef",
-                          Builtins::kWeakFactoryMakeRef, 2, false);
-
     SimpleInstallFunction(isolate(), weak_factory_prototype, "cleanupSome",
                           Builtins::kWeakFactoryCleanupSome, 0, false);
   }
@@ -4579,6 +4576,7 @@ void Genesis::InitializeGlobal_harmony_weak_refs() {
     // Create %WeakRefPrototype%
     Handle<Map> weak_ref_map =
         factory->NewMap(JS_WEAK_REF_TYPE, JSWeakRef::kSize);
+    DCHECK(weak_ref_map->IsJSObjectMap());
     native_context()->set_js_weak_ref_map(*weak_ref_map);
 
     Handle<JSObject> weak_ref_prototype =
@@ -4593,6 +4591,23 @@ void Genesis::InitializeGlobal_harmony_weak_refs() {
 
     SimpleInstallFunction(isolate(), weak_ref_prototype, "deref",
                           Builtins::kWeakRefDeref, 0, false);
+
+    // Create %WeakRef%
+    Handle<String> weak_ref_name = factory->InternalizeUtf8String("WeakRef");
+    Handle<JSFunction> weak_ref_fun = CreateFunction(
+        isolate(), weak_ref_name, JS_WEAK_REF_TYPE, JSWeakRef::kSize, 0,
+        weak_ref_prototype, Builtins::kWeakRefConstructor);
+
+    weak_ref_fun->shared()->DontAdaptArguments();
+    weak_ref_fun->shared()->set_length(1);
+
+    // Install the "constructor" property on the prototype.
+    JSObject::AddProperty(isolate(), weak_ref_prototype,
+                          factory->constructor_string(), weak_ref_fun,
+                          DONT_ENUM);
+
+    JSObject::AddProperty(isolate(), global, weak_ref_name, weak_ref_fun,
+                          DONT_ENUM);
   }
 
   {

diff --git a/src/builtins/builtins-definitions.h b/src/builtins/builtins-definitions.h
@@ -1368,7 +1368,7 @@ namespace internal {
   CPP(WeakFactoryCleanupSome)                                                  \
   CPP(WeakFactoryConstructor)                                                  \
   CPP(WeakFactoryMakeCell)                                                     \
-  CPP(WeakFactoryMakeRef)                                                      \
+  CPP(WeakRefConstructor)                                                      \
   CPP(WeakRefDeref)
 
 #ifdef V8_INTL_SUPPORT

diff --git a/src/builtins/builtins-weak-refs.cc b/src/builtins/builtins-weak-refs.cc
@@ -77,44 +77,6 @@ BUILTIN(WeakFactoryMakeCell) {
   return *weak_cell;
 }
 
-BUILTIN(WeakFactoryMakeRef) {
-  HandleScope scope(isolate);
-  const char* method_name = "WeakFactory.prototype.makeRef";
-
-  CHECK_RECEIVER(JSWeakFactory, weak_factory, method_name);
-
-  Handle<Object> target = args.atOrUndefined(isolate, 1);
-  if (!target->IsJSReceiver()) {
-    THROW_NEW_ERROR_RETURN_FAILURE(
-        isolate,
-        NewTypeError(MessageTemplate::kWeakRefsMakeRefTargetMustBeObject));
-  }
-  Handle<JSReceiver> target_receiver = Handle<JSReceiver>::cast(target);
-  Handle<Object> holdings = args.atOrUndefined(isolate, 2);
-  if (target->SameValue(*holdings)) {
-    THROW_NEW_ERROR_RETURN_FAILURE(
-        isolate,
-        NewTypeError(
-            MessageTemplate::kWeakRefsMakeRefTargetAndHoldingsMustNotBeSame));
-  }
-
-  // TODO(marja): Realms.
-
-  Handle<Map> weak_ref_map(isolate->native_context()->js_weak_ref_map(),
-                           isolate);
-
-  Handle<JSWeakRef> weak_ref =
-      Handle<JSWeakRef>::cast(isolate->factory()->NewJSObjectFromMap(
-          weak_ref_map, TENURED, Handle<AllocationSite>::null()));
-  weak_ref->set_target(*target_receiver);
-  weak_ref->set_holdings(*holdings);
-  weak_factory->AddWeakCell(*weak_ref);
-
-  isolate->heap()->AddKeepDuringJobTarget(target_receiver);
-
-  return *weak_ref;
-}
-
 BUILTIN(WeakFactoryCleanupSome) {
   HandleScope scope(isolate);
   const char* method_name = "WeakFactory.prototype.cleanupSome";
@@ -156,6 +118,38 @@ BUILTIN(WeakCellClear) {
   return ReadOnlyRoots(isolate).undefined_value();
 }
 
+BUILTIN(WeakRefConstructor) {
+  HandleScope scope(isolate);
+  Handle<JSFunction> target = args.target();
+  if (args.new_target()->IsUndefined(isolate)) {  // [[Call]]
+    THROW_NEW_ERROR_RETURN_FAILURE(
+        isolate, NewTypeError(MessageTemplate::kConstructorNotFunction,
+                              handle(target->shared()->Name(), isolate)));
+  }
+  // [[Construct]]
+  Handle<JSReceiver> new_target = Handle<JSReceiver>::cast(args.new_target());
+  Handle<Object> target_object = args.atOrUndefined(isolate, 1);
+  if (!target_object->IsJSReceiver()) {
+    THROW_NEW_ERROR_RETURN_FAILURE(
+        isolate,
+        NewTypeError(
+            MessageTemplate::kWeakRefsWeakRefConstructorTargetMustBeObject));
+  }
+  isolate->heap()->AddKeepDuringJobTarget(
+      Handle<JSReceiver>::cast(target_object));
+
+  // TODO(marja): Realms.
+
+  Handle<JSObject> result;
+  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
+      isolate, result,
+      JSObject::New(target, new_target, Handle<AllocationSite>::null()));
+
+  Handle<JSWeakRef> weak_ref = Handle<JSWeakRef>::cast(result);
+  weak_ref->set_target(*target_object);
+  return *weak_ref;
+}
+
 BUILTIN(WeakRefDeref) {
   HandleScope scope(isolate);
   CHECK_RECEIVER(JSWeakRef, weak_ref, "WeakRef.prototype.deref");

diff --git a/src/heap/concurrent-marking.cc b/src/heap/concurrent-marking.cc
@@ -221,6 +221,28 @@ class ConcurrentMarkingVisitor final
     return VisitJSObjectSubclass(map, object);
   }
 
+  int VisitJSWeakRef(Map map, JSWeakRef weak_ref) {
+    int size = VisitJSObjectSubclass(map, weak_ref);
+    if (size == 0) {
+      return 0;
+    }
+    if (weak_ref->target()->IsHeapObject()) {
+      HeapObject* target = HeapObject::cast(weak_ref->target());
+      if (marking_state_.IsBlackOrGrey(target)) {
+        // Record the slot inside the JSWeakRef, since the
+        // VisitJSObjectSubclass above didn't visit it.
+        ObjectSlot slot =
+            HeapObject::RawField(weak_ref, JSWeakRef::kTargetOffset);
+        MarkCompactCollector::RecordSlot(weak_ref, slot, target);
+      } else {
+        // JSWeakRef points to a potentially dead object. We have to process
+        // them when we know the liveness of the whole transitive closure.
+        weak_objects_->js_weak_refs.Push(task_id_, weak_ref);
+      }
+    }
+    return size;
+  }
+
   int VisitJSWeakCell(Map map, JSWeakCell weak_cell) {
     int size = VisitJSObjectSubclass(map, weak_cell);
     if (size == 0) {
@@ -474,7 +496,7 @@ class ConcurrentMarkingVisitor final
 
     void VisitCustomWeakPointers(HeapObject* host, ObjectSlot start,
                                  ObjectSlot end) override {
-      DCHECK(host->IsJSWeakCell());
+      DCHECK(host->IsJSWeakCell() || host->IsJSWeakRef());
     }
 
    private:
@@ -729,6 +751,7 @@ void ConcurrentMarking::Run(int task_id, TaskState* task_state) {
     weak_objects_->next_ephemerons.FlushToGlobal(task_id);
     weak_objects_->discovered_ephemerons.FlushToGlobal(task_id);
     weak_objects_->weak_references.FlushToGlobal(task_id);
+    weak_objects_->js_weak_refs.FlushToGlobal(task_id);
     weak_objects_->js_weak_cells.FlushToGlobal(task_id);
     weak_objects_->weak_objects_in_code.FlushToGlobal(task_id);
     weak_objects_->bytecode_flushing_candidates.FlushToGlobal(task_id);

diff --git a/src/heap/mark-compact-inl.h b/src/heap/mark-compact-inl.h
@@ -195,6 +195,29 @@ int MarkingVisitor<fixed_array_mode, retaining_path_mode,
   return size;
 }
 
+template <FixedArrayVisitationMode fixed_array_mode,
+          TraceRetainingPathMode retaining_path_mode, typename MarkingState>
+int MarkingVisitor<fixed_array_mode, retaining_path_mode,
+                   MarkingState>::VisitJSWeakRef(Map map, JSWeakRef weak_ref) {
+  if (weak_ref->target()->IsHeapObject()) {
+    HeapObject* target = HeapObject::cast(weak_ref->target());
+    if (marking_state()->IsBlackOrGrey(target)) {
+      // Record the slot inside the JSWeakRef, since the IterateBody below
+      // won't visit it.
+      ObjectSlot slot =
+          HeapObject::RawField(weak_ref, JSWeakCell::kTargetOffset);
+      collector_->RecordSlot(weak_ref, slot, target);
+    } else {
+      // JSWeakRef points to a potentially dead object. We have to process
+      // them when we know the liveness of the whole transitive closure.
+      collector_->AddWeakRef(weak_ref);
+    }
+  }
+  int size = JSWeakRef::BodyDescriptor::SizeOf(map, weak_ref);
+  JSWeakRef::BodyDescriptor::IterateBody(map, weak_ref, size, this);
+  return size;
+}
+
 template <FixedArrayVisitationMode fixed_array_mode,
           TraceRetainingPathMode retaining_path_mode, typename MarkingState>
 int MarkingVisitor<fixed_array_mode, retaining_path_mode,

diff --git a/src/heap/mark-compact.cc b/src/heap/mark-compact.cc
@@ -1931,6 +1931,7 @@ void MarkCompactCollector::ClearNonLiveReferences() {
   DCHECK(weak_objects_.transition_arrays.IsEmpty());
   DCHECK(weak_objects_.weak_references.IsEmpty());
   DCHECK(weak_objects_.weak_objects_in_code.IsEmpty());
+  DCHECK(weak_objects_.js_weak_refs.IsEmpty());
   DCHECK(weak_objects_.js_weak_cells.IsEmpty());
   DCHECK(weak_objects_.bytecode_flushing_candidates.IsEmpty());
 }
@@ -2267,6 +2268,20 @@ void MarkCompactCollector::ClearJSWeakCells() {
   if (!FLAG_harmony_weak_refs) {
     return;
   }
+  JSWeakRef weak_ref;
+  while (weak_objects_.js_weak_refs.Pop(kMainThread, &weak_ref)) {
+    // We do not insert cleared weak cells into the list, so the value
+    // cannot be undefined here.
+    JSReceiver target = JSReceiver::cast(weak_ref->target());
+    if (!non_atomic_marking_state()->IsBlackOrGrey(target)) {
+      weak_ref->set_target(ReadOnlyRoots(isolate()).undefined_value());
+    } else {
+      // The value of the JSWeakRef is alive.
+      ObjectSlot slot =
+          HeapObject::RawField(weak_ref, JSWeakRef::kTargetOffset);
+      RecordSlot(weak_ref, slot, target);
+    }
+  }
   JSWeakCell weak_cell;
   while (weak_objects_.js_weak_cells.Pop(kMainThread, &weak_cell)) {
     // We do not insert cleared weak cells into the list, so the value
@@ -2312,6 +2327,7 @@ void MarkCompactCollector::AbortWeakObjects() {
   weak_objects_.discovered_ephemerons.Clear();
   weak_objects_.weak_references.Clear();
   weak_objects_.weak_objects_in_code.Clear();
+  weak_objects_.js_weak_refs.Clear();
   weak_objects_.js_weak_cells.Clear();
   weak_objects_.bytecode_flushing_candidates.Clear();
 }

diff --git a/src/heap/mark-compact.h b/src/heap/mark-compact.h
@@ -444,6 +444,7 @@ struct WeakObjects {
   Worklist<std::pair<HeapObject*, HeapObjectSlot>, 64> weak_references;
   Worklist<std::pair<HeapObject*, Code>, 64> weak_objects_in_code;
 
+  Worklist<JSWeakRef, 64> js_weak_refs;
   Worklist<JSWeakCell, 64> js_weak_cells;
 
   Worklist<SharedFunctionInfo, 64> bytecode_flushing_candidates;
@@ -683,6 +684,10 @@ class MarkCompactCollector final : public MarkCompactCollectorBase {
                                             std::make_pair(object, code));
   }
 
+  void AddWeakRef(JSWeakRef weak_ref) {
+    weak_objects_.js_weak_refs.Push(kMainThread, weak_ref);
+  }
+
   void AddWeakCell(JSWeakCell weak_cell) {
     weak_objects_.js_weak_cells.Push(kMainThread, weak_cell);
   }
@@ -951,6 +956,7 @@ class MarkingVisitor final
   V8_INLINE int VisitSharedFunctionInfo(Map map, SharedFunctionInfo object);
   V8_INLINE int VisitTransitionArray(Map map, TransitionArray object);
   V8_INLINE int VisitJSWeakCell(Map map, JSWeakCell object);
+  V8_INLINE int VisitJSWeakRef(Map map, JSWeakRef object);
 
   // ObjectVisitor implementation.
   V8_INLINE void VisitPointer(HeapObject* host, ObjectSlot p) final {

diff --git a/src/heap/objects-visiting-inl.h b/src/heap/objects-visiting-inl.h
@@ -64,8 +64,6 @@ ResultType HeapVisitor<ResultType, ConcreteVisitor>::Visit(Map map,
       return visitor->VisitFreeSpace(map, FreeSpace::cast(object));
     case kVisitWeakArray:
       return visitor->VisitWeakArray(map, object);
-    case kVisitJSWeakCell:
-      return visitor->VisitJSWeakCell(map, JSWeakCell::cast(object));
     case kVisitorIdCount:
       UNREACHABLE();
   }

diff --git a/src/heap/objects-visiting.h b/src/heap/objects-visiting.h
@@ -26,6 +26,7 @@ class JSDataView;
 class JSRegExp;
 class JSTypedArray;
 class JSWeakCell;
+class JSWeakRef;
 class JSWeakCollection;
 class NativeContext;
 class UncompiledDataWithoutPreParsedScope;
@@ -56,7 +57,9 @@ class WasmInstanceObject;
   V(JSDataView, JSDataView)                                                   \
   V(JSObject, JSObject)                                                       \
   V(JSTypedArray, JSTypedArray)                                               \
+  V(JSWeakCell, JSWeakCell)                                                   \
   V(JSWeakCollection, JSWeakCollection)                                       \
+  V(JSWeakRef, JSWeakRef)                                                     \
   V(Map, Map)                                                                 \
   V(NativeContext, NativeContext)                                             \
   V(Oddball, Oddball*)                                                        \

diff --git a/src/message-template.h b/src/message-template.h
@@ -542,8 +542,8 @@ namespace internal {
     "WeakFactory.prototype.makeCell: target must be an object")                \
   T(WeakRefsMakeCellTargetAndHoldingsMustNotBeSame,                            \
     "WeakFactory.prototype.makeCell: target and holdings must not be same")    \
-  T(WeakRefsMakeRefTargetMustBeObject,                                         \
-    "WeakFactory.prototype.makeRef: target must be an object")                 \
+  T(WeakRefsWeakRefConstructorTargetMustBeObject,                              \
+    "WeakRef: target must be an object")                                       \
   T(WeakRefsMakeRefTargetAndHoldingsMustNotBeSame,                             \
     "WeakFactory.prototype.makeRef: target and holdings must not be same")
 

diff --git a/src/objects-body-descriptors-inl.h b/src/objects-body-descriptors-inl.h
@@ -237,6 +237,25 @@ class JSWeakCell::BodyDescriptor final : public BodyDescriptorBase {
   }
 };
 
+class JSWeakRef::BodyDescriptor final : public BodyDescriptorBase {
+ public:
+  static bool IsValidSlot(Map map, HeapObject* obj, int offset) {
+    return JSObject::BodyDescriptor::IsValidSlot(map, obj, offset);
+  }
+
+  template <typename ObjectVisitor>
+  static inline void IterateBody(Map map, HeapObject* obj, int object_size,
+                                 ObjectVisitor* v) {
+    IteratePointers(obj, JSReceiver::kPropertiesOrHashOffset, kTargetOffset, v);
+    IterateCustomWeakPointer(obj, kTargetOffset, v);
+    IteratePointers(obj, kTargetOffset + kPointerSize, object_size, v);
+  }
+
+  static inline int SizeOf(Map map, HeapObject* object) {
+    return map->instance_size();
+  }
+};
+
 class SharedFunctionInfo::BodyDescriptor final : public BodyDescriptorBase {
  public:
   static bool IsValidSlot(Map map, HeapObject* obj, int offset) {
@@ -952,8 +971,9 @@ ReturnType BodyDescriptorApply(InstanceType type, T1 p1, T2 p2, T3 p3, T4 p4) {
     case JS_FUNCTION_TYPE:
       return Op::template apply<JSFunction::BodyDescriptor>(p1, p2, p3, p4);
     case JS_WEAK_CELL_TYPE:
-    case JS_WEAK_REF_TYPE:
       return Op::template apply<JSWeakCell::BodyDescriptor>(p1, p2, p3, p4);
+    case JS_WEAK_REF_TYPE:
+      return Op::template apply<JSWeakRef::BodyDescriptor>(p1, p2, p3, p4);
     case ODDBALL_TYPE:
       return Op::template apply<Oddball::BodyDescriptor>(p1, p2, p3, p4);
     case JS_PROXY_TYPE:

diff --git a/src/objects-debug.cc b/src/objects-debug.cc
@@ -331,9 +331,11 @@ void HeapObject::HeapObjectVerify(Isolate* isolate) {
           isolate);
       break;
     case JS_WEAK_CELL_TYPE:
-    case JS_WEAK_REF_TYPE:
       JSWeakCell::cast(this)->JSWeakCellVerify(isolate);
       break;
+    case JS_WEAK_REF_TYPE:
+      JSWeakRef::cast(this)->JSWeakRefVerify(isolate);
+      break;
     case JS_WEAK_FACTORY_TYPE:
       JSWeakFactory::cast(this)->JSWeakFactoryVerify(isolate);
       break;
@@ -1315,6 +1317,12 @@ void JSWeakCell::JSWeakCellVerify(Isolate* isolate) {
   CHECK(factory()->IsUndefined(isolate) || factory()->IsJSWeakFactory());
 }
 
+void JSWeakRef::JSWeakRefVerify(Isolate* isolate) {
+  CHECK(IsJSWeakRef());
+  JSObjectVerify(isolate);
+  CHECK(target()->IsUndefined(isolate) || target()->IsJSReceiver());
+}
+
 void JSWeakFactory::JSWeakFactoryVerify(Isolate* isolate) {
   CHECK(IsJSWeakFactory());
   JSObjectVerify(isolate);