Commit de5a1fdf authored by Michael Achenbach's avatar Michael Achenbach Committed by V8 LUCI CQ

Revert "cppgc: Be more conservative in Seeper::FinishIfOutOfWork"

This reverts commit defa678e.

Reason for revert: Blocks roll:
https://ci.chromium.org/ui/p/v8/builders/ci/Linux%20V8%20FYI%20Release%20(NVIDIA)/21307/overview

Original change's description:
> cppgc: Be more conservative in Seeper::FinishIfOutOfWork
>
> Finalizing sweeping can be beneficial to truly end a GC cylce. We
> should only finalize in `FinishIfOutOfWork()` though if that would not
> introduce any jank. Limit the amount of executing finalizers in that
> scenario.
>
> Bug: v8:13294
> Change-Id: I0237f6b6017d444c457923d83e85147c58586445
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3902222
> Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
> Reviewed-by: Omer Katz <omerkatz@chromium.org>
> Reviewed-by: Anton Bikineev <bikineev@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#83279}

Bug: v8:13294
Change-Id: Ic3cf7e105a076ef41b35a075d8f35918bc412588
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3902582
Owners-Override: Michael Achenbach <machenbach@chromium.org>
Reviewed-by: 's avatarMichael Lippautz <mlippautz@chromium.org>
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: 's avatarOmer Katz <omerkatz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#83295}
parent 4c0e3614
...@@ -52,7 +52,6 @@ namespace internal { ...@@ -52,7 +52,6 @@ namespace internal {
V(MarkVisitCrossThreadPersistents) \ V(MarkVisitCrossThreadPersistents) \
V(MarkVisitStack) \ V(MarkVisitStack) \
V(MarkVisitRememberedSets) \ V(MarkVisitRememberedSets) \
V(SweepFinishIfOutOfWork) \
V(SweepInvokePreFinalizers) \ V(SweepInvokePreFinalizers) \
V(SweepIdleStep) \ V(SweepIdleStep) \
V(SweepInTask) \ V(SweepInTask) \
......
...@@ -400,7 +400,7 @@ class SweepFinalizer final { ...@@ -400,7 +400,7 @@ class SweepFinalizer final {
bool FinalizeSpaceWithDeadline(SpaceState* space_state, bool FinalizeSpaceWithDeadline(SpaceState* space_state,
double deadline_in_seconds) { double deadline_in_seconds) {
DCHECK(platform_); DCHECK(platform_);
static constexpr size_t kDeadlineCheckInterval = 4; static constexpr size_t kDeadlineCheckInterval = 8;
size_t page_count = 1; size_t page_count = 1;
while (auto page_state = space_state->swept_unfinalized_pages.Pop()) { while (auto page_state = space_state->swept_unfinalized_pages.Pop()) {
...@@ -543,7 +543,7 @@ class MutatorThreadSweeper final : private HeapVisitor<MutatorThreadSweeper> { ...@@ -543,7 +543,7 @@ class MutatorThreadSweeper final : private HeapVisitor<MutatorThreadSweeper> {
private: private:
bool SweepSpaceWithDeadline(SpaceState* state, double deadline_in_seconds) { bool SweepSpaceWithDeadline(SpaceState* state, double deadline_in_seconds) {
static constexpr size_t kDeadlineCheckInterval = 4; static constexpr size_t kDeadlineCheckInterval = 8;
size_t page_count = 1; size_t page_count = 1;
while (auto page = state->unswept_pages.Pop()) { while (auto page = state->unswept_pages.Pop()) {
Traverse(**page); Traverse(**page);
...@@ -884,8 +884,6 @@ class Sweeper::SweeperImpl final { ...@@ -884,8 +884,6 @@ class Sweeper::SweeperImpl final {
if (is_in_progress_ && !is_sweeping_on_mutator_thread_ && if (is_in_progress_ && !is_sweeping_on_mutator_thread_ &&
concurrent_sweeper_handle_ && concurrent_sweeper_handle_->IsValid() && concurrent_sweeper_handle_ && concurrent_sweeper_handle_->IsValid() &&
!concurrent_sweeper_handle_->IsActive()) { !concurrent_sweeper_handle_->IsActive()) {
StatsCollector::EnabledScope stats_scope(
stats_collector_, StatsCollector::kSweepFinishIfOutOfWork);
// At this point we know that the concurrent sweeping task has run // At this point we know that the concurrent sweeping task has run
// out-of-work: all pages are swept. The main thread still needs to finish // out-of-work: all pages are swept. The main thread still needs to finish
// sweeping though. // sweeping though.
...@@ -893,19 +891,7 @@ class Sweeper::SweeperImpl final { ...@@ -893,19 +891,7 @@ class Sweeper::SweeperImpl final {
[](const SpaceState& state) { [](const SpaceState& state) {
return state.unswept_pages.IsEmpty(); return state.unswept_pages.IsEmpty();
})); }));
FinishIfRunning();
// There may be unfinalized pages left. Since it's hard to estimate
// the actual amount of sweeping necessary, we sweep with a small
// deadline to see if sweeping can be fully finished.
MutatorThreadSweeper sweeper(heap_.heap(), &space_states_, platform_,
config_.free_memory_handling);
const double deadline =
platform_->MonotonicallyIncreasingTime() +
v8::base::TimeDelta::FromMilliseconds(2).InSecondsF();
if (sweeper.SweepWithDeadline(deadline)) {
FinalizeSweep();
NotifyDone();
}
} }
} }
......
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