From 61b2335a9e54ce531e9692146f23c8f8edfa5a0b Mon Sep 17 00:00:00 2001 From: Sathya Gunasekaran Date: Sat, 9 Jan 2021 06:58:50 +0000 Subject: [PATCH] Revert "Disable bytecode flushing once we toggle coverage mode." This reverts commit 8aa6b15fa0e8114f165ea4b26dfa2bda7cf50ee7. Reason for revert: broke TSAN https://ci.chromium.org/ui/p/v8/builders/ci/V8%20Linux64%20TSAN%20-%20stress-incremental-marking/1497/overview Original change's description: > Disable bytecode flushing once we toggle coverage mode. > > Changing coverage mode generated different bytecode in some cases. > Hence it is not safe to flush bytecode once we toggle coverage mode. > > Bug: chromium:1147917 > Change-Id: I9e640aeaec664d3d4a4aaedf809c568e9ad924fc > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2615020 > Commit-Queue: Mythri Alle > Reviewed-by: Ross McIlroy > Cr-Commit-Position: refs/heads/master@{#71985} TBR=rmcilroy@chromium.org,mythria@chromium.org Change-Id: Id4c95da337e291437b7856e2fe7004e1e6405515 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: chromium:1147917 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2619402 Reviewed-by: Sathya Gunasekaran Commit-Queue: Sathya Gunasekaran Cr-Commit-Position: refs/heads/master@{#71989} --- src/debug/debug-coverage.cc | 4 ---- src/execution/isolate.h | 1 - src/heap/concurrent-marking.cc | 2 +- src/heap/heap-inl.h | 13 ------------- src/heap/heap.h | 9 ++++++++- src/heap/mark-compact.cc | 2 +- 6 files changed, 10 insertions(+), 21 deletions(-) diff --git a/src/debug/debug-coverage.cc b/src/debug/debug-coverage.cc index c4aaebf7c2..e352acc846 100644 --- a/src/debug/debug-coverage.cc +++ b/src/debug/debug-coverage.cc @@ -747,10 +747,6 @@ void Coverage::SelectMode(Isolate* isolate, debug::CoverageMode mode) { // generated for a function, which can interfere with lazy source positions, // so just force source position collection whenever there's such a change. isolate->CollectSourcePositionsForAllBytecodeArrays(); - // Changing the coverage mode changes the generated bytecode and hence it is - // not safe to flush bytecode. Set a flag here, so we can disable bytecode - // flushing. - isolate->set_disable_bytecode_flushing(true); } switch (mode) { diff --git a/src/execution/isolate.h b/src/execution/isolate.h index a269c7a735..9dbe897ffd 100644 --- a/src/execution/isolate.h +++ b/src/execution/isolate.h @@ -458,7 +458,6 @@ using DebugObjectCache = std::vector>; /* Current code coverage mode */ \ V(debug::CoverageMode, code_coverage_mode, debug::CoverageMode::kBestEffort) \ V(debug::TypeProfileMode, type_profile_mode, debug::TypeProfileMode::kNone) \ - V(bool, disable_bytecode_flushing, false) \ V(int, last_console_context_id, 0) \ V(v8_inspector::V8Inspector*, inspector, nullptr) \ V(bool, next_v8_call_is_safe_for_termination, false) \ diff --git a/src/heap/concurrent-marking.cc b/src/heap/concurrent-marking.cc index 5835d2c13f..03aefd67b9 100644 --- a/src/heap/concurrent-marking.cc +++ b/src/heap/concurrent-marking.cc @@ -409,7 +409,7 @@ void ConcurrentMarking::Run(JobDelegate* delegate, unsigned mark_compact_epoch, MarkingWorklists::Local local_marking_worklists(marking_worklists_); ConcurrentMarkingVisitor visitor( task_id, &local_marking_worklists, weak_objects_, heap_, - mark_compact_epoch, Heap::GetBytecodeFlushMode(heap_->isolate()), + mark_compact_epoch, Heap::GetBytecodeFlushMode(), heap_->local_embedder_heap_tracer()->InUse(), is_forced_gc, &task_state->memory_chunk_data); NativeContextInferrer& native_context_inferrer = diff --git a/src/heap/heap-inl.h b/src/heap/heap-inl.h index 8372dd518d..2e6f099688 100644 --- a/src/heap/heap-inl.h +++ b/src/heap/heap-inl.h @@ -72,19 +72,6 @@ Address AllocationResult::ToAddress() { return HeapObject::cast(object_).address(); } -// static -BytecodeFlushMode Heap::GetBytecodeFlushMode(Isolate* isolate) { - if (isolate->disable_bytecode_flushing()) { - return BytecodeFlushMode::kDoNotFlushBytecode; - } - if (FLAG_stress_flush_bytecode) { - return BytecodeFlushMode::kStressFlushBytecode; - } else if (FLAG_flush_bytecode) { - return BytecodeFlushMode::kFlushBytecode; - } - return BytecodeFlushMode::kDoNotFlushBytecode; -} - Isolate* Heap::isolate() { return reinterpret_cast( reinterpret_cast(this) - diff --git a/src/heap/heap.h b/src/heap/heap.h index 7dc9ef7d44..24b92e8bc8 100644 --- a/src/heap/heap.h +++ b/src/heap/heap.h @@ -445,7 +445,14 @@ class Heap { // Helper function to get the bytecode flushing mode based on the flags. This // is required because it is not safe to acess flags in concurrent marker. - static inline BytecodeFlushMode GetBytecodeFlushMode(Isolate* isolate); + static inline BytecodeFlushMode GetBytecodeFlushMode() { + if (FLAG_stress_flush_bytecode) { + return BytecodeFlushMode::kStressFlushBytecode; + } else if (FLAG_flush_bytecode) { + return BytecodeFlushMode::kFlushBytecode; + } + return BytecodeFlushMode::kDoNotFlushBytecode; + } static uintptr_t ZapValue() { return FLAG_clear_free_memory ? kClearedFreeMemoryValue : kZapValue; diff --git a/src/heap/mark-compact.cc b/src/heap/mark-compact.cc index 9d7df06378..0609c2ffc7 100644 --- a/src/heap/mark-compact.cc +++ b/src/heap/mark-compact.cc @@ -499,7 +499,7 @@ void MarkCompactCollector::StartMarking() { std::make_unique(marking_worklists()); marking_visitor_ = std::make_unique( marking_state(), local_marking_worklists(), weak_objects(), heap_, - epoch(), Heap::GetBytecodeFlushMode(heap()->isolate()), + epoch(), Heap::GetBytecodeFlushMode(), heap_->local_embedder_heap_tracer()->InUse(), heap_->is_current_gc_forced()); // Marking bits are cleared by the sweeper.