From 0820b2b7941593ff4f2563a60b81557a2d93bb3c Mon Sep 17 00:00:00 2001 From: "Gang Zhao (Hermes)" Date: Mon, 21 Oct 2024 16:30:22 -0700 Subject: [PATCH] Store segment size in SHSegmentInfo (#1506) 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 --- include/hermes/VM/AlignedHeapSegment.h | 22 +++++++++---- include/hermes/VM/CardTableNC.h | 43 ++++++++++++++++--------- include/hermes/VM/sh_segment_info.h | 3 ++ lib/VM/gcs/CardTableNC.cpp | 18 ++++------- unittests/VMRuntime/CardTableNCTest.cpp | 18 ++++++----- 5 files changed, 63 insertions(+), 41 deletions(-) diff --git a/include/hermes/VM/AlignedHeapSegment.h b/include/hermes/VM/AlignedHeapSegment.h index 19901e6e88d..8278fc0a8be 100644 --- a/include/hermes/VM/AlignedHeapSegment.h +++ b/include/hermes/VM/AlignedHeapSegment.h @@ -164,6 +164,19 @@ class AlignedHeapSegmentBase { return lowLim_; } + /// Read storage size from SHSegmentInfo. + size_t storageSize() const { + auto *segmentInfo = reinterpret_cast(lowLim_); + return segmentInfo->segmentSize; + } + + /// 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 { + return lowLim_ + storageSize(); + } + /// Returns the address at which the first allocation in this segment would /// occur. /// Disable UB sanitization because 'this' may be null during the tests. @@ -274,7 +287,9 @@ class AlignedHeapSegment : public AlignedHeapSegmentBase { /// Mask for isolating the storage being pointed into by a pointer. static constexpr size_t kHighMask{~kLowMask}; - /// Returns the storage size, in bytes, of an \c AlignedHeapSegment. + /// Returns the storage size, in bytes, of an \c AlignedHeapSegment. This + /// replaces AlignedHeapSegmentBase::storageSize, which reads the size from + /// SHSegmentInfo. static constexpr size_t storageSize() { return kSize; } @@ -380,11 +395,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). diff --git a/include/hermes/VM/CardTableNC.h b/include/hermes/VM/CardTableNC.h index f65e73c0581..f4b2c62a5cf 100644 --- a/include/hermes/VM/CardTableNC.h +++ b/include/hermes/VM/CardTableNC.h @@ -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 @@ -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; @@ -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. @@ -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( + reinterpret_cast(lowLim) + getSegmentSize()); + } #endif enum class CardStatus : char { Clean = 0, Dirty = 1 }; @@ -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, kCardTableSize> cards_{}; + std::array, kInlineCardTableSize> cards_{}; }; /// See the comment at kHeapBytesPerCardByte above to see why this is @@ -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. @@ -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()) && @@ -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; } diff --git a/include/hermes/VM/sh_segment_info.h b/include/hermes/VM/sh_segment_info.h index ae4c7ebdf51..0bc4539c52d 100644 --- a/include/hermes/VM/sh_segment_info.h +++ b/include/hermes/VM/sh_segment_info.h @@ -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 diff --git a/lib/VM/gcs/CardTableNC.cpp b/lib/VM/gcs/CardTableNC.cpp index ec94d5e5710..a3d94078364 100644 --- a/lib/VM/gcs/CardTableNC.cpp +++ b/lib/VM/gcs/CardTableNC.cpp @@ -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(high) + kCardSize - 1; @@ -44,19 +38,19 @@ OptValue CardTable::findNextCardWithStatus( } void CardTable::clear() { - cleanRange(kFirstUsedIndex, kValidIndices); + cleanRange(kFirstUsedIndex, getEndIndex()); } void CardTable::updateAfterCompaction(const void *newLevel) { const char *newLevelPtr = static_cast(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) { @@ -147,12 +141,12 @@ 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 @@ -160,7 +154,7 @@ void CardTable::unprotectBoundaryTable() { 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; diff --git a/unittests/VMRuntime/CardTableNCTest.cpp b/unittests/VMRuntime/CardTableNCTest.cpp index 745aa4300b3..67d1450e566 100644 --- a/unittests/VMRuntime/CardTableNCTest.cpp +++ b/unittests/VMRuntime/CardTableNCTest.cpp @@ -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 indices{ CardTable::kFirstUsedIndex, CardTable::kFirstUsedIndex + 1, @@ -105,13 +105,13 @@ TEST_F(CardTableNCTest, AddressToIndexBoundary) { // the storage. ASSERT_EQ(seg.lowLim(), reinterpret_cast(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); @@ -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. @@ -215,7 +216,7 @@ 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); @@ -223,9 +224,10 @@ TEST_F(CardTableNCTest, NextDirtyCardImmediate) { 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); @@ -233,7 +235,7 @@ TEST_F(CardTableNCTest, NextDirtyCard) { 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);