From 6d9b7988f7786f83dc6f84b85326161a29a740cb Mon Sep 17 00:00:00 2001 From: Sigurd Schneider Date: Thu, 22 Aug 2019 13:22:40 +0200 Subject: [PATCH] [testing] Prevent heuristics from triggering optimization in tests This CL adds a mechanism that prevents the RuntimeProfiler from triggering optimization of a function after %PrepareFunctionForOptimization has been called. This is useful to prevent flakiness in tests, as sometimes a function that already got deoptimized would receive a new code object from a concurrent compile that was triggered by a heuristic just in the right moment for the assertUnoptimized test to fail. For example, the following was happening: PrepareFunctionForOptimization [marking `testAdd` for optimized recompilation, reason: small function] [concurrently compiling method `testAdd` using TurboFan] [manually marking `testAdd` for non-concurrent optimization] [synchonously compiling method `testAdd` using TurboFan] [synchonously optimizing `testAdd` produced code object 0xAAAA - took 1.638 ms] Runtime_GetOptimizationStatus OPTIMIZED `testAdd` (code object 0xAAAA) DeoptimizeFunction `testAdd` with Code Object 0xAAAA [concurrently optimizing `testAdd` produced code object 0xBBBB - took 3.377 ms] Runtime_GetOptimizationStatus OPTIMIZED `testAdd` (code object 0xBBBB) Bug: v8:9563 Change-Id: Ia4c846aba95281589317d43b82383e70fe0a35f5 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1763546 Reviewed-by: Ross McIlroy Reviewed-by: Yang Guo Commit-Queue: Sigurd Schneider Cr-Commit-Position: refs/heads/master@{#63343} --- src/codegen/pending-optimization-table.cc | 50 +++++++++++++++++++---- src/codegen/pending-optimization-table.h | 9 +++- src/execution/runtime-profiler.cc | 12 ++++++ src/runtime/runtime-test.cc | 17 +++++++- src/runtime/runtime.h | 2 +- test/mjsunit/interrupt-budget-override.js | 2 +- 6 files changed, 79 insertions(+), 13 deletions(-) diff --git a/src/codegen/pending-optimization-table.cc b/src/codegen/pending-optimization-table.cc index 9e33de7918..b7be9c7775 100644 --- a/src/codegen/pending-optimization-table.cc +++ b/src/codegen/pending-optimization-table.cc @@ -4,6 +4,7 @@ #include "src/codegen/pending-optimization-table.h" +#include "src/base/flags.h" #include "src/execution/isolate-inl.h" #include "src/heap/heap-inl.h" #include "src/objects/hash-table.h" @@ -12,12 +13,24 @@ namespace v8 { namespace internal { -enum class FunctionStatus { kPrepareForOptimize, kMarkForOptimize }; +enum class FunctionStatus : int { + kPrepareForOptimize = 1 << 0, + kMarkForOptimize = 1 << 1, + kAllowHeuristicOptimization = 1 << 2, +}; + +using FunctionStatusFlags = base::Flags; void PendingOptimizationTable::PreparedForOptimization( - Isolate* isolate, Handle function) { + Isolate* isolate, Handle function, + bool allow_heuristic_optimization) { DCHECK(FLAG_testing_d8_test_runner); + FunctionStatusFlags status = FunctionStatus::kPrepareForOptimize; + if (allow_heuristic_optimization) { + status |= FunctionStatus::kAllowHeuristicOptimization; + } + Handle table = isolate->heap()->pending_optimize_for_test_bytecode().IsUndefined() ? ObjectHashTable::New(isolate, 1) @@ -26,15 +39,33 @@ void PendingOptimizationTable::PreparedForOptimization( isolate); Handle tuple = isolate->factory()->NewTuple2( handle(function->shared().GetBytecodeArray(), isolate), - handle( - Smi::FromInt(static_cast(FunctionStatus::kPrepareForOptimize)), - isolate), - AllocationType::kYoung); + handle(Smi::FromInt(status), isolate), AllocationType::kYoung); table = ObjectHashTable::Put(table, handle(function->shared(), isolate), tuple); isolate->heap()->SetPendingOptimizeForTestBytecode(*table); } +bool PendingOptimizationTable::IsHeuristicOptimizationAllowed( + Isolate* isolate, JSFunction function) { + DCHECK(FLAG_testing_d8_test_runner); + + Handle table = + handle(isolate->heap()->pending_optimize_for_test_bytecode(), isolate); + Handle entry = + table->IsUndefined() + ? handle(ReadOnlyRoots(isolate).the_hole_value(), isolate) + : handle(Handle::cast(table)->Lookup( + handle(function.shared(), isolate)), + isolate); + if (entry->IsTheHole()) { + return true; + } + DCHECK(entry->IsTuple2()); + DCHECK(Handle::cast(entry)->value2().IsSmi()); + FunctionStatusFlags status(Smi::ToInt(Handle::cast(entry)->value2())); + return status & FunctionStatus::kAllowHeuristicOptimization; +} + void PendingOptimizationTable::MarkedForOptimization( Isolate* isolate, Handle function) { DCHECK(FLAG_testing_d8_test_runner); @@ -58,8 +89,11 @@ void PendingOptimizationTable::MarkedForOptimization( } DCHECK(entry->IsTuple2()); - Handle::cast(entry)->set_value2( - Smi::FromInt(static_cast(FunctionStatus::kMarkForOptimize))); + DCHECK(Handle::cast(entry)->value2().IsSmi()); + FunctionStatusFlags status(Smi::ToInt(Handle::cast(entry)->value2())); + status = status.without(FunctionStatus::kPrepareForOptimize) | + FunctionStatus::kMarkForOptimize; + Handle::cast(entry)->set_value2(Smi::FromInt(status)); table = ObjectHashTable::Put(Handle::cast(table), handle(function->shared(), isolate), entry); isolate->heap()->SetPendingOptimizeForTestBytecode(*table); diff --git a/src/codegen/pending-optimization-table.h b/src/codegen/pending-optimization-table.h index 2a2782d17a..43b939726d 100644 --- a/src/codegen/pending-optimization-table.h +++ b/src/codegen/pending-optimization-table.h @@ -21,7 +21,8 @@ class PendingOptimizationTable { // strongly in pending optimization table preventing the bytecode to be // flushed. static void PreparedForOptimization(Isolate* isolate, - Handle function); + Handle function, + bool allow_heuristic_optimization); // This function should be called when the function is marked for optimization // via the intrinsics. This will update the state of the bytecode array in the @@ -36,6 +37,12 @@ class PendingOptimizationTable { // then this function removes the entry from pending optimization table. static void FunctionWasOptimized(Isolate* isolate, Handle function); + + // This function returns whether a heuristic is allowed to trigger + // optimization the function. This mechanism is used in tests to prevent + // heuristics from interfering with manually triggered optimization. + static bool IsHeuristicOptimizationAllowed(Isolate* isolate, + JSFunction function); }; } // namespace internal diff --git a/src/execution/runtime-profiler.cc b/src/execution/runtime-profiler.cc index 0ed36cbe10..65476e346f 100644 --- a/src/execution/runtime-profiler.cc +++ b/src/execution/runtime-profiler.cc @@ -8,6 +8,7 @@ #include "src/codegen/assembler.h" #include "src/codegen/compilation-cache.h" #include "src/codegen/compiler.h" +#include "src/codegen/pending-optimization-table.h" #include "src/execution/execution.h" #include "src/execution/frames-inl.h" #include "src/handles/global-handles.h" @@ -119,6 +120,17 @@ void RuntimeProfiler::MaybeOptimize(JSFunction function, } return; } + if (FLAG_testing_d8_test_runner) { + if (!PendingOptimizationTable::IsHeuristicOptimizationAllowed(isolate_, + function)) { + if (FLAG_trace_opt_verbose) { + PrintF("[function "); + function.PrintName(); + PrintF(" has been marked manually for optimization]\n"); + } + return; + } + } if (FLAG_always_osr) { AttemptOnStackReplacement(frame, AbstractCode::kMaxLoopNestingMarker); diff --git a/src/runtime/runtime-test.cc b/src/runtime/runtime-test.cc index 819e94b6aa..f20d6ac5b8 100644 --- a/src/runtime/runtime-test.cc +++ b/src/runtime/runtime-test.cc @@ -349,12 +349,24 @@ RUNTIME_FUNCTION(Runtime_EnsureFeedbackVectorForFunction) { RUNTIME_FUNCTION(Runtime_PrepareFunctionForOptimization) { HandleScope scope(isolate); - DCHECK_EQ(1, args.length()); + DCHECK(args.length() == 1 || args.length() == 2); if (!args[0].IsJSFunction()) { return ReadOnlyRoots(isolate).undefined_value(); } CONVERT_ARG_HANDLE_CHECKED(JSFunction, function, 0); + bool allow_heuristic_optimization = false; + if (args.length() == 2) { + CONVERT_ARG_HANDLE_CHECKED(Object, sync_object, 1); + if (!sync_object->IsString()) + return ReadOnlyRoots(isolate).undefined_value(); + Handle sync = Handle::cast(sync_object); + if (sync->IsOneByteEqualTo( + StaticCharVector("allow heuristic optimization"))) { + allow_heuristic_optimization = true; + } + } + if (!EnsureFeedbackVector(function)) { return ReadOnlyRoots(isolate).undefined_value(); } @@ -375,7 +387,8 @@ RUNTIME_FUNCTION(Runtime_PrepareFunctionForOptimization) { // Hold onto the bytecode array between marking and optimization to ensure // it's not flushed. if (FLAG_testing_d8_test_runner) { - PendingOptimizationTable::PreparedForOptimization(isolate, function); + PendingOptimizationTable::PreparedForOptimization( + isolate, function, allow_heuristic_optimization); } return ReadOnlyRoots(isolate).undefined_value(); diff --git a/src/runtime/runtime.h b/src/runtime/runtime.h index 3e7609dbe3..bc4513c77a 100644 --- a/src/runtime/runtime.h +++ b/src/runtime/runtime.h @@ -502,7 +502,7 @@ namespace internal { F(NotifyContextDisposed, 0, 1) \ F(OptimizeFunctionOnNextCall, -1, 1) \ F(OptimizeOsr, -1, 1) \ - F(PrepareFunctionForOptimization, 1, 1) \ + F(PrepareFunctionForOptimization, -1, 1) \ F(PrintWithNameForAssert, 2, 1) \ F(RedirectToWasmInterpreter, 2, 1) \ F(RunningInSimulator, 0, 1) \ diff --git a/test/mjsunit/interrupt-budget-override.js b/test/mjsunit/interrupt-budget-override.js index 06d3005bbc..5f83b3ccc5 100644 --- a/test/mjsunit/interrupt-budget-override.js +++ b/test/mjsunit/interrupt-budget-override.js @@ -12,7 +12,7 @@ function f() { return s; } -%PrepareFunctionForOptimization(f); +%PrepareFunctionForOptimization(f, "allow heuristic optimization"); f(); f(); f();