From 75c5d82be693ed45e5143af0a0752548012e0578 Mon Sep 17 00:00:00 2001 From: Jakob Gruber Date: Thu, 15 Apr 2021 07:40:26 +0200 Subject: [PATCH] [compiler] Spawn concurrent jobs for --stress-concurrent-inlining .. to increase coverage of concurrent inlining, at least in this stress mode. The common pattern in mjsunit tests is to call `%OptimizeFunctionOnNextCall(f)` for interesting function `f`. This explicitly triggers non-concurrent compilation, significantly decreasing relevant coverage of concurrent inlining. This CL recovers coverage by spawning an additional concurrent compile job when 1. --stress-concurrent-inlining is enabled, and 2. the requested compile mode is non-concurrent. The result of these additional jobs is discarded. Drive-by: Fix two simple uncovered issues. Bug: v8:7790,v8:11513,v8:11648 Change-Id: If1e8ca5ba737e3cecdec9e15e4a86b28fe9fb2de Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2824440 Commit-Queue: Jakob Gruber Reviewed-by: Ross McIlroy Cr-Commit-Position: refs/heads/master@{#73967} --- src/codegen/compiler.cc | 76 +++++++++++++++---- src/codegen/optimized-compilation-info.h | 3 +- .../optimizing-compile-dispatcher.cc | 1 + src/compiler/node-observer.h | 2 +- test/cctest/cctest.status | 4 + 5 files changed, 69 insertions(+), 17 deletions(-) diff --git a/src/codegen/compiler.cc b/src/codegen/compiler.cc index 71d09fb776..e7c2294036 100644 --- a/src/codegen/compiler.cc +++ b/src/codegen/compiler.cc @@ -1177,10 +1177,19 @@ Handle ContinuationForConcurrentOptimization( return BUILTIN_CODE(isolate, InterpreterEntryTrampoline); } +enum class GetOptimizedCodeResultHandling { + // Default behavior, i.e. install the result, insert into caches, etc. + kDefault, + // Used only for stress testing. The compilation result should be discarded. + kDiscardForTesting, +}; + MaybeHandle GetOptimizedCode( Handle function, ConcurrencyMode mode, CodeKind code_kind, BytecodeOffset osr_offset = BytecodeOffset::None(), - JavaScriptFrame* osr_frame = nullptr) { + JavaScriptFrame* osr_frame = nullptr, + GetOptimizedCodeResultHandling result_handling = + GetOptimizedCodeResultHandling::kDefault) { DCHECK(CodeKindIsOptimizedJSFunction(code_kind)); Isolate* isolate = function->GetIsolate(); @@ -1262,6 +1271,10 @@ MaybeHandle GetOptimizedCode( has_script, osr_offset, osr_frame)); OptimizedCompilationInfo* compilation_info = job->compilation_info(); + if (result_handling == GetOptimizedCodeResultHandling::kDiscardForTesting) { + compilation_info->set_discard_result_for_testing(); + } + // Prepare the job and launch concurrent compilation, or compile now. if (mode == ConcurrencyMode::kConcurrent) { if (GetOptimizedCodeLater(std::move(job), isolate, compilation_info, @@ -1280,6 +1293,20 @@ MaybeHandle GetOptimizedCode( return {}; } +// When --stress-concurrent-inlining is enabled, spawn concurrent jobs in +// addition to non-concurrent compiles to increase coverage in mjsunit tests +// (where most interesting compiles are non-concurrent). The result of the +// compilation is thrown out. +void SpawnDuplicateConcurrentJobForStressTesting(Handle function, + ConcurrencyMode mode, + CodeKind code_kind) { + DCHECK(FLAG_stress_concurrent_inlining && FLAG_concurrent_recompilation && + mode == ConcurrencyMode::kNotConcurrent); + USE(GetOptimizedCode(function, ConcurrencyMode::kConcurrent, code_kind, + BytecodeOffset::None(), nullptr, + GetOptimizedCodeResultHandling::kDiscardForTesting)); +} + bool FailAndClearPendingException(Isolate* isolate) { isolate->clear_pending_exception(); return false; @@ -1552,6 +1579,7 @@ void CompileOnBackgroundThread(ParseInfo* parse_info, // Character stream shouldn't be used again. parse_info->ResetCharacterStream(); } + } // namespace CompilationHandleScope::~CompilationHandleScope() { @@ -1992,9 +2020,17 @@ bool Compiler::Compile(Isolate* isolate, Handle function, CompilerTracer::TraceOptimizeForAlwaysOpt(isolate, function, CodeKindForTopTier()); + const CodeKind code_kind = CodeKindForTopTier(); + const ConcurrencyMode concurrency_mode = ConcurrencyMode::kNotConcurrent; + + if (FLAG_stress_concurrent_inlining && FLAG_concurrent_recompilation && + concurrency_mode == ConcurrencyMode::kNotConcurrent) { + SpawnDuplicateConcurrentJobForStressTesting(function, concurrency_mode, + code_kind); + } + Handle maybe_code; - if (GetOptimizedCode(function, ConcurrencyMode::kNotConcurrent, - CodeKindForTopTier()) + if (GetOptimizedCode(function, concurrency_mode, code_kind) .ToHandle(&maybe_code)) { code = maybe_code; } @@ -2091,6 +2127,11 @@ bool Compiler::CompileOptimized(Isolate* isolate, Handle function, DCHECK(CodeKindIsOptimizedJSFunction(code_kind)); DCHECK(AllowCompilation::IsAllowed(isolate)); + if (FLAG_stress_concurrent_inlining && FLAG_concurrent_recompilation && + mode == ConcurrencyMode::kNotConcurrent) { + SpawnDuplicateConcurrentJobForStressTesting(function, mode, code_kind); + } + Handle code; if (!GetOptimizedCode(function, mode, code_kind).ToHandle(&code)) { // Optimization failed, get the existing code. We could have optimized code @@ -3226,9 +3267,10 @@ bool Compiler::FinalizeOptimizedCompilationJob(OptimizedCompilationJob* job, Handle shared = compilation_info->shared_info(); CodeKind code_kind = compilation_info->code_kind(); + const bool use_result = !compilation_info->discard_result_for_testing(); const bool should_install_code_on_function = !CodeKindIsNativeContextIndependentJSFunction(code_kind); - if (should_install_code_on_function) { + if (V8_LIKELY(should_install_code_on_function && use_result)) { // Reset profiler ticks, function is no longer considered hot. compilation_info->closure()->feedback_vector().set_profiler_ticks(0); } @@ -3248,12 +3290,14 @@ bool Compiler::FinalizeOptimizedCompilationJob(OptimizedCompilationJob* job, isolate); job->RecordFunctionCompilation(CodeEventListener::LAZY_COMPILE_TAG, isolate); - InsertCodeIntoOptimizedCodeCache(compilation_info); - InsertCodeIntoCompilationCache(isolate, compilation_info); - CompilerTracer::TraceCompletedJob(isolate, compilation_info); - if (should_install_code_on_function) { - compilation_info->closure()->set_code(*compilation_info->code(), - kReleaseStore); + if (V8_LIKELY(use_result)) { + InsertCodeIntoOptimizedCodeCache(compilation_info); + InsertCodeIntoCompilationCache(isolate, compilation_info); + CompilerTracer::TraceCompletedJob(isolate, compilation_info); + if (should_install_code_on_function) { + compilation_info->closure()->set_code(*compilation_info->code(), + kReleaseStore); + } } return CompilationJob::SUCCEEDED; } @@ -3261,11 +3305,13 @@ bool Compiler::FinalizeOptimizedCompilationJob(OptimizedCompilationJob* job, DCHECK_EQ(job->state(), CompilationJob::State::kFailed); CompilerTracer::TraceAbortedJob(isolate, compilation_info); - compilation_info->closure()->set_code(shared->GetCode(), kReleaseStore); - // Clear the InOptimizationQueue marker, if it exists. - if (!CodeKindIsNativeContextIndependentJSFunction(code_kind) && - compilation_info->closure()->IsInOptimizationQueue()) { - compilation_info->closure()->ClearOptimizationMarker(); + if (V8_LIKELY(use_result)) { + compilation_info->closure()->set_code(shared->GetCode(), kReleaseStore); + // Clear the InOptimizationQueue marker, if it exists. + if (!CodeKindIsNativeContextIndependentJSFunction(code_kind) && + compilation_info->closure()->IsInOptimizationQueue()) { + compilation_info->closure()->ClearOptimizationMarker(); + } } return CompilationJob::FAILED; } diff --git a/src/codegen/optimized-compilation-info.h b/src/codegen/optimized-compilation-info.h index 20386cbbee..798d615467 100644 --- a/src/codegen/optimized-compilation-info.h +++ b/src/codegen/optimized-compilation-info.h @@ -70,7 +70,8 @@ class V8_EXPORT_PRIVATE OptimizedCompilationInfo final { V(TraceTurboAllocation, trace_turbo_allocation, 16) \ V(TraceHeapBroker, trace_heap_broker, 17) \ V(WasmRuntimeExceptionSupport, wasm_runtime_exception_support, 18) \ - V(ConcurrentInlining, concurrent_inlining, 19) + V(ConcurrentInlining, concurrent_inlining, 19) \ + V(DiscardResultForTesting, discard_result_for_testing, 20) enum Flag { #define DEF_ENUM(Camel, Lower, Bit) k##Camel = 1 << Bit, diff --git a/src/compiler-dispatcher/optimizing-compile-dispatcher.cc b/src/compiler-dispatcher/optimizing-compile-dispatcher.cc index ec6e38b365..52f1409a28 100644 --- a/src/compiler-dispatcher/optimizing-compile-dispatcher.cc +++ b/src/compiler-dispatcher/optimizing-compile-dispatcher.cc @@ -185,6 +185,7 @@ void OptimizingCompileDispatcher::Flush(BlockingBehavior blocking_behavior) { } void OptimizingCompileDispatcher::Stop() { + HandleScope handle_scope(isolate_); FlushQueues(BlockingBehavior::kBlock, false); // At this point the optimizing compiler thread's event loop has stopped. // There is no need for a mutex when reading input_queue_length_. diff --git a/src/compiler/node-observer.h b/src/compiler/node-observer.h index 8978156464..a6c4619262 100644 --- a/src/compiler/node-observer.h +++ b/src/compiler/node-observer.h @@ -80,7 +80,7 @@ class NodeObserver : public ZoneObject { bool has_observed_changes() const { return has_observed_changes_; } private: - bool has_observed_changes_ = false; + std::atomic has_observed_changes_{false}; }; inline NodeObserver::~NodeObserver() = default; diff --git a/test/cctest/cctest.status b/test/cctest/cctest.status index 24bcb27a58..35512fe01d 100644 --- a/test/cctest/cctest.status +++ b/test/cctest/cctest.status @@ -707,6 +707,10 @@ ['variant == stress_concurrent_inlining', { # BUG(11524): Crashing flakily. 'test-cpu-profiler/TracingCpuProfiler': [PASS, FAIL], + # crbug.com/v8/11513: Flakily failing due to the additional compile task. + 'test-heap/EnsureAllocationSiteDependentCodesProcessed': [PASS, FAIL], + 'test-heap/LeakNativeContextViaMapProto': [PASS, FAIL], + 'test-heap/ObjectsInEagerlyDeoptimizedCodeAreWeak': [PASS, FAIL], }], # variant == stress_concurrent_inlining ]