Skip to content

Commit

Permalink
Make SHSegmentInfo explicit in CardTable (#1505)
Browse files Browse the repository at this point in the history
Summary:

Currently `SHSegmentInfo` lives in the prefix of CardTable inline
storage (to be specific, prefix of the `cards_` array). But this is
only defined in one comment. Add it into a union with the `cards_`
array to make it clear. It also simplifies the reasoning of following
diffs, in which we need to add more fields to `SHSegmentInfo`.

In addition, `kFirstUsedIndex` should take into account of the size of
`SHSegmentInfo`, since the size of `SHSegmentInfo` could be larger than
`(2 * kCardTableSize) >> kLogCardSize)` for small segment size.

Differential Revision: D61747499
  • Loading branch information
lavenzg authored and facebook-github-bot committed Oct 21, 2024
1 parent 6c7fc1a commit df295ed
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 20 deletions.
40 changes: 23 additions & 17 deletions include/hermes/VM/CardTableNC.h
Original file line number Diff line number Diff line change
Expand Up @@ -77,21 +77,22 @@ class CardTable {
/// guaranteed by a static_assert below.
static constexpr size_t kHeapBytesPerCardByte = kCardSize;

/// A prefix of every segment is occupied by auxilary data
/// structures. The card table is the first such data structure.
/// The card table maps to the segment. Only the suffix of the card
/// table that maps to the suffix of entire segment that is used for
/// allocation is ever used; the prefix that maps to the card table
/// itself is not used. (Nor is the portion that of the card table
/// that maps to the other auxiliary data structure, the mark bit
/// array, but we don't attempt to calculate that here.)
/// It is useful to know the size of this unused region of
/// the card table, so it can be used for other purposes.
/// 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_).
/// A prefix of every segment is occupied by auxiliary data structures. The
/// card table is the first such data structure. The card table maps to the
/// segment. Only the suffix of the card table that maps to the suffix of
/// entire segment that is used for allocation is ever used; the prefix that
/// maps to the card table itself is not used, nor is the portion of the card
/// table that maps to the other auxiliary data structure: the mark bit array
/// and guard pages. This small space can be used for other purpose, such as
/// storing the SHSegmentInfo. The actual first used index should take into
/// account of this. Here we only calculate for CardTable and size of
/// SHSegmentInfo. It's only used as starting index for clearing/dirtying
/// range of bits.
/// 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 =
(2 * kCardTableSize) >> kLogCardSize;
std::max(sizeof(SHSegmentInfo), (2 * kCardTableSize) >> kLogCardSize);

CardTable() = default;
/// CardTable is not copyable or movable: It must be constructed in-place.
Expand Down Expand Up @@ -255,9 +256,14 @@ class CardTable {

void cleanOrDirtyRange(size_t from, size_t to, CardStatus cleanOrDirty);

/// This needs to be atomic so that the background thread in Hades can safely
/// dirty cards when compacting.
std::array<AtomicIfConcurrentGC<CardStatus>, kCardTableSize> cards_{};
union {
/// The bytes occupied by segmentInfo_ are guaranteed to be not override by
/// writes to cards_ array. See static assertions in AlignedHeapSegmentBase.
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_{};
};

/// See the comment at kHeapBytesPerCardByte above to see why this is
/// necessary.
Expand Down
7 changes: 4 additions & 3 deletions unittests/VMRuntime/CardTableNCTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,10 @@ void CardTableNCTest::dirtyRangeTest(

CardTableNCTest::CardTableNCTest() {
// For purposes of this test, we'll assume the first writeable byte of
// the segment comes just after the card table (which is at the
// start of the segment).
auto first = seg.lowLim() + sizeof(CardTable);
// the segment comes just after the memory region that can be mapped by
// kFirstUsedIndex bytes.
auto first = seg.lowLim() +
CardTable::kFirstUsedIndex * CardTable::kHeapBytesPerCardByte;
auto last = reinterpret_cast<char *>(llvh::alignDown(
reinterpret_cast<uintptr_t>(seg.hiLim() - 1), CardTable::kCardSize));

Expand Down

0 comments on commit df295ed

Please sign in to comment.