[debug] Ensure breaking on inlined builtins works

This ensures that breaking on inlined builtins works, even when
compiling concurrently. This CL also introduces the member
Isolate::AbortConcurrentOptimization.

R=sigurds@chromium.org

Bug: v8:178
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng
Change-Id: Ie6cbb48ebde18036888af2dd715862e7a14ddf9d
Reviewed-on: https://chromium-review.googlesource.com/912468
Commit-Queue: Yang Guo <yangguo@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51384}

src/api.cc

@@ -10405,7 +10405,7 @@ void Testing::PrepareStressRun(int run) {
 void Testing::DeoptimizeAll(Isolate* isolate) {
   i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);
   i::HandleScope scope(i_isolate);
-  internal::Deoptimizer::DeoptimizeAll(i_isolate);
+  i::Deoptimizer::DeoptimizeAll(i_isolate);
 }
 
 

src/compiler-dispatcher/compiler-dispatcher.cc

@@ -92,7 +92,7 @@ MemoryPressureTask::MemoryPressureTask(Isolate* isolate,
 MemoryPressureTask::~MemoryPressureTask() {}
 
 void MemoryPressureTask::RunInternal() {
-  dispatcher_->AbortAll(CompilerDispatcher::BlockingBehavior::kDontBlock);
+  dispatcher_->AbortAll(BlockingBehavior::kDontBlock);
 }
 
 }  // namespace

src/compiler-dispatcher/compiler-dispatcher.h

@@ -72,8 +72,6 @@ class V8_EXPORT_PRIVATE CompilerDispatcher {
  public:
   typedef uintptr_t JobId;
 
-  enum class BlockingBehavior { kBlock, kDontBlock };
-
   CompilerDispatcher(Isolate* isolate, Platform* platform,
                      size_t max_stack_size);
   ~CompilerDispatcher();

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

@@ -23,8 +23,6 @@ class SharedFunctionInfo;
 
 class V8_EXPORT_PRIVATE OptimizingCompileDispatcher {
  public:
-  enum class BlockingBehavior { kBlock, kDontBlock };
-
   explicit OptimizingCompileDispatcher(Isolate* isolate)
       : isolate_(isolate),
         input_queue_capacity_(FLAG_concurrent_recompilation_queue_length),

src/compiler/js-call-reducer.cc

@@ -2887,6 +2887,9 @@ Reduction JSCallReducer::ReduceJSCall(Node* node) {
       Handle<JSFunction> function = Handle<JSFunction>::cast(m.Value());
       Handle<SharedFunctionInfo> shared(function->shared(), isolate());
 
+      // Do not reduce calls to functions with break points.
+      if (shared->HasBreakInfo()) return NoChange();
+
       // Raise a TypeError if the {target} is a "classConstructor".
       if (IsClassConstructor(shared->kind())) {
         NodeProperties::ReplaceValueInputs(node, target);

src/debug/debug.cc

@@ -12,7 +12,6 @@
 #include "src/bootstrapper.h"
 #include "src/code-stubs.h"
 #include "src/compilation-cache.h"
-#include "src/compiler-dispatcher/optimizing-compile-dispatcher.h"
 #include "src/compiler.h"
 #include "src/debug/debug-evaluate.h"
 #include "src/debug/liveedit.h"
@@ -1140,10 +1139,7 @@ class RedirectActiveFunctions : public ThreadVisitor {
 void Debug::DeoptimizeFunction(Handle<SharedFunctionInfo> shared) {
   // Deoptimize all code compiled from this shared function info including
   // inlining.
-  if (isolate_->concurrent_recompilation_enabled()) {
-    isolate_->optimizing_compile_dispatcher()->Flush(
-        OptimizingCompileDispatcher::BlockingBehavior::kBlock);
-  }
+  isolate_->AbortConcurrentOptimization(BlockingBehavior::kBlock);
 
   // Make sure we abort incremental marking.
   isolate_->heap()->CollectAllGarbage(Heap::kMakeHeapIterableMask,

src/deoptimizer.cc

@@ -276,6 +276,7 @@ void Deoptimizer::DeoptimizeAll(Isolate* isolate) {
     CodeTracer::Scope scope(isolate->GetCodeTracer());
     PrintF(scope.file(), "[deoptimize all code in all contexts]\n");
   }
+  isolate->AbortConcurrentOptimization(BlockingBehavior::kBlock);
   DisallowHeapAllocation no_allocation;
   // For all contexts, mark all code, then deoptimize.
   Object* context = isolate->heap()->native_contexts_list();

src/globals.h

@@ -1463,6 +1463,8 @@ inline std::ostream& operator<<(std::ostream& os,
   return os;
 }
 
+enum class BlockingBehavior { kBlock, kDontBlock };
+
 enum class ConcurrencyMode { kNotConcurrent, kConcurrent };
 
 #define FOR_EACH_ISOLATE_ADDRESS_NAME(C)                       \

src/heap/heap.cc

@@ -17,7 +17,6 @@
 #include "src/bootstrapper.h"
 #include "src/code-stubs.h"
 #include "src/compilation-cache.h"
-#include "src/compiler-dispatcher/optimizing-compile-dispatcher.h"
 #include "src/conversions.h"
 #include "src/debug/debug.h"
 #include "src/deoptimizer.h"
@@ -1166,12 +1165,9 @@ void Heap::CollectAllAvailableGarbage(GarbageCollectionReason gc_reason) {
   }
   RuntimeCallTimerScope runtime_timer(
       isolate(), RuntimeCallCounterId::kGC_Custom_AllAvailableGarbage);
-  if (isolate()->concurrent_recompilation_enabled()) {
+
   // The optimizing compiler may be unnecessarily holding on to memory.
-    DisallowHeapAllocation no_recursive_gc;
-    isolate()->optimizing_compile_dispatcher()->Flush(
-        OptimizingCompileDispatcher::BlockingBehavior::kDontBlock);
-  }
+  isolate()->AbortConcurrentOptimization(BlockingBehavior::kDontBlock);
   isolate()->ClearSerializerData();
   set_current_gc_flags(kMakeHeapIterableMask | kReduceMemoryFootprintMask);
   isolate_->compilation_cache()->Clear();
@@ -1375,11 +1371,8 @@ int Heap::NotifyContextDisposed(bool dependant_context) {
     event.time_ms = MonotonicallyIncreasingTimeInMs();
     memory_reducer_->NotifyPossibleGarbage(event);
   }
-  if (isolate()->concurrent_recompilation_enabled()) {
-    // Flush the queued recompilation tasks.
-    isolate()->optimizing_compile_dispatcher()->Flush(
-        OptimizingCompileDispatcher::BlockingBehavior::kDontBlock);
-  }
+  isolate()->AbortConcurrentOptimization(BlockingBehavior::kDontBlock);
+
   number_of_disposed_maps_ = retained_maps()->Length();
   tracer()->AddContextDisposalTime(MonotonicallyIncreasingTimeInMs());
   return ++contexts_disposed_;
@@ -4526,12 +4519,8 @@ class MemoryPressureInterruptTask : public CancelableTask {
 
 void Heap::CheckMemoryPressure() {
   if (HighMemoryPressure()) {
-    if (isolate()->concurrent_recompilation_enabled()) {
     // The optimizing compiler may be unnecessarily holding on to memory.
-      DisallowHeapAllocation no_recursive_gc;
-      isolate()->optimizing_compile_dispatcher()->Flush(
-          OptimizingCompileDispatcher::BlockingBehavior::kDontBlock);
-    }
+    isolate()->AbortConcurrentOptimization(BlockingBehavior::kDontBlock);
   }
   if (memory_pressure_level_.Value() == MemoryPressureLevel::kCritical) {
     CollectGarbageOnMemoryPressure();

src/isolate.cc

@@ -2643,7 +2643,7 @@ void Isolate::Deinit() {
   delete heap_profiler_;
   heap_profiler_ = nullptr;
 
-  compiler_dispatcher_->AbortAll(CompilerDispatcher::BlockingBehavior::kBlock);
+  compiler_dispatcher_->AbortAll(BlockingBehavior::kBlock);
   delete compiler_dispatcher_;
   compiler_dispatcher_ = nullptr;
 
@@ -3258,6 +3258,12 @@ void Isolate::DumpAndResetStats() {
   }
 }
 
+void Isolate::AbortConcurrentOptimization(BlockingBehavior behavior) {
+  if (concurrent_recompilation_enabled()) {
+    DisallowHeapAllocation no_recursive_gc;
+    optimizing_compile_dispatcher()->Flush(behavior);
+  }
+}
 
 CompilationStatistics* Isolate::GetTurboStatistics() {
   if (turbo_statistics() == nullptr)

src/isolate.h

@@ -1139,6 +1139,9 @@ class Isolate {
   OptimizingCompileDispatcher* optimizing_compile_dispatcher() {
     return optimizing_compile_dispatcher_;
   }
+  // Flushes all pending concurrent optimzation jobs from the optimizing
+  // compile dispatcher's queue.
+  void AbortConcurrentOptimization(BlockingBehavior blocking_behavior);
 
   int id() const { return static_cast<int>(id_); }
 

test/cctest/test-debug.cc

@@ -1142,6 +1142,7 @@ TEST(BreakPointReturn) {
 // Test that a break point can be set at a return store location.
 TEST(BreakPointBuiltin) {
   i::FLAG_allow_natives_syntax = true;
+  i::FLAG_block_concurrent_recompilation = true;
   break_point_hit_count = 0;
   DebugLocalContext env;
   v8::HandleScope scope(env->GetIsolate());
@@ -1190,6 +1191,56 @@ TEST(BreakPointBuiltin) {
   CompileRun("test(0.3);");
   CHECK_EQ(3, break_point_hit_count);
 
+  // === Test concurrent optimization ===
+  break_point_hit_count = 0;
+  builtin = CompileRun("Math.sin").As<v8::Function>();
+  CompileRun("function test(x) { return 1 + Math.sin(x) }");
+  // Trigger concurrent compile job. It is suspended until unblock.
+  CompileRun(
+      "test(0.5); test(0.6);"
+      "%OptimizeFunctionOnNextCall(test, 'concurrent'); test(0.7);");
+  CHECK_EQ(0, break_point_hit_count);
+
+  // Run with breakpoint.
+  bp = SetBreakPoint(builtin, 0);
+  // Have the concurrent compile job finish now.
+  CompileRun(
+      "%UnblockConcurrentRecompilation();"
+      "%GetOptimizationStatus(test, 'sync');");
+  CompileRun("test(0.2);");
+  CHECK_EQ(1, break_point_hit_count);
+
+  // Run without breakpoints.
+  ClearBreakPoint(bp);
+  CompileRun("test(0.3);");
+  CHECK_EQ(1, break_point_hit_count);
+
+  // === Test builtin represented as operator ===
+  break_point_hit_count = 0;
+  builtin = CompileRun("String.prototype.indexOf").As<v8::Function>();
+  CompileRun("function test(x) { return 1 + 'foo'.indexOf(x) }");
+  CompileRun(
+      "test('a'); test('b');"
+      "%OptimizeFunctionOnNextCall(test); test('c');");
+  CHECK_EQ(0, break_point_hit_count);
+
+  // Run with breakpoint.
+  bp = SetBreakPoint(builtin, 0);
+  CompileRun("'bar'.indexOf('x');");
+  CHECK_EQ(1, break_point_hit_count);
+  CompileRun("test('d');");
+  CHECK_EQ(2, break_point_hit_count);
+
+  // Re-optimize.
+  CompileRun("%OptimizeFunctionOnNextCall(test);");
+  CompileRun("test('e');");
+  CHECK_EQ(3, break_point_hit_count);
+
+  // Run without breakpoints.
+  ClearBreakPoint(bp);
+  CompileRun("test('f');");
+  CHECK_EQ(3, break_point_hit_count);
+
   SetDebugEventListener(env->GetIsolate(), nullptr);
   CheckDebuggerUnloaded();
 }

test/unittests/compiler-dispatcher/compiler-dispatcher-unittest.cc

@@ -336,7 +336,7 @@ TEST_F(CompilerDispatcherTest, IsEnqueued) {
   ASSERT_FALSE(dispatcher.IsEnqueued(shared));
   ASSERT_TRUE(dispatcher.Enqueue(shared));
   ASSERT_TRUE(dispatcher.IsEnqueued(shared));
-  dispatcher.AbortAll(CompilerDispatcher::BlockingBehavior::kBlock);
+  dispatcher.AbortAll(BlockingBehavior::kBlock);
   ASSERT_FALSE(dispatcher.IsEnqueued(shared));
   ASSERT_TRUE(platform.IdleTaskPending());
   platform.ClearIdleTask();
@@ -640,7 +640,7 @@ TEST_F(CompilerDispatcherTest, AsyncAbortAllPendingBackgroundTask) {
   ASSERT_TRUE(platform.BackgroundTasksPending());
 
   // The background task hasn't yet started, so we can just cancel it.
-  dispatcher.AbortAll(CompilerDispatcher::BlockingBehavior::kDontBlock);
+  dispatcher.AbortAll(BlockingBehavior::kDontBlock);
   ASSERT_FALSE(platform.ForegroundTasksPending());
 
   ASSERT_FALSE(dispatcher.IsEnqueued(shared));
@@ -692,7 +692,7 @@ TEST_F(CompilerDispatcherTest, AsyncAbortAllRunningBackgroundTask) {
   // Busy loop until the background task started running.
   while (dispatcher.block_for_testing_.Value()) {
   }
-  dispatcher.AbortAll(CompilerDispatcher::BlockingBehavior::kDontBlock);
+  dispatcher.AbortAll(BlockingBehavior::kDontBlock);
   ASSERT_TRUE(platform.ForegroundTasksPending());
 
   // We can't schedule new tasks while we're aborting.
@@ -768,7 +768,7 @@ TEST_F(CompilerDispatcherTest, FinishNowDuringAbortAll) {
   // Busy loop until the background task started running.
   while (dispatcher.block_for_testing_.Value()) {
   }
-  dispatcher.AbortAll(CompilerDispatcher::BlockingBehavior::kDontBlock);
+  dispatcher.AbortAll(BlockingBehavior::kDontBlock);
   ASSERT_TRUE(platform.ForegroundTasksPending());
 
   // Run the first AbortTask. Since the background job is still pending, it

test/unittests/compiler-dispatcher/optimizing-compile-dispatcher-unittest.cc

@@ -84,7 +84,7 @@ TEST_F(OptimizingCompileDispatcherTest, NonBlockingFlush) {
   }
 
   // Should not block.
-  dispatcher.Flush(OptimizingCompileDispatcher::BlockingBehavior::kDontBlock);
+  dispatcher.Flush(BlockingBehavior::kDontBlock);
 
   // Unblock the job & finish.
   job->Signal();