Skip to content

Commit

Permalink
Remove hasPointer
Browse files Browse the repository at this point in the history
Summary: `hasPointer` and `hasValue` essentially do the same thing, since hasPointer is never called when the slot is freed. Therefore we can just keep `hasValue`.

Reviewed By: neildhar

Differential Revision: D35722782

fbshipit-source-id: 77a693687adb64a6e1e546e8c282b50f8dba958c
  • Loading branch information
Michael Anthony Leon authored and facebook-github-bot committed Apr 25, 2022
1 parent 586a279 commit 75513ab
Show file tree
Hide file tree
Showing 6 changed files with 11 additions and 17 deletions.
6 changes: 0 additions & 6 deletions include/hermes/VM/WeakRefSlot.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,6 @@ class WeakRefSlot {

// GC methods to update slot when referent moves/dies.

/// Return true if this slot stores a non-null pointer to something. For any
/// slot reachable by the mutator, that something is a GCCell.
bool hasPointer() const {
return value_.isPointer();
}

/// Return the pointer to a GCCell, whether or not this slot is marked.
inline GCCell *getPointer(PointerBase &base) const;

Expand Down
2 changes: 1 addition & 1 deletion lib/VM/CheckHeapWellFormedAcceptor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ void CheckHeapWellFormedAcceptor::accept(WeakRefBase &wr) {
// empty weak ref.
const WeakRefSlot *slot = wr.unsafeGetSlot();
// If the weak value is a pointer, check that it's within the valid region.
if (slot->state() != WeakSlotState::Free && slot->hasPointer()) {
if (slot->state() != WeakSlotState::Free && slot->hasValue()) {
GCCell *cell = slot->getPointer(gc.getPointerBase());
accept(cell);
}
Expand Down
4 changes: 2 additions & 2 deletions lib/VM/GCBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ struct EdgeAddingAcceptor : public SnapshotAcceptor, public WeakRefAcceptor {
// If the slot is free, there's no edge to add.
return;
}
if (!slot->hasPointer()) {
if (!slot->hasValue()) {
// Filter out empty refs from adding edges.
return;
}
Expand Down Expand Up @@ -373,7 +373,7 @@ struct SnapshotRootAcceptor : public SnapshotAcceptor,
// If the slot is free, there's no edge to add.
return;
}
if (!slot->hasPointer()) {
if (!slot->hasValue()) {
// Filter out empty refs from adding edges.
return;
}
Expand Down
4 changes: 2 additions & 2 deletions lib/VM/gcs/HadesGC.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2824,7 +2824,7 @@ void HadesGC::updateWeakReferencesForYoungGen() {
LLVM_FALLTHROUGH;
case WeakSlotState::Unmarked: {
// Both marked and unmarked weak ref slots need to be updated.
if (!slot.hasPointer()) {
if (!slot.hasValue()) {
// Non-pointers need no work.
break;
}
Expand Down Expand Up @@ -2863,7 +2863,7 @@ void HadesGC::updateWeakReferencesForOldGen() {
case WeakSlotState::Marked: {
// Set all allocated slots to unmarked.
slot.unmark();
if (!slot.hasPointer()) {
if (!slot.hasValue()) {
// Skip non-pointers.
break;
}
Expand Down
2 changes: 1 addition & 1 deletion lib/VM/gcs/MallocGC.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -527,7 +527,7 @@ void MallocGC::updateWeakReferences() {
break;
case WeakSlotState::Marked:
// If it's not a pointer, nothing to do.
if (!slot.hasPointer()) {
if (!slot.hasValue()) {
break;
}
auto *cell = slot.getPointer(getPointerBase());
Expand Down
10 changes: 5 additions & 5 deletions unittests/VMRuntime/GCBasicsTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -169,30 +169,30 @@ TEST_F(GCBasicsTest, WeakRefSlotTest) {

WeakRefSlot s(hv);
EXPECT_EQ(WeakSlotState::Unmarked, s.state());
EXPECT_TRUE(s.hasPointer());
EXPECT_EQ(hv, s.value());
EXPECT_TRUE(s.hasValue());
EXPECT_EQ(obj, s.getPointer(rt));

// Update pointer of unmarked slot.
auto obj2 = (void *)0x76543210;
s.setPointer(obj2);
EXPECT_EQ(WeakSlotState::Unmarked, s.state());
EXPECT_TRUE(s.hasPointer());
EXPECT_TRUE(s.hasValue());
EXPECT_EQ(obj2, s.getPointer(rt));

// Marked slot.
s.mark();
EXPECT_EQ(WeakSlotState::Marked, s.state());
EXPECT_TRUE(s.hasPointer());
EXPECT_TRUE(s.hasValue());
EXPECT_EQ(obj2, s.getPointer(rt));
s.setPointer(obj);
EXPECT_EQ(WeakSlotState::Marked, s.state());
EXPECT_TRUE(s.hasPointer());
EXPECT_TRUE(s.hasValue());
EXPECT_EQ(obj, s.getPointer(rt));

s.clearPointer();
EXPECT_EQ(WeakSlotState::Marked, s.state());
EXPECT_FALSE(s.hasPointer());
EXPECT_FALSE(s.hasValue());

// Unmark and free.
s.unmark();
Expand Down

0 comments on commit 75513ab

Please sign in to comment.