Commit 7898475e authored by machenbach's avatar machenbach Committed by Commit bot

Revert of Make full GC reduce memory footprint an explicit event in the idle...

Revert of Make full GC reduce memory footprint an explicit event in the idle notification handler. (patchset #2 id:20001 of https://codereview.chromium.org/1072363002/)

Reason for revert:
[Sheriff] breaks nosnap with timeouts:
http://build.chromium.org/p/client.v8/builders/V8%20Linux%20-%20nosnap/builds/2513
http://build.chromium.org/p/client.v8/builders/V8%20Win32%20-%20nosnap%20-%20shared/builds/6220

Original issue's description:
> Make full GC reduce memory footprint an explicit event in the idle notification handler.
>
> BUG=
>
> Committed: https://crrev.com/845705aa99b6bfa8d264cfda1c3b5f1229802ab5
> Cr-Commit-Position: refs/heads/master@{#27753}

TBR=ulan@chromium.org,rmcilroy@chromium.org,hpayer@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=

Review URL: https://codereview.chromium.org/1081443002

Cr-Commit-Position: refs/heads/master@{#27755}
parent 88630d4e
...@@ -39,9 +39,6 @@ void GCIdleTimeAction::Print() { ...@@ -39,9 +39,6 @@ void GCIdleTimeAction::Print() {
case DO_FULL_GC: case DO_FULL_GC:
PrintF("full GC"); PrintF("full GC");
break; break;
case DO_FULL_GC_COMPACT:
PrintF("full GC compact");
break;
case DO_FINALIZE_SWEEPING: case DO_FINALIZE_SWEEPING:
PrintF("finalize sweeping"); PrintF("finalize sweeping");
break; break;
...@@ -159,18 +156,13 @@ bool GCIdleTimeHandler::ShouldDoScavenge( ...@@ -159,18 +156,13 @@ bool GCIdleTimeHandler::ShouldDoScavenge(
bool GCIdleTimeHandler::ShouldDoMarkCompact( bool GCIdleTimeHandler::ShouldDoMarkCompact(
size_t idle_time_in_ms, size_t size_of_objects, size_t idle_time_in_ms, size_t size_of_objects,
size_t mark_compact_speed_in_bytes_per_ms) { size_t mark_compact_speed_in_bytes_per_ms) {
return idle_time_in_ms >= return idle_time_in_ms >= kMaxScheduledIdleTime &&
idle_time_in_ms >=
EstimateMarkCompactTime(size_of_objects, EstimateMarkCompactTime(size_of_objects,
mark_compact_speed_in_bytes_per_ms); mark_compact_speed_in_bytes_per_ms);
} }
bool GCIdleTimeHandler::ShouldDoReduceMemoryMarkCompact(
size_t idle_time_in_ms) {
return idle_time_in_ms >= kMinTimeForReduceMemory;
}
bool GCIdleTimeHandler::ShouldDoContextDisposalMarkCompact( bool GCIdleTimeHandler::ShouldDoContextDisposalMarkCompact(
int contexts_disposed, double contexts_disposal_rate) { int contexts_disposed, double contexts_disposal_rate) {
return contexts_disposed > 0 && contexts_disposal_rate > 0 && return contexts_disposed > 0 && contexts_disposal_rate > 0 &&
...@@ -215,14 +207,13 @@ GCIdleTimeAction GCIdleTimeHandler::NothingOrDone() { ...@@ -215,14 +207,13 @@ GCIdleTimeAction GCIdleTimeHandler::NothingOrDone() {
// (3) If there is currently no MarkCompact idle round going on, we start a // (3) If there is currently no MarkCompact idle round going on, we start a
// new idle round if enough garbage was created. Otherwise we do not perform // new idle round if enough garbage was created. Otherwise we do not perform
// garbage collection to keep system utilization low. // garbage collection to keep system utilization low.
// (4) If we have long idle time, we try to reduce the memory footprint. // (4) If incremental marking is done, we perform a full garbage collection
// (5) If incremental marking is done, we perform a full garbage collection
// if we are allowed to still do full garbage collections during this idle // if we are allowed to still do full garbage collections during this idle
// round or if we are not allowed to start incremental marking. Otherwise we // round or if we are not allowed to start incremental marking. Otherwise we
// do not perform garbage collection to keep system utilization low. // do not perform garbage collection to keep system utilization low.
// (6) If sweeping is in progress and we received a large enough idle time // (5) If sweeping is in progress and we received a large enough idle time
// request, we finalize sweeping here. // request, we finalize sweeping here.
// (7) If incremental marking is in progress, we perform a marking step. Note, // (6) If incremental marking is in progress, we perform a marking step. Note,
// that this currently may trigger a full garbage collection. // that this currently may trigger a full garbage collection.
GCIdleTimeAction GCIdleTimeHandler::Compute(double idle_time_in_ms, GCIdleTimeAction GCIdleTimeHandler::Compute(double idle_time_in_ms,
HeapState heap_state) { HeapState heap_state) {
...@@ -256,10 +247,6 @@ GCIdleTimeAction GCIdleTimeHandler::Compute(double idle_time_in_ms, ...@@ -256,10 +247,6 @@ GCIdleTimeAction GCIdleTimeHandler::Compute(double idle_time_in_ms,
} }
} }
if (ShouldDoReduceMemoryMarkCompact(static_cast<size_t>(idle_time_in_ms))) {
return GCIdleTimeAction::FullGCCompact();
}
if (heap_state.incremental_marking_stopped) { if (heap_state.incremental_marking_stopped) {
if (ShouldDoMarkCompact(static_cast<size_t>(idle_time_in_ms), if (ShouldDoMarkCompact(static_cast<size_t>(idle_time_in_ms),
heap_state.size_of_objects, heap_state.size_of_objects,
......
...@@ -16,7 +16,6 @@ enum GCIdleTimeActionType { ...@@ -16,7 +16,6 @@ enum GCIdleTimeActionType {
DO_INCREMENTAL_MARKING, DO_INCREMENTAL_MARKING,
DO_SCAVENGE, DO_SCAVENGE,
DO_FULL_GC, DO_FULL_GC,
DO_FULL_GC_COMPACT,
DO_FINALIZE_SWEEPING DO_FINALIZE_SWEEPING
}; };
...@@ -63,14 +62,6 @@ class GCIdleTimeAction { ...@@ -63,14 +62,6 @@ class GCIdleTimeAction {
return result; return result;
} }
static GCIdleTimeAction FullGCCompact() {
GCIdleTimeAction result;
result.type = DO_FULL_GC_COMPACT;
result.parameter = 0;
result.additional_work = false;
return result;
}
static GCIdleTimeAction FinalizeSweeping() { static GCIdleTimeAction FinalizeSweeping() {
GCIdleTimeAction result; GCIdleTimeAction result;
result.type = DO_FINALIZE_SWEEPING; result.type = DO_FINALIZE_SWEEPING;
...@@ -131,10 +122,6 @@ class GCIdleTimeHandler { ...@@ -131,10 +122,6 @@ class GCIdleTimeHandler {
// 16 ms when there is currently no rendering going on. // 16 ms when there is currently no rendering going on.
static const size_t kMaxScheduledIdleTime = 50; static const size_t kMaxScheduledIdleTime = 50;
// This is the minimum time needed to trigger a full garbage collection which
// tries to reduce memory footprint.
static const size_t kMinTimeForReduceMemory = 600;
// We conservatively assume that in the next kTimeUntilNextIdleEvent ms // We conservatively assume that in the next kTimeUntilNextIdleEvent ms
// no idle notification happens. // no idle notification happens.
static const size_t kTimeUntilNextIdleEvent = 100; static const size_t kTimeUntilNextIdleEvent = 100;
...@@ -208,8 +195,6 @@ class GCIdleTimeHandler { ...@@ -208,8 +195,6 @@ class GCIdleTimeHandler {
size_t size_of_objects, size_t size_of_objects,
size_t mark_compact_speed_in_bytes_per_ms); size_t mark_compact_speed_in_bytes_per_ms);
static bool ShouldDoReduceMemoryMarkCompact(size_t idle_time_in_ms);
static bool ShouldDoContextDisposalMarkCompact(int context_disposed, static bool ShouldDoContextDisposalMarkCompact(int context_disposed,
double contexts_disposal_rate); double contexts_disposal_rate);
......
...@@ -4502,15 +4502,14 @@ void Heap::MakeHeapIterable() { ...@@ -4502,15 +4502,14 @@ void Heap::MakeHeapIterable() {
} }
void Heap::IdleMarkCompact(bool reduce_memory, const char* message) { void Heap::IdleMarkCompact(const char* message) {
bool uncommit = false; bool uncommit = false;
if (gc_count_at_last_idle_gc_ == gc_count_) { if (gc_count_at_last_idle_gc_ == gc_count_) {
// No GC since the last full GC, the mutator is probably not active. // No GC since the last full GC, the mutator is probably not active.
isolate_->compilation_cache()->Clear(); isolate_->compilation_cache()->Clear();
uncommit = true; uncommit = true;
} }
int flags = reduce_memory ? kReduceMemoryFootprintMask : kNoGCFlags; CollectAllGarbage(kReduceMemoryFootprintMask, message);
CollectAllGarbage(flags, message);
gc_idle_time_handler_.NotifyIdleMarkCompact(); gc_idle_time_handler_.NotifyIdleMarkCompact();
gc_count_at_last_idle_gc_ = gc_count_; gc_count_at_last_idle_gc_ = gc_count_;
if (uncommit) { if (uncommit) {
...@@ -4642,14 +4641,10 @@ bool Heap::IdleNotification(double deadline_in_seconds) { ...@@ -4642,14 +4641,10 @@ bool Heap::IdleNotification(double deadline_in_seconds) {
gc_idle_time_handler_.NotifyIdleMarkCompact(); gc_idle_time_handler_.NotifyIdleMarkCompact();
gc_count_at_last_idle_gc_ = gc_count_; gc_count_at_last_idle_gc_ = gc_count_;
} else { } else {
IdleMarkCompact(false, "idle notification: finalize idle round"); IdleMarkCompact("idle notification: finalize idle round");
} }
break; break;
} }
case DO_FULL_GC_COMPACT: {
IdleMarkCompact(true, "idle notification: reduce memory footprint");
break;
}
case DO_SCAVENGE: case DO_SCAVENGE:
CollectGarbage(NEW_SPACE, "idle notification: scavenge"); CollectGarbage(NEW_SPACE, "idle notification: scavenge");
break; break;
......
...@@ -2049,7 +2049,7 @@ class Heap { ...@@ -2049,7 +2049,7 @@ class Heap {
void SelectScavengingVisitorsTable(); void SelectScavengingVisitorsTable();
void IdleMarkCompact(bool reduce_memory, const char* message); void IdleMarkCompact(const char* message);
bool TryFinalizeIdleIncrementalMarking( bool TryFinalizeIdleIncrementalMarking(
double idle_time_in_ms, size_t size_of_objects, double idle_time_in_ms, size_t size_of_objects,
......
...@@ -516,18 +516,5 @@ TEST_F(GCIdleTimeHandlerTest, DoneIfNotMakingProgressOnIncrementalMarking) { ...@@ -516,18 +516,5 @@ TEST_F(GCIdleTimeHandlerTest, DoneIfNotMakingProgressOnIncrementalMarking) {
EXPECT_EQ(DONE, action.type); EXPECT_EQ(DONE, action.type);
} }
TEST_F(GCIdleTimeHandlerTest, ReduceMemory) {
GCIdleTimeHandler::HeapState heap_state = DefaultHeapState();
double idle_time = GCIdleTimeHandler::kMinTimeForReduceMemory;
for (int i = 0; i < GCIdleTimeHandler::kMaxMarkCompactsInIdleRound; i++) {
GCIdleTimeAction action = handler()->Compute(idle_time, heap_state);
EXPECT_EQ(DO_FULL_GC_COMPACT, action.type);
handler()->NotifyIdleMarkCompact();
}
GCIdleTimeAction action = handler()->Compute(idle_time, heap_state);
EXPECT_EQ(DONE, action.type);
}
} // namespace internal } // namespace internal
} // namespace v8 } // namespace v8
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