From 12420537c8e4f391b010950f154be3ad3a2a080e Mon Sep 17 00:00:00 2001 From: Dan Elphick Date: Thu, 29 Mar 2018 16:19:03 +0100 Subject: [PATCH] [heap] fix Sweeper::kNumberOfSweepingSpaces When indexing into vectors of sweeping spaces, convert the AllocationSpace to an index (by subtracting FIRST_GROWABLE_PAGED_SPACE) to avoid wasted space at the start. Change-Id: Ia23fe6dae42d5accea9f7fe7ec5c3b303ef857b4 Reviewed-on: https://chromium-review.googlesource.com/978242 Reviewed-by: Hannes Payer Commit-Queue: Dan Elphick Cr-Commit-Position: refs/heads/master@{#52320} --- src/heap/sweeper.cc | 52 ++++++++++++++++++++++++--------------------- src/heap/sweeper.h | 13 +++++++++--- 2 files changed, 38 insertions(+), 27 deletions(-) diff --git a/src/heap/sweeper.cc b/src/heap/sweeper.cc index b2c4549389..90c93d6d0f 100644 --- a/src/heap/sweeper.cc +++ b/src/heap/sweeper.cc @@ -48,15 +48,18 @@ Sweeper::FilterSweepingPagesScope::FilterSweepingPagesScope( USE(pause_or_complete_scope_); if (!sweeping_in_progress_) return; - old_space_sweeping_list_ = std::move(sweeper_->sweeping_list_[OLD_SPACE]); - sweeper_->sweeping_list_[OLD_SPACE].clear(); + int old_space_index = GetSweepSpaceIndex(OLD_SPACE); + old_space_sweeping_list_ = + std::move(sweeper_->sweeping_list_[old_space_index]); + sweeper_->sweeping_list_[old_space_index].clear(); } Sweeper::FilterSweepingPagesScope::~FilterSweepingPagesScope() { DCHECK_EQ(sweeping_in_progress_, sweeper_->sweeping_in_progress()); if (!sweeping_in_progress_) return; - sweeper_->sweeping_list_[OLD_SPACE] = std::move(old_space_sweeping_list_); + sweeper_->sweeping_list_[GetSweepSpaceIndex(OLD_SPACE)] = + std::move(old_space_sweeping_list_); // old_space_sweeping_list_ does not need to be cleared as we don't use it. } @@ -79,19 +82,16 @@ class Sweeper::SweeperTask final : public CancelableTask { void RunInternal() final { TRACE_BACKGROUND_GC(tracer_, GCTracer::BackgroundScope::MC_BACKGROUND_SWEEPING); - DCHECK_GE(space_to_start_, FIRST_GROWABLE_PAGED_SPACE); - DCHECK_LE(space_to_start_, LAST_GROWABLE_PAGED_SPACE); + DCHECK(IsValidSweepingSpace(space_to_start_)); const int offset = space_to_start_ - FIRST_GROWABLE_PAGED_SPACE; - const int num_spaces = - LAST_GROWABLE_PAGED_SPACE - FIRST_GROWABLE_PAGED_SPACE + 1; - for (int i = 0; i < num_spaces; i++) { - const int space_id = - FIRST_GROWABLE_PAGED_SPACE + ((i + offset) % num_spaces); + for (int i = 0; i < kNumberOfSweepingSpaces; i++) { + const AllocationSpace space_id = static_cast( + FIRST_GROWABLE_PAGED_SPACE + + ((i + offset) % kNumberOfSweepingSpaces)); // Do not sweep code space concurrently. - if (static_cast(space_id) == CODE_SPACE) continue; - DCHECK_GE(space_id, FIRST_GROWABLE_PAGED_SPACE); - DCHECK_LE(space_id, LAST_GROWABLE_PAGED_SPACE); - sweeper_->SweepSpaceFromTask(static_cast(space_id)); + if (space_id == CODE_SPACE) continue; + DCHECK(IsValidSweepingSpace(space_id)); + sweeper_->SweepSpaceFromTask(space_id); } num_sweeping_tasks_->Decrement(1); pending_sweeper_tasks_->Signal(); @@ -139,7 +139,9 @@ void Sweeper::StartSweeping() { MajorNonAtomicMarkingState* marking_state = heap_->mark_compact_collector()->non_atomic_marking_state(); ForAllSweepingSpaces([this, marking_state](AllocationSpace space) { - std::sort(sweeping_list_[space].begin(), sweeping_list_[space].end(), + int space_index = GetSweepSpaceIndex(space); + std::sort(sweeping_list_[space_index].begin(), + sweeping_list_[space_index].end(), [marking_state](Page* a, Page* b) { return marking_state->live_bytes(a) < marking_state->live_bytes(b); @@ -180,7 +182,7 @@ void Sweeper::SweepOrWaitUntilSweepingCompleted(Page* page) { Page* Sweeper::GetSweptPageSafe(PagedSpace* space) { base::LockGuard guard(&mutex_); - SweptList& list = swept_list_[space->identity()]; + SweptList& list = swept_list_[GetSweepSpaceIndex(space->identity())]; if (!list.empty()) { auto last_page = list.back(); list.pop_back(); @@ -217,8 +219,9 @@ void Sweeper::EnsureCompleted() { AbortAndWaitForTasks(); - ForAllSweepingSpaces( - [this](AllocationSpace space) { CHECK(sweeping_list_[space].empty()); }); + ForAllSweepingSpaces([this](AllocationSpace space) { + CHECK(sweeping_list_[GetSweepSpaceIndex(space)].empty()); + }); sweeping_in_progress_ = false; } @@ -380,7 +383,7 @@ bool Sweeper::SweepSpaceIncrementallyFromTask(AllocationSpace identity) { if (Page* page = GetSweepingPageSafe(identity)) { ParallelSweepPage(page, identity); } - return sweeping_list_[identity].empty(); + return sweeping_list_[GetSweepSpaceIndex(identity)].empty(); } int Sweeper::ParallelSweepSpace(AllocationSpace identity, @@ -437,7 +440,7 @@ int Sweeper::ParallelSweepPage(Page* page, AllocationSpace identity) { { base::LockGuard guard(&mutex_); - swept_list_[identity].push_back(page); + swept_list_[GetSweepSpaceIndex(identity)].push_back(page); } return max_freed; } @@ -465,7 +468,7 @@ void Sweeper::AddPage(AllocationSpace space, Page* page, DCHECK_EQ(Sweeper::READD_TEMPORARY_REMOVED_PAGE, mode); } DCHECK_EQ(Page::kSweepingPending, page->concurrent_sweeping_state().Value()); - sweeping_list_[space].push_back(page); + sweeping_list_[GetSweepSpaceIndex(space)].push_back(page); } void Sweeper::PrepareToBeSweptPage(AllocationSpace space, Page* page) { @@ -482,10 +485,11 @@ void Sweeper::PrepareToBeSweptPage(AllocationSpace space, Page* page) { Page* Sweeper::GetSweepingPageSafe(AllocationSpace space) { base::LockGuard guard(&mutex_); DCHECK(IsValidSweepingSpace(space)); + int space_index = GetSweepSpaceIndex(space); Page* page = nullptr; - if (!sweeping_list_[space].empty()) { - page = sweeping_list_[space].front(); - sweeping_list_[space].pop_front(); + if (!sweeping_list_[space_index].empty()) { + page = sweeping_list_[space_index].front(); + sweeping_list_[space_index].pop_front(); } return page; } diff --git a/src/heap/sweeper.h b/src/heap/sweeper.h index a7fd1f0493..820e348b99 100644 --- a/src/heap/sweeper.h +++ b/src/heap/sweeper.h @@ -51,7 +51,8 @@ class Sweeper { void FilterOldSpaceSweepingPages(Callback callback) { if (!sweeping_in_progress_) return; - SweepingList* sweeper_list = &sweeper_->sweeping_list_[OLD_SPACE]; + SweepingList* sweeper_list = + &sweeper_->sweeping_list_[GetSweepSpaceIndex(OLD_SPACE)]; // Iteration here is from most free space to least free space. for (auto it = old_space_sweeping_list_.begin(); it != old_space_sweeping_list_.end(); it++) { @@ -123,7 +124,8 @@ class Sweeper { class IterabilityTask; class SweeperTask; - static const int kNumberOfSweepingSpaces = LAST_GROWABLE_PAGED_SPACE + 1; + static const int kNumberOfSweepingSpaces = + LAST_GROWABLE_PAGED_SPACE - FIRST_GROWABLE_PAGED_SPACE + 1; static const int kMaxSweeperTasks = 3; template @@ -162,11 +164,16 @@ class Sweeper { return space == NEW_SPACE || space == RO_SPACE; } - bool IsValidSweepingSpace(AllocationSpace space) { + static bool IsValidSweepingSpace(AllocationSpace space) { return space >= FIRST_GROWABLE_PAGED_SPACE && space <= LAST_GROWABLE_PAGED_SPACE; } + static int GetSweepSpaceIndex(AllocationSpace space) { + DCHECK(IsValidSweepingSpace(space)); + return space - FIRST_GROWABLE_PAGED_SPACE; + } + Heap* const heap_; MajorNonAtomicMarkingState* marking_state_; int num_tasks_;