Skip to content

Commit

Permalink
Store segment size in SHSegmentInfo
Browse files Browse the repository at this point in the history
Summary:
We need the segment size in several places, such as CardTable, heap
segment itself and getting sizes for large object GCCell. Add this size
into `SHSegmentInfo`.

In addition, in CardTableNCTest, when search dirty bits, we should
start from kFirstUsedIndex instead of 0, since the value of
`SHSegmentInfo` may write some bytes to non-zero.

Differential Revision: D61807366
  • Loading branch information
lavenzg authored and facebook-github-bot committed Sep 6, 2024
1 parent 3171dbe commit 90bc2f2
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 40 deletions.
13 changes: 8 additions & 5 deletions include/hermes/VM/AlignedHeapSegment.h
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,14 @@ class AlignedHeapSegmentBase {
return lowLim_;
}

/// Returns the address that is the upper bound of the segment.
/// This is only used in debugging code and computing memory footprint, so
/// just read the segment size from SHSegmentInfo.
char *hiLim() const {
auto *segmentInfo = reinterpret_cast<const SHSegmentInfo *>(lowLim_);
return lowLim_ + segmentInfo->segmentSize;
}

/// Returns the address at which the first allocation in this segment would
/// occur.
/// Disable UB sanitization because 'this' may be null during the tests.
Expand Down Expand Up @@ -380,11 +388,6 @@ class AlignedHeapSegment : public AlignedHeapSegmentBase {
/// The number of bytes in the segment that are available for allocation.
inline size_t available() const;

/// Returns the address that is the upper bound of the segment.
char *hiLim() const {
return lowLim() + storageSize();
}

/// Returns the first address after the region in which allocations can occur,
/// taking external memory credits into a account (they decrease the effective
/// end).
Expand Down
43 changes: 28 additions & 15 deletions include/hermes/VM/CardTableNC.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,9 @@ class CardTable {
static constexpr size_t kCardSize = 1 << kLogCardSize; // ==> 512-byte cards.
static constexpr size_t kSegmentSize = 1 << HERMESVM_LOG_HEAP_SEGMENT_SIZE;

/// The number of valid indices into the card table.
static constexpr size_t kValidIndices = kSegmentSize >> kLogCardSize;

/// The size of the card table.
static constexpr size_t kCardTableSize = kValidIndices;
/// The size of the maximum inline card table. CardStatus array and boundary
/// array for larger segment has larger size and is storage separately.
static constexpr size_t kInlineCardTableSize = kSegmentSize >> kLogCardSize;

/// For convenience, this is a conversion factor to determine how many bytes
/// in the heap correspond to a single byte in the card table. This is
Expand All @@ -91,10 +89,14 @@ class CardTable {
/// Note that the total size of the card table is 2 times kCardTableSize,
/// since the CardTable contains two byte arrays of that size (cards_ and
/// boundaries_).
static constexpr size_t kFirstUsedIndex =
std::max(sizeof(SHSegmentInfo), (2 * kCardTableSize) >> kLogCardSize);
static constexpr size_t kFirstUsedIndex = std::max(
sizeof(SHSegmentInfo),
(2 * kInlineCardTableSize) >> kLogCardSize);

CardTable() = default;
CardTable() {
// Preserve the segment size.
segmentInfo_.segmentSize = kSegmentSize;
}
/// CardTable is not copyable or movable: It must be constructed in-place.
CardTable(const CardTable &) = delete;
CardTable(CardTable &&) = delete;
Expand Down Expand Up @@ -185,6 +187,11 @@ class CardTable {
/// is the first object.)
GCCell *firstObjForCard(unsigned index) const;

/// The end index of the card table (all valid indices should be smaller).
size_t getEndIndex() const {
return getSegmentSize() >> kLogCardSize;
}

#ifdef HERMES_EXTRA_DEBUG
/// Temporary debugging hack: yield the numeric value of the boundaries_ array
/// for the given \p index.
Expand Down Expand Up @@ -215,10 +222,16 @@ class CardTable {
#endif // HERMES_SLOW_DEBUG

private:
unsigned getSegmentSize() const {
return segmentInfo_.segmentSize;
}

#ifndef NDEBUG
/// Returns the pointer to the end of the storage containing \p ptr
/// (exclusive).
static void *storageEnd(const void *ptr);
/// Returns the pointer to the end of the storage starting at \p lowLim.
void *storageEnd(const void *lowLim) const {
return reinterpret_cast<char *>(
reinterpret_cast<uintptr_t>(lowLim) + getSegmentSize());
}
#endif

enum class CardStatus : char { Clean = 0, Dirty = 1 };
Expand Down Expand Up @@ -262,7 +275,7 @@ class CardTable {
SHSegmentInfo segmentInfo_;
/// This needs to be atomic so that the background thread in Hades can
/// safely dirty cards when compacting.
std::array<AtomicIfConcurrentGC<CardStatus>, kCardTableSize> cards_{};
std::array<AtomicIfConcurrentGC<CardStatus>, kInlineCardTableSize> cards_{};
};

/// See the comment at kHeapBytesPerCardByte above to see why this is
Expand All @@ -281,7 +294,7 @@ class CardTable {
/// time: If we allocate a large object that crosses many cards, the first
/// crossed cards gets a non-negative value, and each subsequent one uses the
/// maximum exponent that stays within the card range for the object.
int8_t boundaries_[kCardTableSize];
int8_t boundaries_[kInlineCardTableSize];
};

/// Implementations of inlines.
Expand Down Expand Up @@ -311,7 +324,7 @@ inline size_t CardTable::addressToIndex(const void *addr) const {
}

inline const char *CardTable::indexToAddress(size_t index) const {
assert(index <= kValidIndices && "index must be within the index range");
assert(index <= getEndIndex() && "index must be within the index range");
const char *res = base() + (index << kLogCardSize);
assert(
base() <= res && res <= storageEnd(base()) &&
Expand All @@ -329,7 +342,7 @@ inline bool CardTable::isCardForAddressDirty(const void *addr) const {
}

inline bool CardTable::isCardForIndexDirty(size_t index) const {
assert(index < kValidIndices && "index is required to be in range.");
assert(index < getEndIndex() && "index is required to be in range.");
return cards_[index].load(std::memory_order_relaxed) == CardStatus::Dirty;
}

Expand Down
3 changes: 3 additions & 0 deletions include/hermes/VM/sh_segment_info.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@
/// contain segment-specific information.
typedef struct SHSegmentInfo {
unsigned index;
/// The storage size for this segment. We practically don't support segment
/// with size larger than UINT32_MAX.
unsigned segmentSize;
} SHSegmentInfo;

#endif
18 changes: 6 additions & 12 deletions lib/VM/gcs/CardTableNC.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,6 @@
namespace hermes {
namespace vm {

#ifndef NDEBUG
/* static */ void *CardTable::storageEnd(const void *ptr) {
return AlignedHeapSegment::storageEnd(ptr);
}
#endif

void CardTable::dirtyCardsForAddressRange(const void *low, const void *high) {
// If high is in the middle of some card, ensure that we dirty that card.
high = reinterpret_cast<const char *>(high) + kCardSize - 1;
Expand All @@ -44,19 +38,19 @@ OptValue<size_t> CardTable::findNextCardWithStatus(
}

void CardTable::clear() {
cleanRange(kFirstUsedIndex, kValidIndices);
cleanRange(kFirstUsedIndex, getEndIndex());
}

void CardTable::updateAfterCompaction(const void *newLevel) {
const char *newLevelPtr = static_cast<const char *>(newLevel);
size_t firstCleanCardIndex = addressToIndex(newLevelPtr + kCardSize - 1);
assert(
firstCleanCardIndex <= kValidIndices &&
firstCleanCardIndex <= getEndIndex() &&
firstCleanCardIndex >= kFirstUsedIndex && "Invalid index.");
// Dirty the occupied cards (below the level), and clean the cards above the
// level.
dirtyRange(kFirstUsedIndex, firstCleanCardIndex);
cleanRange(firstCleanCardIndex, kValidIndices);
cleanRange(firstCleanCardIndex, getEndIndex());
}

void CardTable::cleanRange(size_t from, size_t to) {
Expand Down Expand Up @@ -147,20 +141,20 @@ protectBoundaryTableWork(void *table, size_t sz, oscompat::ProtectMode mode) {

void CardTable::protectBoundaryTable() {
protectBoundaryTableWork(
&boundaries_[0], kValidIndices, oscompat::ProtectMode::None);
&boundaries_[0], getEndIndex(), oscompat::ProtectMode::None);
}

void CardTable::unprotectBoundaryTable() {
protectBoundaryTableWork(
&boundaries_[0], kValidIndices, oscompat::ProtectMode::ReadWrite);
&boundaries_[0], getEndIndex(), oscompat::ProtectMode::ReadWrite);
}
#endif // HERMES_EXTRA_DEBUG

#ifdef HERMES_SLOW_DEBUG
void CardTable::verifyBoundaries(char *start, char *level) const {
// Start should be card-aligned.
assert(isCardAligned(start));
for (unsigned index = addressToIndex(start); index < kValidIndices; index++) {
for (unsigned index = addressToIndex(start); index < getEndIndex(); index++) {
const char *boundary = indexToAddress(index);
if (level <= boundary) {
break;
Expand Down
18 changes: 10 additions & 8 deletions unittests/VMRuntime/CardTableNCTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ CardTableNCTest::CardTableNCTest() {
TEST_F(CardTableNCTest, AddressToIndex) {
// Expected indices in the card table corresponding to the probe
// addresses into the storage.
const size_t lastIx = CardTable::kValidIndices - 1;
const size_t lastIx = table->getEndIndex() - 1;
std::vector<size_t> indices{
CardTable::kFirstUsedIndex,
CardTable::kFirstUsedIndex + 1,
Expand All @@ -105,13 +105,13 @@ TEST_F(CardTableNCTest, AddressToIndexBoundary) {
// the storage.
ASSERT_EQ(seg.lowLim(), reinterpret_cast<char *>(table));

const size_t hiLim = CardTable::kValidIndices;
const size_t hiLim = table->getEndIndex();
EXPECT_EQ(0, table->addressToIndex(seg.lowLim()));
EXPECT_EQ(hiLim, table->addressToIndex(seg.hiLim()));
}

TEST_F(CardTableNCTest, DirtyAddress) {
const size_t lastIx = CardTable::kValidIndices - 1;
const size_t lastIx = table->getEndIndex() - 1;

for (char *addr : addrs) {
size_t ind = table->addressToIndex(addr);
Expand All @@ -138,7 +138,8 @@ TEST_F(CardTableNCTest, DirtyAddress) {
TEST_F(CardTableNCTest, DirtyAddressRangeEmpty) {
char *addr = addrs.at(0);
table->dirtyCardsForAddressRange(addr, addr);
EXPECT_FALSE(table->findNextDirtyCard(0, CardTable::kValidIndices));
EXPECT_FALSE(table->findNextDirtyCard(
CardTable::kFirstUsedIndex, table->getEndIndex()));
}

/// Dirty an address range smaller than a single card.
Expand Down Expand Up @@ -215,25 +216,26 @@ TEST_F(CardTableNCTest, NextDirtyCardImmediate) {
size_t ind = table->addressToIndex(addr);

table->dirtyCardForAddress(addr);
auto dirty = table->findNextDirtyCard(ind, CardTable::kValidIndices);
auto dirty = table->findNextDirtyCard(ind, table->getEndIndex());

ASSERT_TRUE(dirty);
EXPECT_EQ(ind, *dirty);
}

TEST_F(CardTableNCTest, NextDirtyCard) {
/// Empty case: No dirty cards
EXPECT_FALSE(table->findNextDirtyCard(0, CardTable::kValidIndices));
EXPECT_FALSE(table->findNextDirtyCard(
CardTable::kFirstUsedIndex, table->getEndIndex()));

size_t from = 0;
size_t from = CardTable::kFirstUsedIndex;
for (char *addr : addrs) {
table->dirtyCardForAddress(addr);

auto ind = table->addressToIndex(addr);
EXPECT_FALSE(table->findNextDirtyCard(from, ind));

auto atEnd = table->findNextDirtyCard(from, ind + 1);
auto inMiddle = table->findNextDirtyCard(from, CardTable::kValidIndices);
auto inMiddle = table->findNextDirtyCard(from, table->getEndIndex());

ASSERT_TRUE(atEnd);
EXPECT_EQ(ind, *atEnd);
Expand Down

0 comments on commit 90bc2f2

Please sign in to comment.