Revert "[heap] Introduce per-thread storage for concurrent sweeping"

This reverts commit a1b863c1.

Reason for revert: https://ci.chromium.org/ui/p/v8/builders/ci/V8%20Clusterfuzz%20Linux%20ASAN%20no%20inline%20-%20release%20builder/8288/overview

Original change's description:
> [heap] Introduce per-thread storage for concurrent sweeping
>
> Introduce ConcurrentSweeper as indirection between SweeperJob and
> Sweeper to hold per-thread state during sweeping.
> This will be used by MinorMC sweeping to hold the pretenuring feedback
> map.
>
> Bug: v8:12612
> Change-Id: Ib363339f9109b405e4cae7f2c08cb4f0eacff8d0
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3829466
> Commit-Queue: Omer Katz <omerkatz@chromium.org>
> Reviewed-by: Dominik Inführ <dinfuehr@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#82442}

Bug: v8:12612
Change-Id: I66865a807908a6ef296e06530f293dcf27fea1a3
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3829478
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Owners-Override: Nico Hartmann <nicohartmann@chromium.org>
Auto-Submit: Nico Hartmann <nicohartmann@chromium.org>
Cr-Commit-Position: refs/heads/main@{#82443}

src/heap/sweeper.cc

@@ -4,9 +4,6 @@
 
 #include "src/heap/sweeper.h"
 
-#include <memory>
-#include <vector>
-
 #include "src/common/globals.h"
 #include "src/execution/vm-state-inl.h"
 #include "src/heap/base/active-system-pages.h"
@@ -28,8 +25,6 @@ Sweeper::Sweeper(Heap* heap, NonAtomicMarkingState* marking_state)
       sweeping_in_progress_(false),
       should_reduce_memory_(false) {}
 
-Sweeper::~Sweeper() { DCHECK(concurrent_sweepers_.empty()); }
-
 Sweeper::PauseScope::PauseScope(Sweeper* sweeper) : sweeper_(sweeper) {
   if (!sweeper_->sweeping_in_progress()) return;
 
@@ -68,31 +63,10 @@ Sweeper::FilterSweepingPagesScope::~FilterSweepingPagesScope() {
   // old_space_sweeping_list_ does not need to be cleared as we don't use it.
 }
 
-class Sweeper::ConcurrentSweeper final {
- public:
-  explicit ConcurrentSweeper(Sweeper* sweeper) : sweeper_(sweeper) {}
-
-  bool ConcurrentSweepSpace(AllocationSpace identity, JobDelegate* delegate) {
-    while (!delegate->ShouldYield()) {
-      Page* page = sweeper_->GetSweepingPageSafe(identity);
-      if (page == nullptr) return true;
-      sweeper_->ParallelSweepPage(page, identity,
-                                  SweepingMode::kLazyOrConcurrent);
-    }
-    return false;
-  }
-
- private:
-  Sweeper* const sweeper_;
-};
-
 class Sweeper::SweeperJob final : public JobTask {
  public:
-  SweeperJob(Isolate* isolate, Sweeper* sweeper,
-             std::vector<ConcurrentSweeper>* concurrent_sweepers)
-      : sweeper_(sweeper),
-        concurrent_sweepers_(concurrent_sweepers),
-        tracer_(isolate->heap()->tracer()) {}
+  SweeperJob(Isolate* isolate, Sweeper* sweeper)
+      : sweeper_(sweeper), tracer_(isolate->heap()->tracer()) {}
 
   ~SweeperJob() override = default;
 
@@ -113,7 +87,7 @@ class Sweeper::SweeperJob final : public JobTask {
   size_t GetMaxConcurrency(size_t worker_count) const override {
     const size_t kPagePerTask = 2;
     return std::min<size_t>(
-        concurrent_sweepers_->size(),
+        kMaxSweeperTasks,
         worker_count +
             (sweeper_->ConcurrentSweepingPageCount() + kPagePerTask - 1) /
                 kPagePerTask);
@@ -122,19 +96,15 @@ class Sweeper::SweeperJob final : public JobTask {
  private:
   void RunImpl(JobDelegate* delegate) {
     const int offset = delegate->GetTaskId();
-    DCHECK_LT(offset, concurrent_sweepers_->size());
-    ConcurrentSweeper& sweeper = (*concurrent_sweepers_)[offset];
     for (int i = 0; i < kNumberOfSweepingSpaces; i++) {
       const AllocationSpace space_id = static_cast<AllocationSpace>(
           FIRST_GROWABLE_PAGED_SPACE +
           ((i + offset) % kNumberOfSweepingSpaces));
       DCHECK(IsValidSweepingSpace(space_id));
-      if (!sweeper.ConcurrentSweepSpace(space_id, delegate)) return;
+      if (!sweeper_->ConcurrentSweepSpace(space_id, delegate)) return;
     }
   }
-
   Sweeper* const sweeper_;
-  std::vector<ConcurrentSweeper>* const concurrent_sweepers_;
   GCTracer* const tracer_;
 };
 
@@ -164,26 +134,13 @@ void Sweeper::StartSweeping() {
   });
 }
 
-int Sweeper::NumberOfConcurrentSweepers() const {
-  DCHECK(FLAG_concurrent_sweeping);
-  return std::min(Sweeper::kMaxSweeperTasks,
-                  V8::GetCurrentPlatform()->NumberOfWorkerThreads() + 1);
-}
-
 void Sweeper::StartSweeperTasks() {
   DCHECK(!job_handle_ || !job_handle_->IsValid());
   if (FLAG_concurrent_sweeping && sweeping_in_progress_ &&
       !heap_->delay_sweeper_tasks_for_testing_) {
-    if (concurrent_sweepers_.empty()) {
-      for (int i = 0; i < NumberOfConcurrentSweepers(); ++i) {
-        concurrent_sweepers_.emplace_back(this);
-      }
-    }
-    DCHECK_EQ(NumberOfConcurrentSweepers(), concurrent_sweepers_.size());
     job_handle_ = V8::GetCurrentPlatform()->PostJob(
         TaskPriority::kUserVisible,
-        std::make_unique<SweeperJob>(heap_->isolate(), this,
-                                     &concurrent_sweepers_));
+        std::make_unique<SweeperJob>(heap_->isolate(), this));
   }
 }
 
@@ -212,8 +169,6 @@ void Sweeper::EnsureCompleted() {
   ForAllSweepingSpaces([this](AllocationSpace space) {
     CHECK(sweeping_list_[GetSweepSpaceIndex(space)].empty());
   });
-
-  concurrent_sweepers_.clear();
   sweeping_in_progress_ = false;
 }
 
@@ -459,6 +414,20 @@ size_t Sweeper::ConcurrentSweepingPageCount() {
          sweeping_list_[GetSweepSpaceIndex(MAP_SPACE)].size();
 }
 
+bool Sweeper::ConcurrentSweepSpace(AllocationSpace identity,
+                                   JobDelegate* delegate) {
+  while (!delegate->ShouldYield()) {
+    Page* page = GetSweepingPageSafe(identity);
+    if (page == nullptr) return true;
+    // Typed slot sets are only recorded on code pages. Code pages
+    // are not swept concurrently to the application to ensure W^X.
+    DCHECK_NULL((page->typed_slot_set<OLD_TO_NEW>()));
+    DCHECK_NULL((page->typed_slot_set<OLD_TO_OLD>()));
+    ParallelSweepPage(page, identity, SweepingMode::kLazyOrConcurrent);
+  }
+  return false;
+}
+
 int Sweeper::ParallelSweepSpace(AllocationSpace identity,
                                 SweepingMode sweeping_mode,
                                 int required_freed_bytes, int max_pages) {

src/heap/sweeper.h

@@ -75,7 +75,6 @@ class Sweeper {
   enum class SweepingMode { kEagerDuringGC, kLazyOrConcurrent };
 
   Sweeper(Heap* heap, NonAtomicMarkingState* marking_state);
-  ~Sweeper();
 
   bool sweeping_in_progress() const { return sweeping_in_progress_; }
 
@@ -108,7 +107,6 @@ class Sweeper {
   Page* GetSweptPageSafe(PagedSpaceBase* space);
 
  private:
-  class ConcurrentSweeper;
   class SweeperJob;
 
   static const int kNumberOfSweepingSpaces =
@@ -159,6 +157,10 @@ class Sweeper {
 
   size_t ConcurrentSweepingPageCount();
 
+  // Concurrently sweeps many page from the given space. Returns true if there
+  // are no more pages to sweep in the given space.
+  bool ConcurrentSweepSpace(AllocationSpace identity, JobDelegate* delegate);
+
   Page* GetSweepingPageSafe(AllocationSpace space);
   bool TryRemoveSweepingPageSafe(AllocationSpace space, Page* page);
 
@@ -174,8 +176,6 @@ class Sweeper {
     return space - FIRST_GROWABLE_PAGED_SPACE;
   }
 
-  int NumberOfConcurrentSweepers() const;
-
   Heap* const heap_;
   NonAtomicMarkingState* marking_state_;
   std::unique_ptr<JobHandle> job_handle_;
@@ -183,7 +183,6 @@ class Sweeper {
   base::ConditionVariable cv_page_swept_;
   SweptList swept_list_[kNumberOfSweepingSpaces];
   SweepingList sweeping_list_[kNumberOfSweepingSpaces];
-  std::vector<ConcurrentSweeper> concurrent_sweepers_;
   // Main thread can finalize sweeping, while background threads allocation slow
   // path checks this flag to see whether it could support concurrent sweeping.
   std::atomic<bool> sweeping_in_progress_;