From df295edb32126b70cd8366242fa6d315c30c6cd9 Mon Sep 17 00:00:00 2001 From: "Gang Zhao (Hermes)" Date: Mon, 21 Oct 2024 16:36:32 -0700 Subject: [PATCH] Make SHSegmentInfo explicit in CardTable (#1505) 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 --- include/hermes/VM/CardTableNC.h | 40 ++++++++++++++----------- unittests/VMRuntime/CardTableNCTest.cpp | 7 +++-- 2 files changed, 27 insertions(+), 20 deletions(-) diff --git a/include/hermes/VM/CardTableNC.h b/include/hermes/VM/CardTableNC.h index 5bfa40f2102..f65e73c0581 100644 --- a/include/hermes/VM/CardTableNC.h +++ b/include/hermes/VM/CardTableNC.h @@ -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. @@ -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, 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, kCardTableSize> cards_{}; + }; /// See the comment at kHeapBytesPerCardByte above to see why this is /// necessary. diff --git a/unittests/VMRuntime/CardTableNCTest.cpp b/unittests/VMRuntime/CardTableNCTest.cpp index adaffe0651d..745aa4300b3 100644 --- a/unittests/VMRuntime/CardTableNCTest.cpp +++ b/unittests/VMRuntime/CardTableNCTest.cpp @@ -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(llvh::alignDown( reinterpret_cast(seg.hiLim() - 1), CardTable::kCardSize));