Always mark newly created WeakRefSlots

Summary:
Instead of initialising all slots to be unmarked and then conditionally
marking them, we can simply initialise all slots as marked. This makes
the code simpler and more efficient.

Reviewed By: jpporto

Differential Revision: D38096876

fbshipit-source-id: f283839f7b1b24ab03bbfcba1fbe339391653007

include/hermes/VM/WeakRefSlot.h

@@ -87,7 +87,7 @@ class WeakRefSlot {
   }
 
   void mark() {
-    assert(state() == Unmarked && "already marked");
+    assert(state() != Free && "Cannot mark a free slot.");
     state_ = Marked;
   }
 
@@ -112,10 +112,8 @@ class WeakRefSlot {
 
   /// Re-initialize a freed slot.
   void reset(CompressedPointer ptr) {
-    static_assert(Unmarked == 0, "unmarked state should not need tagging");
-    state_ = Unmarked;
+    state_ = Marked;
     value_.root = ptr;
-    assert(state() == Unmarked && "initial state should be unmarked");
   }
 
  private:

lib/VM/gcs/HadesGC.cpp

@@ -800,9 +800,7 @@ class HadesGC::MarkAcceptor final : public RootAndSlotAcceptor,
     assert(
         slot->state() != WeakSlotState::Free &&
         "marking a freed weak ref slot");
-    if (slot->state() != WeakSlotState::Marked) {
-      slot->mark();
-    }
+    slot->mark();
   }
 
   /// Set the drain rate that'll be used for any future calls to drain APIs.
@@ -2126,26 +2124,14 @@ WeakRefSlot *HadesGC::allocWeakSlot(CompressedPointer ptr) {
       "allocWeakSlot should only be called from the mutator");
   // The weak ref mutex doesn't need to be held since weakSlots_ and
   // firstFreeWeak_ are only modified while the world is stopped.
-  WeakRefSlot *slot;
-  if (firstFreeWeak_) {
-    assert(
-        firstFreeWeak_->state() == WeakSlotState::Free &&
-        "invalid free slot state");
-    slot = firstFreeWeak_;
+  if (auto *slot = firstFreeWeak_) {
+    assert(slot->state() == WeakSlotState::Free && "invalid free slot state");
     firstFreeWeak_ = firstFreeWeak_->nextFree();
     slot->reset(ptr);
-  } else {
-    weakSlots_.push_back({ptr});
-    slot = &weakSlots_.back();
-  }
-  if (ogMarkingBarriers_) {
-    // During the mark phase, if a WeakRef is created, it might not be marked
-    // if the object holding this new WeakRef has already been visited.
-    // This doesn't need the WeakRefMutex because nothing is using this slot
-    // yet.
-    slot->mark();
+    return slot;
   }
-  return slot;
+  weakSlots_.push_back({ptr});
+  return &weakSlots_.back();
 }
 
 void HadesGC::freeWeakSlot(WeakRefSlot *slot) {

unittests/VMRuntime/GCBasicsTest.cpp

@@ -170,6 +170,7 @@ TEST_F(GCBasicsTest, WeakRefSlotTest) {
       CompressedPointer::encode(static_cast<GCCell *>(obj), rt);
 
   WeakRefSlot s(ptr);
+  s.unmark();
   EXPECT_EQ(WeakSlotState::Unmarked, s.state());
   EXPECT_TRUE(s.hasValue());
   EXPECT_EQ(ptr, s.getNoBarrierUnsafe());