[compiler][test] Give tests control over finalization

Some tests want to invalidate part of the VM state after an optimization
has consumed the old state but before the code is installed.

The existing mechanism for this is --block-concurrent-recompilation
and %UnblockConcurrentRecompilation(). The former suspends optimization
right after PrepareJob, before the background ExecuteJob phase. The
intrinsic can then be used to unblock it again.

This was good enough so far because the main "consume" work used to
happen on the main thread. With concurrent inlining this is no longer
true and we need something else.

This CL introduces three intrinsics:

%DisableOptimizationFinalization turns off automatic finalization of
background optimizations.

%FinalizeOptimization() can then be called at an appropriate time to
manually finalize (and thus install) the code and reenable automatic
finalization.

In case one wants to perform some action on the main thread after the
concurrent optimization has finished but before it is finalized, one can
do so with the help of %WaitForBackgroundOptimization() (see tests).

In a followup CL I'm removing the old mechanism since it now seems
redundant.

Bug: v8:12041, v8:7790
Change-Id: Ib7195789105922eb7e4bff86dc5bc11e96a4f97b
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3071400
Commit-Queue: Georg Neis <neis@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#76190}

src/compiler-dispatcher/optimizing-compile-dispatcher.cc

@@ -131,7 +131,7 @@ void OptimizingCompileDispatcher::CompileNext(OptimizedCompilationJob* job,
     output_queue_.push(job);
   }
 
-  isolate_->stack_guard()->RequestInstallCode();
+  if (finalize()) isolate_->stack_guard()->RequestInstallCode();
 }
 
 void OptimizingCompileDispatcher::FlushOutputQueue(bool restore_function_code) {
@@ -159,6 +159,18 @@ void OptimizingCompileDispatcher::FlushInputQueue() {
   }
 }
 
+void OptimizingCompileDispatcher::AwaitCompileTasks() {
+  {
+    base::MutexGuard lock_guard(&ref_count_mutex_);
+    while (ref_count_ > 0) ref_count_zero_.Wait(&ref_count_mutex_);
+  }
+
+#ifdef DEBUG
+  base::MutexGuard access_input_queue(&input_queue_mutex_);
+  CHECK_EQ(input_queue_length_, 0);
+#endif  // DEBUG
+}
+
 void OptimizingCompileDispatcher::FlushQueues(
     BlockingBehavior blocking_behavior, bool restore_function_code) {
   if (FLAG_block_concurrent_recompilation) Unblock();

src/compiler-dispatcher/optimizing-compile-dispatcher.h

@@ -43,6 +43,7 @@ class V8_EXPORT_PRIVATE OptimizingCompileDispatcher {
   // Takes ownership of |job|.
   void QueueForOptimization(OptimizedCompilationJob* job);
   void Unblock();
+  void AwaitCompileTasks();
   void InstallOptimizedFunctions();
 
   inline bool IsQueueAvailable() {
@@ -55,6 +56,16 @@ class V8_EXPORT_PRIVATE OptimizingCompileDispatcher {
   // This method must be called on the main thread.
   bool HasJobs();
 
+  // Whether to finalize and thus install the optimized code.  Defaults to true.
+  // Only set to false for testing (where finalization is then manually
+  // requested using %FinalizeOptimization).
+  bool finalize() const { return finalize_; }
+  void set_finalize(bool finalize) {
+    DCHECK_IMPLIES(!finalize, FLAG_testing_d8_test_runner);
+    CHECK(!HasJobs());
+    finalize_ = finalize;
+  }
+
  private:
   class CompileTask;
 
@@ -101,6 +112,8 @@ class V8_EXPORT_PRIVATE OptimizingCompileDispatcher {
   // Since flags might get modified while the background thread is running, it
   // is not safe to access them directly.
   int recompilation_delay_;
+
+  bool finalize_ = true;
 };
 }  // namespace internal
 }  // namespace v8

src/runtime/runtime-test.cc

@@ -678,6 +678,34 @@ RUNTIME_FUNCTION(Runtime_UnblockConcurrentRecompilation) {
   return ReadOnlyRoots(isolate).undefined_value();
 }
 
+RUNTIME_FUNCTION(Runtime_DisableOptimizationFinalization) {
+  DCHECK_EQ(0, args.length());
+  DCHECK(!FLAG_block_concurrent_recompilation);
+  CHECK(isolate->concurrent_recompilation_enabled());
+  isolate->optimizing_compile_dispatcher()->AwaitCompileTasks();
+  isolate->optimizing_compile_dispatcher()->InstallOptimizedFunctions();
+  isolate->optimizing_compile_dispatcher()->set_finalize(false);
+  return ReadOnlyRoots(isolate).undefined_value();
+}
+
+RUNTIME_FUNCTION(Runtime_WaitForBackgroundOptimization) {
+  DCHECK_EQ(0, args.length());
+  DCHECK(!FLAG_block_concurrent_recompilation);
+  CHECK(isolate->concurrent_recompilation_enabled());
+  isolate->optimizing_compile_dispatcher()->AwaitCompileTasks();
+  return ReadOnlyRoots(isolate).undefined_value();
+}
+
+RUNTIME_FUNCTION(Runtime_FinalizeOptimization) {
+  DCHECK_EQ(0, args.length());
+  DCHECK(!FLAG_block_concurrent_recompilation);
+  CHECK(isolate->concurrent_recompilation_enabled());
+  isolate->optimizing_compile_dispatcher()->AwaitCompileTasks();
+  isolate->optimizing_compile_dispatcher()->InstallOptimizedFunctions();
+  isolate->optimizing_compile_dispatcher()->set_finalize(true);
+  return ReadOnlyRoots(isolate).undefined_value();
+}
+
 static void ReturnNull(const v8::FunctionCallbackInfo<v8::Value>& args) {
   args.GetReturnValue().SetNull();
 }

src/runtime/runtime.h

@@ -463,8 +463,9 @@ namespace internal {
 
 #define FOR_EACH_INTRINSIC_TEST(F, I)         \
   F(Abort, 1, 1)                              \
-  F(AbortJS, 1, 1)                            \
   F(AbortCSAAssert, 1, 1)                     \
+  F(AbortJS, 1, 1)                            \
+  F(ArrayIteratorProtector, 0, 1)             \
   F(ArraySpeciesProtector, 0, 1)              \
   F(BaselineOsr, -1, 1)                       \
   F(ClearFunctionFeedback, 1, 1)              \
@@ -481,11 +482,10 @@ namespace internal {
   F(DisallowCodegenFromStrings, 1, 1)         \
   F(DisassembleFunction, 1, 1)                \
   F(DynamicCheckMapsEnabled, 0, 1)            \
-  F(IsTopTierTurboprop, 0, 1)                 \
-  F(IsMidTierTurboprop, 0, 1)                 \
-  F(IsAtomicsWaitAllowed, 0, 1)               \
   F(EnableCodeLoggingForTesting, 0, 1)        \
   F(EnsureFeedbackVectorForFunction, 1, 1)    \
+  F(DisableOptimizationFinalization, 0, 1)    \
+  F(FinalizeOptimization, 0, 1)               \
   F(GetCallable, 0, 1)                        \
   F(GetInitializerFunction, 1, 1)             \
   F(GetOptimizationStatus, -1, 1)             \
@@ -496,7 +496,6 @@ namespace internal {
   F(HasElementsInALargeObjectSpace, 1, 1)     \
   F(HasFastElements, 1, 1)                    \
   F(HasFastProperties, 1, 1)                  \
-  F(HasOwnConstDataProperty, 2, 1)            \
   F(HasFixedBigInt64Elements, 1, 1)           \
   F(HasFixedBigUint64Elements, 1, 1)          \
   F(HasFixedFloat32Elements, 1, 1)            \
@@ -510,6 +509,7 @@ namespace internal {
   F(HasFixedUint8Elements, 1, 1)              \
   F(HasHoleyElements, 1, 1)                   \
   F(HasObjectElements, 1, 1)                  \
+  F(HasOwnConstDataProperty, 2, 1)            \
   F(HasPackedElements, 1, 1)                  \
   F(HasSloppyArgumentsElements, 1, 1)         \
   F(HasSmiElements, 1, 1)                     \
@@ -519,45 +519,48 @@ namespace internal {
   F(ICsAreEnabled, 0, 1)                      \
   F(InLargeObjectSpace, 1, 1)                 \
   F(InYoungGeneration, 1, 1)                  \
+  F(Is64Bit, 0, 1)                            \
+  F(IsAtomicsWaitAllowed, 0, 1)               \
   F(IsBeingInterpreted, 0, 1)                 \
+  F(IsConcatSpreadableProtector, 0, 1)        \
   F(IsConcurrentRecompilationSupported, 0, 1) \
   F(IsDictPropertyConstTrackingEnabled, 0, 1) \
-  F(RegexpHasBytecode, 2, 1)                  \
-  F(RegexpHasNativeCode, 2, 1)                \
-  F(RegexpTypeTag, 1, 1)                      \
-  F(RegexpIsUnmodified, 1, 1)                 \
+  F(IsMidTierTurboprop, 0, 1)                 \
+  F(IsTopTierTurboprop, 0, 1)                 \
   F(MapIteratorProtector, 0, 1)               \
-  F(ArrayIteratorProtector, 0, 1)             \
   F(NeverOptimizeFunction, 1, 1)              \
+  F(NewRegExpWithBacktrackLimit, 3, 1)        \
   F(NotifyContextDisposed, 0, 1)              \
-  F(OptimizeFunctionOnNextCall, -1, 1)        \
-  F(TierupFunctionOnNextCall, -1, 1)          \
   F(OptimizeFunctionForTopTier, 1, 1)         \
+  F(OptimizeFunctionOnNextCall, -1, 1)        \
   F(OptimizeOsr, -1, 1)                       \
-  F(NewRegExpWithBacktrackLimit, 3, 1)        \
   F(PrepareFunctionForOptimization, -1, 1)    \
   F(PretenureAllocationSite, 1, 1)            \
   F(PrintWithNameForAssert, 2, 1)             \
+  F(PromiseSpeciesProtector, 0, 1)            \
+  F(RegexpHasBytecode, 2, 1)                  \
+  F(RegexpHasNativeCode, 2, 1)                \
+  F(RegexpIsUnmodified, 1, 1)                 \
+  F(RegExpSpeciesProtector, 0, 1)             \
+  F(RegexpTypeTag, 1, 1)                      \
   F(RunningInSimulator, 0, 1)                 \
   F(RuntimeEvaluateREPL, 1, 1)                \
+  F(ScheduleGCInStackCheck, 0, 1)             \
   F(SerializeDeserializeNow, 0, 1)            \
   F(SetAllocationTimeout, -1 /* 2 || 3 */, 1) \
   F(SetForceSlowPath, 1, 1)                   \
   F(SetIteratorProtector, 0, 1)               \
   F(SimulateNewspaceFull, 0, 1)               \
-  F(ScheduleGCInStackCheck, 0, 1)             \
   F(StringIteratorProtector, 0, 1)            \
   F(SystemBreak, 0, 1)                        \
+  F(TierupFunctionOnNextCall, -1, 1)          \
   F(TraceEnter, 0, 1)                         \
   F(TraceExit, 1, 1)                          \
   F(TurbofanStaticAssert, 1, 1)               \
   F(TypedArraySpeciesProtector, 0, 1)         \
   F(UnblockConcurrentRecompilation, 0, 1)     \
-  I(DeoptimizeNow, 0, 1)                      \
-  F(PromiseSpeciesProtector, 0, 1)            \
-  F(IsConcatSpreadableProtector, 0, 1)        \
-  F(RegExpSpeciesProtector, 0, 1)             \
-  F(Is64Bit, 0, 1)
+  F(WaitForBackgroundOptimization, 0, 1)      \
+  I(DeoptimizeNow, 0, 1)
 
 #define FOR_EACH_INTRINSIC_TYPEDARRAY(F, I)    \
   F(ArrayBufferDetach, 1, 1)                   \

test/mjsunit/compiler/concurrent-invalidate-transition-map.js

@@ -25,9 +25,7 @@
 // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
-// Flags: --allow-natives-syntax
-// Flags: --concurrent-recompilation --block-concurrent-recompilation
-// Flags: --no-always-opt
+// Flags: --allow-natives-syntax --concurrent-recompilation --no-always-opt
 
 if (!%IsConcurrentRecompilationSupported()) {
   print("Concurrent recompilation is disabled. Skipping this test.");
@@ -51,25 +49,25 @@ var obj1 = new_object();
 var obj2 = new_object();
 add_field(obj1);
 add_field(obj2);
+%DisableOptimizationFinalization();
 %OptimizeFunctionOnNextCall(add_field, "concurrent");
 
 var o = new_object();
 // Kick off recompilation.
 add_field(o);
 // Invalidate transition map after compile graph has been created.
+%WaitForBackgroundOptimization();
 o.c = 2.2;
-// In the mean time, concurrent recompiling is still blocked.
 assertUnoptimized(add_field, "no sync");
-// Let concurrent recompilation proceed.
-%UnblockConcurrentRecompilation();
 // Sync with background thread to conclude optimization that bailed out.
+%FinalizeOptimization();
 if (!%IsDictPropertyConstTrackingEnabled()) {
   // TODO(v8:11457) Currently, we cannot inline property stores if there is a
   // dictionary mode prototype on the prototype chain. Therefore, if
   // v8_dict_property_const_tracking is enabled, the optimized code only
   // contains a call to the IC handler and doesn't get invalidated when the
   // transition map changes.
-  assertUnoptimized(add_field, "sync");
+  assertUnoptimized(add_field);
 }
 // Clear type info for stress runs.
 %ClearFunctionFeedback(add_field);

test/mjsunit/compiler/concurrent-proto-change.js

@@ -25,9 +25,7 @@
 // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
-// Flags: --allow-natives-syntax --no-always-opt
-// Flags: --concurrent-recompilation --block-concurrent-recompilation
-// Flags: --no-always-opt
+// Flags: --allow-natives-syntax --no-always-opt --concurrent-recompilation
 
 if (!%IsConcurrentRecompilationSupported()) {
   print("Concurrent recompilation is disabled. Skipping this test.");
@@ -44,18 +42,17 @@ o.__proto__ = { __proto__: { bar: function() { return 1; } } };
 assertEquals(1, f(o));
 assertEquals(1, f(o));
 
-// Mark for concurrent optimization.
+%DisableOptimizationFinalization();
 %OptimizeFunctionOnNextCall(f, "concurrent");
 // Kick off recompilation.
 assertEquals(1, f(o));
 // Change the prototype chain after compile graph has been created.
+%WaitForBackgroundOptimization();
 o.__proto__.__proto__ = { bar: function() { return 2; } };
-// At this point, concurrent recompilation thread has not yet done its job.
 assertUnoptimized(f, "no sync");
-// Let the background thread proceed.
-%UnblockConcurrentRecompilation();
-// Optimization eventually bails out due to map dependency.
-assertUnoptimized(f, "sync");
+%FinalizeOptimization();
+// Optimization failed due to map dependency.
+assertUnoptimized(f);
 assertEquals(2, f(o));
-//Clear type info for stress runs.
+// Clear type info for stress runs.
 %ClearFunctionFeedback(f);

test/mjsunit/compiler/regress-905555-2.js

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// Flags: --allow-natives-syntax --block-concurrent-recompilation --noalways-opt
+// Flags: --allow-natives-syntax --noalways-opt
 
 global = 1;
 
@@ -13,12 +13,13 @@ function boom(value) {
 %PrepareFunctionForOptimization(boom);
 assertEquals(1, boom());
 assertEquals(1, boom());
+%DisableOptimizationFinalization();
 %OptimizeFunctionOnNextCall(boom, "concurrent");
 assertEquals(1, boom());
 
+%WaitForBackgroundOptimization();
 delete this.global;
-
-%UnblockConcurrentRecompilation();
+%FinalizeOptimization();
 
 // boom should be deoptimized because the global property cell has changed.
 assertUnoptimized(boom, "sync");

test/mjsunit/compiler/regress-905555.js

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// Flags: --allow-natives-syntax --block-concurrent-recompilation --noalways-opt
+// Flags: --allow-natives-syntax --noalways-opt
 
 global = 1;
 
@@ -13,14 +13,15 @@ function boom(value) {
 %PrepareFunctionForOptimization(boom);
 assertEquals(1, boom());
 assertEquals(1, boom());
+%DisableOptimizationFinalization();
 %OptimizeFunctionOnNextCall(boom, "concurrent");
 assertEquals(1, boom());
 
+%WaitForBackgroundOptimization();
 this.__defineGetter__("global", () => 42);
-
-%UnblockConcurrentRecompilation();
+%FinalizeOptimization();
 
 // boom should be deoptimized because the global property cell has changed.
-assertUnoptimized(boom, "sync");
+assertUnoptimized(boom);
 
 assertEquals(42, boom());

test/mjsunit/concurrent-initial-prototype-change-1.js

@@ -26,7 +26,7 @@
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 // Flags: --allow-natives-syntax
-// Flags: --concurrent-recompilation --block-concurrent-recompilation
+// Flags: --concurrent-recompilation
 // Flags: --nostress-opt --no-always-opt
 // Flags: --no-turboprop
 
@@ -52,20 +52,20 @@ assertEquals(0.5, f1(arr, 0));
 assertEquals(0.5, f1(arr, 0));
 
 // Optimized code of f1 depends on initial object and array maps.
+%DisableOptimizationFinalization();
 %OptimizeFunctionOnNextCall(f1, "concurrent");
 // Kick off recompilation. Note that the NoElements protector is read by the
 // compiler in the main-thread phase of compilation, i.e., before the store to
 // Object.prototype below.
 assertEquals(0.5, f1(arr, 0));
 // Invalidate current initial object map after compile graph has been created.
+%WaitForBackgroundOptimization();
 Object.prototype[1] = 1.5;
 assertEquals(2, f1(arr, 1));
-// Not yet optimized since concurrent recompilation is blocked.
 assertUnoptimized(f1, "no sync");
-// Let concurrent recompilation proceed.
-%UnblockConcurrentRecompilation();
 // Sync with background thread to conclude optimization, which bails out
 // due to map dependency.
+%FinalizeOptimization();
 assertUnoptimized(f1, "sync");
 // Clear type info for stress runs.
 %ClearFunctionFeedback(f1);

test/mjsunit/const-dict-tracking.js

@@ -3,7 +3,7 @@
 // found in the LICENSE file.
 //
 // Flags: --allow-natives-syntax --opt --no-always-opt
-// Flags: --no-stress-flush-code --block-concurrent-recompilation
+// Flags: --no-stress-flush-code
 //
 // Tests tracking of constness of properties stored in dictionary
 // mode prototypes.
@@ -712,14 +712,17 @@ function testbench(o, proto, update_proto, check_constness) {
 
   %PrepareFunctionForOptimization(read_length);
   assertEquals(1, read_length(o));
+  %DisableOptimizationFinalization();
   %OptimizeFunctionOnNextCall(read_length, "concurrent");
   assertEquals(1, read_length(o));
   assertUnoptimized(read_length, "no sync");
 
+  %WaitForBackgroundOptimization();
   var other_proto1 = [];
   Object.setPrototypeOf(proto2, other_proto1);
-  %UnblockConcurrentRecompilation();
-  assertUnoptimized(read_length, "sync");
+  %FinalizeOptimization();
+
+  assertUnoptimized(read_length);
   assertEquals(0, read_length(o));
 
   if (%IsDictPropertyConstTrackingEnabled()) {

test/mjsunit/mjsunit.status

@@ -209,15 +209,6 @@
   'interrupt-budget-override': [PASS,FAIL],
   'never-optimize': [PASS,FAIL],
 
-  # TODO(v8:12041): Reimplement an %UnblockConcurrentRecompilation pattern that
-  # works with concurrent compilation (i.e. without main-thread serialization).
-  'compiler/concurrent-invalidate-transition-map': [SKIP],
-  'compiler/concurrent-proto-change': [SKIP],
-  'compiler/regress-905555': [SKIP],
-  'compiler/regress-905555-2': [SKIP],
-  'concurrent-initial-prototype-change-1': [SKIP],
-  'const-dict-tracking': [SKIP],
-
   # TODO(v8:12031): Reimplement elements kinds transitions when concurrent
   # inlining.
   'compiler/call-with-arraylike-or-spread-4': [SKIP],