Allocate cards and boundaries array separately in CardTable for large segment

include/hermes/VM/CardTableNC.h

@@ -22,10 +22,16 @@ namespace hermes {
 namespace vm {
 
 /// The card table optimizes young gen collections by restricting the amount of
-/// heap belonging to the old gen that must be scanned.  The card table expects
-/// to be constructed inside an AlignedHeapSegment's storage, at some position
-/// before the allocation region, and covers the extent of that storage's
-/// memory.
+/// heap belonging to the old gen that must be scanned. The card table expects
+/// to be constructed at the beginning of a segment's storage, and covers the
+/// extent of that storage's memory. There are two cases:
+/// 1. For AlignedHeapSegment, the inline CardStatus array and Boundary array
+/// in the card table is large enough.
+/// 2. For JumboHeapSegment, the two arrays are allocated separately.
+/// In either case, the pointers to the CardStatus array and Boundary array are
+/// stored in \c cards and \c boundaries field of SHSegmentInfo, which occupies
+/// the prefix bytes of card table that are mapped to auxiliary data structures
+/// for a segment.
 ///
 /// Also supports the following query:  Given a card in the heap that intersects
 /// with the used portion of its segment, find its "crossing object" -- the
@@ -58,16 +64,19 @@ class CardTable {
     const char *address_{nullptr};
   };
 
+  enum class CardStatus : char { Clean = 0, Dirty = 1 };
+
   /// The size (and base-two log of the size) of cards used in the card table.
   static constexpr size_t kLogCardSize = 9; // ==> 512-byte cards.
   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;
+  /// Maximum ize of segment that can have inline cards and boundaries array.
+  static constexpr size_t kSegmentUnitSize = 1
+      << HERMESVM_LOG_HEAP_SEGMENT_SIZE;
 
-  /// 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 =
+      kSegmentUnitSize >> 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
@@ -77,29 +86,57 @@ 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_).
-  static constexpr size_t kFirstUsedIndex =
-      (2 * kCardTableSize) >> kLogCardSize;
-
-  CardTable() = default;
+  /// 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 = std::max(
+      sizeof(SHSegmentInfo),
+      (2 * kInlineCardTableSize) >> kLogCardSize);
+
+  CardTable(size_t segmentSize) {
+    assert(
+        segmentSize && segmentSize % kSegmentUnitSize == 0 &&
+        "segmentSize must be a multiple of kSegmentUnitSize");
+
+    segmentInfo_.segmentSize = segmentSize;
+    if (segmentSize == kSegmentUnitSize) {
+      // Just use the inline storage.
+      setCards(inlineCardStatusArray);
+      setBoundaries(inlineBoundaryArray_);
+    } else {
+      size_t cardTableSize = segmentSize >> kLogCardSize;
+      // CardStatus is clean by default, so must zero-initialize it.
+      setCards(new AtomicIfConcurrentGC<CardStatus>[cardTableSize] {});
+      setBoundaries(new int8_t[cardTableSize]);
+    }
+  }
   /// CardTable is not copyable or movable: It must be constructed in-place.
   CardTable(const CardTable &) = delete;
   CardTable(CardTable &&) = delete;
   CardTable &operator=(const CardTable &) = delete;
   CardTable &operator=(CardTable &&) = delete;
 
+  ~CardTable() {
+    // If CardStatus/Boundary array is allocated separately, free them.
+    if (cards() != inlineCardStatusArray) {
+      delete[] cards();
+    }
+    if (boundaries() != inlineBoundaryArray_) {
+      delete[] boundaries();
+    }
+  }
+
   /// Returns the card table index corresponding to a byte at the given address.
   /// \pre \p addr must be within the bounds of the segment owning this card
   /// table or at most 1 card after it, that is to say
@@ -112,8 +149,7 @@ class CardTable {
   /// of how this is used.
   inline size_t addressToIndex(const void *addr) const LLVM_NO_SANITIZE("null");
 
-  /// Returns the address corresponding to the given card table
-  /// index.
+  /// Returns the address corresponding to the given card table index.
   ///
   /// \pre \p index is bounded:
   ///
@@ -143,7 +179,7 @@ class CardTable {
   inline OptValue<size_t> findNextDirtyCard(size_t fromIndex, size_t endIndex)
       const;
 
-  /// If there is a card card at or after \p fromIndex, at an index less than
+  /// If there is a card at or after \p fromIndex, at an index less than
   /// \p endIndex, returns the index of the clean card, else returns none.
   inline OptValue<size_t> findNextCleanCard(size_t fromIndex, size_t endIndex)
       const;
@@ -184,12 +220,17 @@ 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.
   /// TODO(T48709128): remove this when the problem is diagnosed.
   int8_t cardObjectTableValue(unsigned index) const {
-    return boundaries_[index];
+    return boundaries()[index];
   }
 
   /// These methods protect and unprotect, respectively, the memory
@@ -214,13 +255,33 @@ 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 };
+  void setCards(AtomicIfConcurrentGC<CardStatus> *cards) {
+    segmentInfo_.cards = cards;
+  }
+
+  AtomicIfConcurrentGC<CardStatus> *cards() const {
+    return static_cast<AtomicIfConcurrentGC<CardStatus> *>(segmentInfo_.cards);
+  }
+
+  void setBoundaries(int8_t *boundaries) {
+    segmentInfo_.boundaries = boundaries;
+  }
+
+  int8_t *boundaries() const {
+    return segmentInfo_.boundaries;
+  }
 
   /// \return The lowest address whose card can be dirtied in this array. i.e.
   ///     The smallest address such that
@@ -255,14 +316,27 @@ 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.
+    /// Pointers to the underlying CardStatus array and boundary array are
+    /// stored in it. Note that we could also store the boundary array in a
+    /// union along with inlineBoundaryArray_, since that array has unused
+    /// prefix bytes as well. It will save 8 bytes here. But it makes the size
+    /// check more complex as we need to ensure that the segment size is large
+    /// enough so that inlineBoundaryArray_ has enough unused prefix bytes to
+    /// store the pointer.
+    SHSegmentInfo segmentInfo_;
+    /// This needs to be atomic so that the background thread in Hades can
+    /// safely dirty cards when compacting.
+    AtomicIfConcurrentGC<CardStatus>
+        inlineCardStatusArray[kInlineCardTableSize]{};
+  };
 
   /// See the comment at kHeapBytesPerCardByte above to see why this is
   /// necessary.
   static_assert(
-      sizeof(cards_[0]) == 1,
+      sizeof(inlineCardStatusArray[0]) == 1,
       "Validate assumption that card table entries are one byte");
 
   /// Each card has a corresponding signed byte in the boundaries_ table.  A
@@ -275,7 +349,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 inlineBoundaryArray_[kInlineCardTableSize];
 };
 
 /// Implementations of inlines.
@@ -305,7 +379,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()) &&
@@ -314,7 +388,7 @@ inline const char *CardTable::indexToAddress(size_t index) const {
 }
 
 inline void CardTable::dirtyCardForAddress(const void *addr) {
-  cards_[addressToIndex(addr)].store(
+  cards()[addressToIndex(addr)].store(
       CardStatus::Dirty, std::memory_order_relaxed);
 }
 
@@ -323,8 +397,8 @@ 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.");
-  return cards_[index].load(std::memory_order_relaxed) == CardStatus::Dirty;
+  assert(index < getEndIndex() && "index is required to be in range.");
+  return cards()[index].load(std::memory_order_relaxed) == CardStatus::Dirty;
 }
 
 inline OptValue<size_t> CardTable::findNextDirtyCard(
@@ -348,9 +422,9 @@ inline CardTable::Boundary CardTable::nextBoundary(const char *level) const {
 }
 
 inline const char *CardTable::base() const {
-  // As we know the card table is laid out inline before the allocation region
-  // of its aligned heap segment, we can use its own this pointer as the base
-  // address.
+  // As we know the card table is laid out inline at the beginning of the
+  // segment storage, which is before the allocation region, we can use its own
+  // this pointer as the base address.
   return reinterpret_cast<const char *>(this);
 }
 

include/hermes/VM/HeapRuntime.h

@@ -22,7 +22,7 @@ class HeapRuntime {
  public:
   ~HeapRuntime() {
     runtime_->~RT();
-    sp_->deleteStorage(runtime_);
+    sp_->deleteStorage(runtime_, kHeapRuntimeStorageSize);
   }
 
   /// Allocate a segment and create an aliased shared_ptr that points to the
@@ -36,16 +36,17 @@ class HeapRuntime {
 
  private:
   HeapRuntime(std::shared_ptr<StorageProvider> sp) : sp_{std::move(sp)} {
-    auto ptrOrError = sp_->newStorage("hermes-rt");
+    auto ptrOrError = sp_->newStorage("hermes-rt", kHeapRuntimeStorageSize);
     if (!ptrOrError)
       hermes_fatal("Cannot initialize Runtime storage.", ptrOrError.getError());
-    static_assert(
-        sizeof(RT) < AlignedHeapSegment::storageSize(), "Segments too small.");
+    static_assert(sizeof(RT) < kHeapRuntimeStorageSize, "Segments too small.");
     runtime_ = static_cast<RT *>(*ptrOrError);
   }
 
   std::shared_ptr<StorageProvider> sp_;
   RT *runtime_;
+  static constexpr size_t kHeapRuntimeStorageSize =
+      AlignedHeapSegment::storageSize();
 };
 } // namespace vm
 } // namespace hermes

include/hermes/VM/LimitedStorageProvider.h

@@ -29,9 +29,9 @@ class LimitedStorageProvider final : public StorageProvider {
       : delegate_(std::move(provider)), limit_(limit) {}
 
  protected:
-  llvh::ErrorOr<void *> newStorageImpl(const char *name) override;
+  llvh::ErrorOr<void *> newStorageImpl(const char *name, size_t sz) override;
 
-  void deleteStorageImpl(void *storage) override;
+  void deleteStorageImpl(void *storage, size_t sz) override;
 };
 
 } // namespace vm

include/hermes/VM/StorageProvider.h

@@ -37,20 +37,21 @@ class StorageProvider {
 
   /// @}
 
-  /// Create a new segment memory space.
-  llvh::ErrorOr<void *> newStorage() {
-    return newStorage(nullptr);
+  /// Create a new segment memory space with given size \p sz.
+  llvh::ErrorOr<void *> newStorage(size_t sz) {
+    return newStorage(nullptr, sz);
   }
-  /// Create a new segment memory space and give this memory the name \p name.
-  /// \return A pointer to a block of memory that has
-  /// AlignedHeapSegment::storageSize() bytes, and is aligned on
-  /// AlignedHeapSegment::storageSize().
-  llvh::ErrorOr<void *> newStorage(const char *name);
+  /// \return A pointer to a block of memory that has \p sz bytes, and is
+  /// aligned on AlignedHeapSegmentBase::kSegmentUnitSize. Note that \p sz
+  /// must be equals to or a multiple of
+  /// AlignedHeapSegmentBase::kSegmentUnitSize.
+  llvh::ErrorOr<void *> newStorage(const char *name, size_t sz);
 
   /// Delete the given segment's memory space, and make it available for re-use.
-  /// \post Nothing in the range [storage, storage +
-  /// AlignedHeapSegment::storageSize()) is valid memory to be read or written.
-  void deleteStorage(void *storage);
+  /// Note that \p sz must be the same as used to allocating \p storage.
+  /// \post Nothing in the range [storage, storage + sz) is valid memory to be
+  /// read or written.
+  void deleteStorage(void *storage, size_t sz);
 
   /// The number of storages this provider has allocated in its lifetime.
   size_t numSucceededAllocs() const;
@@ -67,8 +68,8 @@ class StorageProvider {
   size_t numLiveAllocs() const;
 
  protected:
-  virtual llvh::ErrorOr<void *> newStorageImpl(const char *name) = 0;
-  virtual void deleteStorageImpl(void *storage) = 0;
+  virtual llvh::ErrorOr<void *> newStorageImpl(const char *name, size_t sz) = 0;
+  virtual void deleteStorageImpl(void *storage, size_t sz) = 0;
 
  private:
   size_t numSucceededAllocs_{0};

include/hermes/VM/sh_segment_info.h

@@ -12,6 +12,15 @@
 /// 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;
+  /// Pointer that points to the CardStatus array for this segment.
+  /// Erase the actual type AtomicIfConcurrent<CardStatus> here to avoid using
+  /// C++ type and forward declaring nested type.
+  void *cards;
+  /// Pointer that points to the boundary array for this segment.
+  int8_t *boundaries;
 } SHSegmentInfo;
 
 #endif