Revert "[heap] Fix racy OOM in new space concurrent sweeping"

This reverts commit 95eece3068.

Reason for revert: Broke single generation bot

Original change's description:
> [heap] Fix racy OOM in new space concurrent sweeping
>
> Some tests are flakily failing due to a timing issue between new space
> concurrent sweeping and allocations.
> When new spaces and other spaces are also swept, each concurrent thread
> will take one new space page. If a young allocation happens right after
> the atomic pause finished, it's possible that all new space pages are
> held by concurrent threads. The main thread will try to contribute to
> sweeping but get no pages, and fail to allocate.
>
> Fix by restoring the round robin order of sweeping, such that not all
> threads start with new space.
>
> Bug: v8:12612, v8:13413
> Change-Id: I3b448199b4678c339f9e59f7ca31d9e1e0e76011
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3976043
> Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
> Commit-Queue: Omer Katz <omerkatz@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#83918}

Bug: v8:12612, v8:13413
Change-Id: Id65358e55721b98d10f6737adaf057482aef103b
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3981275
Commit-Queue: Omer Katz <omerkatz@chromium.org>
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/heads/main@{#83919}
This commit is contained in:
Omer Katz 2022-10-26 08:59:40 +00:00 committed by V8 LUCI CQ
parent 95eece3068
commit 158de3ef88
3 changed files with 18 additions and 29 deletions

View File

@ -982,11 +982,11 @@ using WeakSlotCallbackWithHeap = bool (*)(Heap* heap, FullObjectSlot pointer);
// consecutive.
enum AllocationSpace {
RO_SPACE, // Immortal, immovable and immutable objects,
NEW_SPACE, // Young generation space for regular objects collected
// with Scavenger/MinorMC.
OLD_SPACE, // Old generation regular object space.
CODE_SPACE, // Old generation code object space, marked executable.
SHARED_SPACE, // Space shared between multiple isolates. Optional.
NEW_SPACE, // Young generation space for regular objects collected
// with Scavenger/MinorMC.
LO_SPACE, // Old generation large object space.
CODE_LO_SPACE, // Old generation large code object space.
NEW_LO_SPACE, // Young generation large object space.
@ -994,12 +994,12 @@ enum AllocationSpace {
FIRST_SPACE = RO_SPACE,
LAST_SPACE = SHARED_LO_SPACE,
FIRST_MUTABLE_SPACE = NEW_SPACE,
FIRST_MUTABLE_SPACE = OLD_SPACE,
LAST_MUTABLE_SPACE = SHARED_LO_SPACE,
FIRST_GROWABLE_PAGED_SPACE = OLD_SPACE,
LAST_GROWABLE_PAGED_SPACE = SHARED_SPACE,
FIRST_SWEEPABLE_SPACE = NEW_SPACE,
LAST_SWEEPABLE_SPACE = SHARED_SPACE
FIRST_SWEEPABLE_SPACE = OLD_SPACE,
LAST_SWEEPABLE_SPACE = NEW_SPACE
};
constexpr int kSpaceTagSize = 4;
static_assert(FIRST_SPACE == 0);

View File

@ -85,41 +85,30 @@ class Sweeper::SweeperJob final : public JobTask {
private:
void RunImpl(JobDelegate* delegate, bool is_joining_thread) {
static_assert(NEW_SPACE == FIRST_SWEEPABLE_SPACE);
static constexpr int non_new_space_sweeping_spaces =
kNumberOfSweepingSpaces - 1;
static_assert(NEW_SPACE ==
FIRST_SWEEPABLE_SPACE + non_new_space_sweeping_spaces);
const int offset = delegate->GetTaskId();
DCHECK_LT(offset, concurrent_sweepers_->size());
ConcurrentSweeper& concurrent_sweeper = (*concurrent_sweepers_)[offset];
if (offset > 0) {
if (!SweepNonNewSpaces(concurrent_sweeper, delegate, is_joining_thread,
offset, kNumberOfSweepingSpaces))
return;
}
ConcurrentSweeper& sweeper = (*concurrent_sweepers_)[offset];
{
TRACE_GC_EPOCH(
tracer_, sweeper_->GetTracingScope(NEW_SPACE, is_joining_thread),
is_joining_thread ? ThreadKind::kMain : ThreadKind::kBackground);
if (!concurrent_sweeper.ConcurrentSweepSpace(NEW_SPACE, delegate)) return;
if (!sweeper.ConcurrentSweepSpace(NEW_SPACE, delegate)) return;
}
if (!SweepNonNewSpaces(concurrent_sweeper, delegate, is_joining_thread, 1,
offset == 0 ? kNumberOfSweepingSpaces : offset))
return;
}
bool SweepNonNewSpaces(ConcurrentSweeper& concurrent_sweeper,
JobDelegate* delegate, bool is_joining_thread,
int first_space_index, int last_space_index) {
if (!sweeper_->should_sweep_non_new_spaces_) return true;
if (!sweeper_->should_sweep_non_new_spaces_) return;
TRACE_GC_EPOCH(
tracer_, sweeper_->GetTracingScope(OLD_SPACE, is_joining_thread),
is_joining_thread ? ThreadKind::kMain : ThreadKind::kBackground);
for (int i = first_space_index; i < last_space_index; i++) {
const AllocationSpace space_id =
static_cast<AllocationSpace>(FIRST_SWEEPABLE_SPACE + i);
for (int i = 0; i < non_new_space_sweeping_spaces; i++) {
const AllocationSpace space_id = static_cast<AllocationSpace>(
FIRST_SWEEPABLE_SPACE +
((i + offset) % non_new_space_sweeping_spaces));
DCHECK_NE(NEW_SPACE, space_id);
if (!concurrent_sweeper.ConcurrentSweepSpace(space_id, delegate))
return false;
if (!sweeper.ConcurrentSweepSpace(space_id, delegate)) return;
}
return true;
}
Sweeper* const sweeper_;

View File

@ -130,7 +130,7 @@ class Sweeper {
static const int kNumberOfSweepingSpaces =
LAST_SWEEPABLE_SPACE - FIRST_SWEEPABLE_SPACE + 1;
static constexpr int kMaxSweeperTasks = kNumberOfSweepingSpaces;
static constexpr int kMaxSweeperTasks = 3;
template <typename Callback>
void ForAllSweepingSpaces(Callback callback) const {