commit c48811c3f3adf6d2aa0671a2174c8bc5ef38cce8
author Lokesh Gidra <lokeshgidra@google.com> Fri Dec 23 18:03:09 2022 +0000
committer Lokesh Gidra <lokeshgidra@google.com> Wed Feb 01 18:58:51 2023 +0000
tree fb4e2efa8b76261c07d37b310707a34a576f8c9e
parent dc41395969220505ed41db97e2ceaccac55d8af1

Reland "Clear mark-bitmap after compaction"

This reverts commit 62a9bce97c8b75ab2c53be26fb95eb04289bec93.

Reason for revert: Fix the issue: non-moving space gets changed in
pre-zygote fork, so can't be const initialized in the constructor.

Bug: 263341475
Bug: 263326265
Bug: 160737021
Test: pre-submit and crystallbal/MPTS
Change-Id: I5322c15fe17014c1a199ffd8660917dc01a4b201
(cherry picked from commit de88488dc76c8be175273ed7f6eb8684a02f48f4)
Merged-In: I5322c15fe17014c1a199ffd8660917dc01a4b201

runtime/gc/collector/mark_compact.cc

diff --git a/runtime/gc/collector/mark_compact.cc b/runtime/gc/collector/mark_compact.cc
index 015027d..c9b2ff4 100644
--- a/runtime/gc/collector/mark_compact.cc
+++ b/runtime/gc/collector/mark_compact.cc
@@ -243,6 +243,7 @@
       gc_barrier_(0),
       mark_stack_lock_("mark compact mark stack lock", kMarkSweepMarkStackLock),
       bump_pointer_space_(heap->GetBumpPointerSpace()),
+      moving_space_bitmap_(bump_pointer_space_->GetMarkBitmap()),
       moving_to_space_fd_(kFdUnused),
       moving_from_space_fd_(kFdUnused),
       uffd_(kFdUnused),
@@ -435,13 +436,8 @@
       // in these spaces. This card-table will eventually be used to track
       // mutations while concurrent marking is going on.
       card_table->ClearCardRange(space->Begin(), space->Limit());
-      if (space == bump_pointer_space_) {
-        // It is OK to clear the bitmap with mutators running since the only
-        // place it is read is VisitObjects which has exclusion with this GC.
-        moving_space_bitmap_ = bump_pointer_space_->GetMarkBitmap();
-        moving_space_bitmap_->Clear();
-      } else {
-        CHECK(space == heap_->GetNonMovingSpace());
+      if (space != bump_pointer_space_) {
+        CHECK_EQ(space, heap_->GetNonMovingSpace());
         non_moving_space_ = space;
         non_moving_space_bitmap_ = space->GetMarkBitmap();
       }
@@ -3752,6 +3748,11 @@
 
   info_map_.MadviseDontNeedAndZero();
   live_words_bitmap_->ClearBitmap();
+  // TODO: We can clear this bitmap right before compaction pause. But in that
+  // case we need to ensure that we don't assert on this bitmap afterwards.
+  // Also, we would still need to clear it here again as we may have to use the
+  // bitmap for black-allocations (see UpdateMovingSpaceBlackAllocations()).
+  moving_space_bitmap_->Clear();
 
   if (UNLIKELY(is_zygote && IsValidFd(uffd_))) {
     heap_->DeleteThreadPool();

runtime/gc/collector/mark_compact.h

diff --git a/runtime/gc/collector/mark_compact.h b/runtime/gc/collector/mark_compact.h
index 498a56d..f97c0d7 100644
--- a/runtime/gc/collector/mark_compact.h
+++ b/runtime/gc/collector/mark_compact.h
@@ -628,11 +628,11 @@
   size_t last_checked_reclaim_page_idx_;
   uint8_t* last_reclaimed_page_;
 
-  // The main space bitmap
-  accounting::ContinuousSpaceBitmap* moving_space_bitmap_;
-  accounting::ContinuousSpaceBitmap* non_moving_space_bitmap_;
   space::ContinuousSpace* non_moving_space_;
   space::BumpPointerSpace* const bump_pointer_space_;
+  // The main space bitmap
+  accounting::ContinuousSpaceBitmap* const moving_space_bitmap_;
+  accounting::ContinuousSpaceBitmap* non_moving_space_bitmap_;
   Thread* thread_running_gc_;
   // Array of pages' compaction status.
   Atomic<PageState>* moving_pages_status_;