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

Author: lavenzg

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<const SHSegmentInfo *>(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).

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<char *>(
+        reinterpret_cast<uintptr_t>(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<AtomicIfConcurrentGC<CardStatus>, kCardTableSize> cards_{};
+    std::array<AtomicIfConcurrentGC<CardStatus>, 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;
 }
 

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

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<const char *>(high) + kCardSize - 1;
@@ -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) {
@@ -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;

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<size_t> indices{
       CardTable::kFirstUsedIndex,
       CardTable::kFirstUsedIndex + 1,
@@ -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);
@@ -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,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);