Commit 4b46522a authored by Sigurd Schneider's avatar Sigurd Schneider Committed by Commit Bot

Revert "[heap] fix Sweeper::kNumberOfSweepingSpaces"

This reverts commit 12420537.

Reason for revert: MSAN complains
https://build.chromium.org/p/client.v8/builders/V8%20Linux%20-%20arm64%20-%20sim%20-%20MSAN/builds/20527

Original change's description:
> [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 <hpayer@chromium.org>
> Commit-Queue: Dan Elphick <delphick@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#52320}

TBR=hpayer@chromium.org,delphick@chromium.org

Change-Id: I9894dc10f122c9fab409e08b2a45389f1f51748f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/992152Reviewed-by: 's avatarSigurd Schneider <sigurds@chromium.org>
Commit-Queue: Sigurd Schneider <sigurds@chromium.org>
Cr-Commit-Position: refs/heads/master@{#52323}
parent 122ece2d
......@@ -48,18 +48,15 @@ Sweeper::FilterSweepingPagesScope::FilterSweepingPagesScope(
USE(pause_or_complete_scope_);
if (!sweeping_in_progress_) return;
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();
old_space_sweeping_list_ = std::move(sweeper_->sweeping_list_[OLD_SPACE]);
sweeper_->sweeping_list_[OLD_SPACE].clear();
}
Sweeper::FilterSweepingPagesScope::~FilterSweepingPagesScope() {
DCHECK_EQ(sweeping_in_progress_, sweeper_->sweeping_in_progress());
if (!sweeping_in_progress_) return;
sweeper_->sweeping_list_[GetSweepSpaceIndex(OLD_SPACE)] =
std::move(old_space_sweeping_list_);
sweeper_->sweeping_list_[OLD_SPACE] = std::move(old_space_sweeping_list_);
// old_space_sweeping_list_ does not need to be cleared as we don't use it.
}
......@@ -82,16 +79,19 @@ class Sweeper::SweeperTask final : public CancelableTask {
void RunInternal() final {
TRACE_BACKGROUND_GC(tracer_,
GCTracer::BackgroundScope::MC_BACKGROUND_SWEEPING);
DCHECK(IsValidSweepingSpace(space_to_start_));
DCHECK_GE(space_to_start_, FIRST_GROWABLE_PAGED_SPACE);
DCHECK_LE(space_to_start_, LAST_GROWABLE_PAGED_SPACE);
const int offset = space_to_start_ - FIRST_GROWABLE_PAGED_SPACE;
for (int i = 0; i < kNumberOfSweepingSpaces; i++) {
const AllocationSpace space_id = static_cast<AllocationSpace>(
FIRST_GROWABLE_PAGED_SPACE +
((i + offset) % kNumberOfSweepingSpaces));
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);
// Do not sweep code space concurrently.
if (space_id == CODE_SPACE) continue;
DCHECK(IsValidSweepingSpace(space_id));
sweeper_->SweepSpaceFromTask(space_id);
if (static_cast<AllocationSpace>(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<AllocationSpace>(space_id));
}
num_sweeping_tasks_->Decrement(1);
pending_sweeper_tasks_->Signal();
......@@ -139,9 +139,7 @@ void Sweeper::StartSweeping() {
MajorNonAtomicMarkingState* marking_state =
heap_->mark_compact_collector()->non_atomic_marking_state();
ForAllSweepingSpaces([this, marking_state](AllocationSpace space) {
int space_index = GetSweepSpaceIndex(space);
std::sort(sweeping_list_[space_index].begin(),
sweeping_list_[space_index].end(),
std::sort(sweeping_list_[space].begin(), sweeping_list_[space].end(),
[marking_state](Page* a, Page* b) {
return marking_state->live_bytes(a) <
marking_state->live_bytes(b);
......@@ -182,7 +180,7 @@ void Sweeper::SweepOrWaitUntilSweepingCompleted(Page* page) {
Page* Sweeper::GetSweptPageSafe(PagedSpace* space) {
base::LockGuard<base::Mutex> guard(&mutex_);
SweptList& list = swept_list_[GetSweepSpaceIndex(space->identity())];
SweptList& list = swept_list_[space->identity()];
if (!list.empty()) {
auto last_page = list.back();
list.pop_back();
......@@ -219,9 +217,8 @@ void Sweeper::EnsureCompleted() {
AbortAndWaitForTasks();
ForAllSweepingSpaces([this](AllocationSpace space) {
CHECK(sweeping_list_[GetSweepSpaceIndex(space)].empty());
});
ForAllSweepingSpaces(
[this](AllocationSpace space) { CHECK(sweeping_list_[space].empty()); });
sweeping_in_progress_ = false;
}
......@@ -383,7 +380,7 @@ bool Sweeper::SweepSpaceIncrementallyFromTask(AllocationSpace identity) {
if (Page* page = GetSweepingPageSafe(identity)) {
ParallelSweepPage(page, identity);
}
return sweeping_list_[GetSweepSpaceIndex(identity)].empty();
return sweeping_list_[identity].empty();
}
int Sweeper::ParallelSweepSpace(AllocationSpace identity,
......@@ -440,7 +437,7 @@ int Sweeper::ParallelSweepPage(Page* page, AllocationSpace identity) {
{
base::LockGuard<base::Mutex> guard(&mutex_);
swept_list_[GetSweepSpaceIndex(identity)].push_back(page);
swept_list_[identity].push_back(page);
}
return max_freed;
}
......@@ -468,7 +465,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_[GetSweepSpaceIndex(space)].push_back(page);
sweeping_list_[space].push_back(page);
}
void Sweeper::PrepareToBeSweptPage(AllocationSpace space, Page* page) {
......@@ -485,11 +482,10 @@ void Sweeper::PrepareToBeSweptPage(AllocationSpace space, Page* page) {
Page* Sweeper::GetSweepingPageSafe(AllocationSpace space) {
base::LockGuard<base::Mutex> guard(&mutex_);
DCHECK(IsValidSweepingSpace(space));
int space_index = GetSweepSpaceIndex(space);
Page* page = nullptr;
if (!sweeping_list_[space_index].empty()) {
page = sweeping_list_[space_index].front();
sweeping_list_[space_index].pop_front();
if (!sweeping_list_[space].empty()) {
page = sweeping_list_[space].front();
sweeping_list_[space].pop_front();
}
return page;
}
......
......@@ -51,8 +51,7 @@ class Sweeper {
void FilterOldSpaceSweepingPages(Callback callback) {
if (!sweeping_in_progress_) return;
SweepingList* sweeper_list =
&sweeper_->sweeping_list_[GetSweepSpaceIndex(OLD_SPACE)];
SweepingList* sweeper_list = &sweeper_->sweeping_list_[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++) {
......@@ -124,8 +123,7 @@ class Sweeper {
class IterabilityTask;
class SweeperTask;
static const int kNumberOfSweepingSpaces =
LAST_GROWABLE_PAGED_SPACE - FIRST_GROWABLE_PAGED_SPACE + 1;
static const int kNumberOfSweepingSpaces = LAST_GROWABLE_PAGED_SPACE + 1;
static const int kMaxSweeperTasks = 3;
template <typename Callback>
......@@ -164,16 +162,11 @@ class Sweeper {
return space == NEW_SPACE || space == RO_SPACE;
}
static bool IsValidSweepingSpace(AllocationSpace space) {
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_;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment