Commit a47417b8 authored by mlippautz's avatar mlippautz Committed by Commit bot

[heap] Ensure progress when incrementally marking wrappers

The problem here is estimating the marking step size for wrapper tracing. If the
steps are too small, we cannot keep up with the mutator creating new wrappers.
The result is an endless stream of incremental marking steps, alternating v8 and
wrappers tracing, without ever finalizing in a GC.

The mitigation here is to abort finding the fix point after 10 incremental
iterations.

A proper solution would track newly created wrappers on the blink side during
wrapper tracing. Will give this more thought after the holidays.

BUG=chromium:668164, chromium:468240

Review-Url: https://codereview.chromium.org/2592403002
Cr-Commit-Position: refs/heads/master@{#41923}
parent 8435cc85
...@@ -13,6 +13,8 @@ void LocalEmbedderHeapTracer::TracePrologue() { ...@@ -13,6 +13,8 @@ void LocalEmbedderHeapTracer::TracePrologue() {
if (!InUse()) return; if (!InUse()) return;
CHECK(cached_wrappers_to_trace_.empty()); CHECK(cached_wrappers_to_trace_.empty());
num_v8_marking_deque_was_empty_ = 0;
in_final_pause_ = false;
remote_tracer_->TracePrologue(); remote_tracer_->TracePrologue();
} }
...@@ -33,6 +35,7 @@ void LocalEmbedderHeapTracer::AbortTracing() { ...@@ -33,6 +35,7 @@ void LocalEmbedderHeapTracer::AbortTracing() {
void LocalEmbedderHeapTracer::EnterFinalPause() { void LocalEmbedderHeapTracer::EnterFinalPause() {
if (!InUse()) return; if (!InUse()) return;
in_final_pause_ = true;
remote_tracer_->EnterFinalPause(); remote_tracer_->EnterFinalPause();
} }
...@@ -41,7 +44,10 @@ bool LocalEmbedderHeapTracer::Trace( ...@@ -41,7 +44,10 @@ bool LocalEmbedderHeapTracer::Trace(
if (!InUse()) return false; if (!InUse()) return false;
RegisterWrappersWithRemoteTracer(); RegisterWrappersWithRemoteTracer();
return remote_tracer_->AdvanceTracing(deadline, actions); return (in_final_pause_ ||
(num_v8_marking_deque_was_empty_ <= kMaxIncrementalMarkingRounds))
? remote_tracer_->AdvanceTracing(deadline, actions)
: false;
} }
size_t LocalEmbedderHeapTracer::NumberOfWrappersToTrace() { size_t LocalEmbedderHeapTracer::NumberOfWrappersToTrace() {
......
...@@ -17,7 +17,10 @@ class V8_EXPORT_PRIVATE LocalEmbedderHeapTracer final { ...@@ -17,7 +17,10 @@ class V8_EXPORT_PRIVATE LocalEmbedderHeapTracer final {
public: public:
typedef std::pair<void*, void*> WrapperInfo; typedef std::pair<void*, void*> WrapperInfo;
LocalEmbedderHeapTracer() : remote_tracer_(nullptr) {} LocalEmbedderHeapTracer()
: remote_tracer_(nullptr),
num_v8_marking_deque_was_empty_(0),
in_final_pause_(false) {}
void SetRemoteTracer(EmbedderHeapTracer* tracer) { remote_tracer_ = tracer; } void SetRemoteTracer(EmbedderHeapTracer* tracer) { remote_tracer_ = tracer; }
bool InUse() { return remote_tracer_ != nullptr; } bool InUse() { return remote_tracer_ != nullptr; }
...@@ -43,11 +46,17 @@ class V8_EXPORT_PRIVATE LocalEmbedderHeapTracer final { ...@@ -43,11 +46,17 @@ class V8_EXPORT_PRIVATE LocalEmbedderHeapTracer final {
// are too many of them. // are too many of them.
bool RequiresImmediateWrapperProcessing(); bool RequiresImmediateWrapperProcessing();
void NotifyV8MarkingDequeWasEmpty() { num_v8_marking_deque_was_empty_++; }
private: private:
typedef std::vector<WrapperInfo> WrapperCache; typedef std::vector<WrapperInfo> WrapperCache;
static const size_t kMaxIncrementalMarkingRounds = 10;
EmbedderHeapTracer* remote_tracer_; EmbedderHeapTracer* remote_tracer_;
WrapperCache cached_wrappers_to_trace_; WrapperCache cached_wrappers_to_trace_;
size_t num_v8_marking_deque_was_empty_;
bool in_final_pause_;
}; };
} // namespace internal } // namespace internal
......
...@@ -1137,11 +1137,12 @@ size_t IncrementalMarking::Step(size_t bytes_to_process, ...@@ -1137,11 +1137,12 @@ size_t IncrementalMarking::Step(size_t bytes_to_process,
const bool incremental_wrapper_tracing = const bool incremental_wrapper_tracing =
FLAG_incremental_marking_wrappers && FLAG_incremental_marking_wrappers &&
heap_->local_embedder_heap_tracer()->InUse(); heap_->local_embedder_heap_tracer()->InUse();
const bool process_wrappers = const bool v8_marking_deque_empty =
incremental_wrapper_tracing && heap_->mark_compact_collector()->marking_deque()->IsEmpty();
(heap_->local_embedder_heap_tracer() const bool process_wrappers = incremental_wrapper_tracing &&
->RequiresImmediateWrapperProcessing() || (heap_->local_embedder_heap_tracer()
heap_->mark_compact_collector()->marking_deque()->IsEmpty()); ->RequiresImmediateWrapperProcessing() ||
v8_marking_deque_empty);
bool wrapper_work_left = incremental_wrapper_tracing; bool wrapper_work_left = incremental_wrapper_tracing;
if (!process_wrappers) { if (!process_wrappers) {
bytes_processed = ProcessMarkingDeque(bytes_to_process); bytes_processed = ProcessMarkingDeque(bytes_to_process);
...@@ -1153,6 +1154,8 @@ size_t IncrementalMarking::Step(size_t bytes_to_process, ...@@ -1153,6 +1154,8 @@ size_t IncrementalMarking::Step(size_t bytes_to_process,
heap_->MonotonicallyIncreasingTimeInMs() + kStepSizeInMs; heap_->MonotonicallyIncreasingTimeInMs() + kStepSizeInMs;
TRACE_GC(heap()->tracer(), TRACE_GC(heap()->tracer(),
GCTracer::Scope::MC_INCREMENTAL_WRAPPER_TRACING); GCTracer::Scope::MC_INCREMENTAL_WRAPPER_TRACING);
if (v8_marking_deque_empty)
heap_->local_embedder_heap_tracer()->NotifyV8MarkingDequeWasEmpty();
wrapper_work_left = heap_->local_embedder_heap_tracer()->Trace( wrapper_work_left = heap_->local_embedder_heap_tracer()->Trace(
wrapper_deadline, EmbedderHeapTracer::AdvanceTracingActions( wrapper_deadline, EmbedderHeapTracer::AdvanceTracingActions(
EmbedderHeapTracer::ForceCompletionAction:: EmbedderHeapTracer::ForceCompletionAction::
......
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