Revert "Deoptimization and multithreading. "

This reverts commit c87f8954.

Reason for revert: LazyDeoptimizationMultithread failing.

https://build.chromium.org/p/client.v8/builders/V8%20Linux64%20TSAN%20-%20concurrent%20marking/builds/1876/steps/Bisect%20c87f8954.Retry/logs/LazyDeoptimizationMul..

Original change's description:
> Deoptimization and multithreading. 
> 
> When using Lockers and Unlockers it is possible to create a 
> scenario where multiple threads point to the same optimized 
> code object. When that happens, if one of the threads triggers
> deoptimization, then the stack replacement needs to happen in 
> the stacks of all threads.
> With this CL, the deoptimizer visits all threads to do so.
> The CL also adds three tests where V8 used to crash.
> 
> Bug: v8:6563
> Change-Id: Iea88f47af2f31181c0ef06d898faccde9ad14432
> Reviewed-on: https://chromium-review.googlesource.com/657423
> Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
> Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
> Commit-Queue: Juliana Patricia Vicente Franco <jupvfranco@google.com>
> Cr-Commit-Position: refs/heads/master@{#48033}

TBR=mstarzinger@chromium.org,jarin@chromium.org,bmeurer@chromium.org,jupvfranco@google.com

Change-Id: I290c9e339c367f68c0d1b6f7c0780cdbbbdf3f8a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:6563
Reviewed-on: https://chromium-review.googlesource.com/669399Reviewed-by: Bill Budge <bbudge@chromium.org>
Commit-Queue: Bill Budge <bbudge@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48049}

src/deoptimizer.cc

@@ -144,59 +144,15 @@ void Deoptimizer::GenerateDeoptimizationEntries(MacroAssembler* masm,
   generator.Generate();
 }
 
-namespace {
-class ActivationsFinder : public ThreadVisitor {
- public:
-  explicit ActivationsFinder(std::set<Code*>* codes,
-                             Code* topmost_optimized_code,
-                             bool safe_to_deopt_topmost_optimized_code)
-      : codes_(codes) {
-#ifdef DEBUG
-    topmost_ = topmost_optimized_code;
-    safe_to_deopt_ = safe_to_deopt_topmost_optimized_code;
-#endif
-  }
-
-  // Find the frames with activations of codes marked for deoptimization, search
-  // for the trampoline to the deoptimizer call respective to each code, and use
-  // it to replace the current pc on the stack.
-  void VisitThread(Isolate* isolate, ThreadLocalTop* top) {
-    for (StackFrameIterator it(isolate, top); !it.done(); it.Advance()) {
-      if (it.frame()->type() == StackFrame::OPTIMIZED) {
-        Code* code = it.frame()->LookupCode();
-        if (code->kind() == Code::OPTIMIZED_FUNCTION &&
-            code->marked_for_deoptimization()) {
-          codes_->erase(code);
-          // Obtain the trampoline to the deoptimizer call.
-          SafepointEntry safepoint = code->GetSafepointEntry(it.frame()->pc());
-          int trampoline_pc = safepoint.trampoline_pc();
-          DCHECK_IMPLIES(code == topmost_, safe_to_deopt_);
-          // Replace the current pc on the stack with the trampoline.
-          it.frame()->set_pc(code->instruction_start() + trampoline_pc);
-        }
-      }
-    }
-  }
-
- private:
-  std::set<Code*>* codes_;
-
-#ifdef DEBUG
-  Code* topmost_;
-  bool safe_to_deopt_;
-#endif
-};
-}  // namespace
-
 // Move marked code from the optimized code list to the deoptimized code list,
 // and replace pc on the stack for codes marked for deoptimization.
 void Deoptimizer::DeoptimizeMarkedCodeForContext(Context* context) {
   DisallowHeapAllocation no_allocation;
 
   Isolate* isolate = context->GetHeap()->isolate();
+#ifdef DEBUG
   Code* topmost_optimized_code = NULL;
   bool safe_to_deopt_topmost_optimized_code = false;
-#ifdef DEBUG
   // Make sure all activations of optimized code can deopt at their current PC.
   // The topmost optimized code has special handling because it cannot be
   // deoptimized due to weak object dependency.
@@ -271,16 +227,28 @@ void Deoptimizer::DeoptimizeMarkedCodeForContext(Context* context) {
     element = next;
   }
 
-  ActivationsFinder visitor(&codes, topmost_optimized_code,
-                            safe_to_deopt_topmost_optimized_code);
-  // Iterate over the stack of this thread.
-  visitor.VisitThread(isolate, isolate->thread_local_top());
-  // In addition to iterate over the stack of this thread, we also
-  // need to consider all the other threads as they may also use
-  // the code currently beings deoptimized.
-  isolate->thread_manager()->IterateArchivedThreads(&visitor);
+  // Find the frames with activations of codes marked for deoptimization, search
+  // for the trampoline to the deoptimizer call respective to each code, and use
+  // it to replace the current pc on the stack.
+  for (StackFrameIterator it(isolate, isolate->thread_local_top()); !it.done();
+       it.Advance()) {
+    if (it.frame()->type() == StackFrame::OPTIMIZED) {
+      Code* code = it.frame()->LookupCode();
+      if (code->kind() == Code::OPTIMIZED_FUNCTION &&
+          code->marked_for_deoptimization()) {
+        codes.erase(code);
+        // Obtain the trampoline to the deoptimizer call.
+        SafepointEntry safepoint = code->GetSafepointEntry(it.frame()->pc());
+        int trampoline_pc = safepoint.trampoline_pc();
+        DCHECK_IMPLIES(code == topmost_optimized_code,
+                       safe_to_deopt_topmost_optimized_code);
+        // Replace the current pc on the stack with the trampoline.
+        it.frame()->set_pc(code->instruction_start() + trampoline_pc);
+      }
+    }
+  }
 
-  // If there's no activation of a code in any stack then we can remove its
+  // If there's no activation of a code in the stack then we can remove its
   // deoptimization data. We do this to ensure that Code objects that will be
   // unlinked won't be kept alive.
   std::set<Code*>::iterator it;

src/runtime/runtime-compiler.cc

@@ -137,6 +137,28 @@ RUNTIME_FUNCTION(Runtime_NotifyStubFailure) {
   return isolate->heap()->undefined_value();
 }
 
+class ActivationsFinder : public ThreadVisitor {
+ public:
+  Code* code_;
+  bool has_code_activations_;
+
+  explicit ActivationsFinder(Code* code)
+      : code_(code), has_code_activations_(false) {}
+
+  void VisitThread(Isolate* isolate, ThreadLocalTop* top) {
+    JavaScriptFrameIterator it(isolate, top);
+    VisitFrames(&it);
+  }
+
+  void VisitFrames(JavaScriptFrameIterator* it) {
+    for (; !it->done(); it->Advance()) {
+      JavaScriptFrame* frame = it->frame();
+      if (code_->contains(frame->pc())) has_code_activations_ = true;
+    }
+  }
+};
+
+
 RUNTIME_FUNCTION(Runtime_NotifyDeoptimized) {
   HandleScope scope(isolate);
   DCHECK_EQ(1, args.length());

test/cctest/test-lockers.cc

@@ -54,240 +54,6 @@ using ::v8::String;
 using ::v8::Value;
 using ::v8::V8;
 
-namespace {
-
-class DeoptimizeCodeThread : public v8::base::Thread {
- public:
-  DeoptimizeCodeThread(v8::Isolate* isolate, v8::Local<v8::Context> context,
-                       const char* trigger)
-      : Thread(Options("DeoptimizeCodeThread")),
-        isolate_(isolate),
-        context_(isolate, context),
-        source_(trigger) {}
-
-  void Run() {
-    v8::Locker locker(isolate_);
-    isolate_->Enter();
-    v8::HandleScope handle_scope(isolate_);
-    v8::Local<v8::Context> context =
-        v8::Local<v8::Context>::New(isolate_, context_);
-    v8::Context::Scope context_scope(context);
-    CHECK_EQ(isolate_, v8::Isolate::GetCurrent());
-    // This code triggers deoptimization of some function that will be
-    // used in a different thread.
-    CompileRun(source_);
-    isolate_->Exit();
-  }
-
- private:
-  v8::Isolate* isolate_;
-  Persistent<v8::Context> context_;
-  // The code that triggers the deoptimization.
-  const char* source_;
-};
-
-void UnlockForDeoptimization(const v8::FunctionCallbackInfo<v8::Value>& args) {
-  v8::Isolate* isolate = v8::Isolate::GetCurrent();
-  // Gets the pointer to the thread that will trigger the deoptimization of the
-  // code.
-  DeoptimizeCodeThread* deoptimizer =
-      reinterpret_cast<DeoptimizeCodeThread*>(isolate->GetData(0));
-  {
-    // Exits and unlocks the isolate.
-    isolate->Exit();
-    v8::Unlocker unlocker(isolate);
-    // Starts the deoptimizing thread.
-    deoptimizer->Start();
-    // Waits for deoptimization to finish.
-    deoptimizer->Join();
-  }
-  // The deoptimizing thread has finished its work, and the isolate
-  // will now be used by the current thread.
-  isolate->Enter();
-}
-
-void UnlockForDeoptimizationIfReady(
-    const v8::FunctionCallbackInfo<v8::Value>& args) {
-  v8::Isolate* isolate = v8::Isolate::GetCurrent();
-  bool* ready_to_deoptimize = reinterpret_cast<bool*>(isolate->GetData(1));
-  if (*ready_to_deoptimize) {
-    // The test should enter here only once, so put the flag back to false.
-    *ready_to_deoptimize = false;
-    // Gets the pointer to the thread that will trigger the deoptimization of
-    // the code.
-    DeoptimizeCodeThread* deoptimizer =
-        reinterpret_cast<DeoptimizeCodeThread*>(isolate->GetData(0));
-    {
-      // Exits and unlocks the thread.
-      isolate->Exit();
-      v8::Unlocker unlocker(isolate);
-      // Starts the thread that deoptimizes the function.
-      deoptimizer->Start();
-      // Waits for the deoptimizing thread to finish.
-      deoptimizer->Join();
-    }
-    // The deoptimizing thread has finished its work, and the isolate
-    // will now be used by the current thread.
-    isolate->Enter();
-  }
-}
-}  // namespace
-
-TEST(LazyDeoptimizationMultithread) {
-  i::FLAG_allow_natives_syntax = true;
-  v8::Isolate::CreateParams create_params;
-  create_params.array_buffer_allocator = CcTest::array_buffer_allocator();
-  v8::Isolate* isolate = v8::Isolate::New(create_params);
-  {
-    v8::Locker locker(isolate);
-    v8::Isolate::Scope isolate_scope(isolate);
-    v8::HandleScope scope(isolate);
-    v8::Local<v8::Context> context = v8::Context::New(isolate);
-    const char* trigger_deopt = "obj = { y: 0, x: 1 };";
-
-    // We use the isolate to pass arguments to the UnlockForDeoptimization
-    // function. Namely, we pass a pointer to the deoptimizing thread.
-    DeoptimizeCodeThread deoptimize_thread(isolate, context, trigger_deopt);
-    isolate->SetData(0, &deoptimize_thread);
-    v8::Context::Scope context_scope(context);
-
-    // Create the function templace for C++ code that is invoked from
-    // JavaScript code.
-    Local<v8::FunctionTemplate> fun_templ =
-        v8::FunctionTemplate::New(isolate, UnlockForDeoptimization);
-    Local<Function> fun = fun_templ->GetFunction(context).ToLocalChecked();
-    CHECK(context->Global()
-              ->Set(context, v8_str("unlock_for_deoptimization"), fun)
-              .FromJust());
-
-    // Optimizes a function f, which will be deoptimized in another
-    // thread.
-    CompileRun(
-        "var b = false; var obj = { x: 1 };"
-        "function f() { g(); return obj.x; }"
-        "function g() { if (b) { unlock_for_deoptimization(); } }"
-        "%NeverOptimizeFunction(g);"
-        "f(); f(); %OptimizeFunctionOnNextCall(f);"
-        "f();");
-
-    // Trigger the unlocking.
-    Local<Value> v = CompileRun("b = true; f();");
-
-    // Once the isolate has been unlocked, the thread will wait for the
-    // other thread to finish its task. Once this happens, this thread
-    // continues with its execution, that is, with the execution of the
-    // function g, which then returns to f. The function f should have
-    // also been deoptimized. If the replacement did not happen on this
-    // thread's stack, then the test will fail here.
-    CHECK(v->IsNumber());
-    CHECK_EQ(1, static_cast<int>(v->NumberValue(context).FromJust()));
-  }
-}
-
-TEST(LazyDeoptimizationMultithreadWithNatives) {
-  i::FLAG_allow_natives_syntax = true;
-  v8::Isolate::CreateParams create_params;
-  create_params.array_buffer_allocator = CcTest::array_buffer_allocator();
-  v8::Isolate* isolate = v8::Isolate::New(create_params);
-  {
-    v8::Locker locker(isolate);
-    v8::Isolate::Scope isolate_scope(isolate);
-    v8::HandleScope scope(isolate);
-    v8::Local<v8::Context> context = v8::Context::New(isolate);
-    const char* trigger_deopt = "%DeoptimizeFunction(f);";
-
-    // We use the isolate to pass arguments to the UnlockForDeoptimization
-    // function. Namely, we pass a pointer to the deoptimizing thread.
-    DeoptimizeCodeThread deoptimize_thread(isolate, context, trigger_deopt);
-    isolate->SetData(0, &deoptimize_thread);
-    bool ready_to_deopt = false;
-    isolate->SetData(1, &ready_to_deopt);
-    v8::Context::Scope context_scope(context);
-
-    // Create the function templace for C++ code that is invoked from
-    // JavaScript code.
-    Local<v8::FunctionTemplate> fun_templ =
-        v8::FunctionTemplate::New(isolate, UnlockForDeoptimizationIfReady);
-    Local<Function> fun = fun_templ->GetFunction(context).ToLocalChecked();
-    CHECK(context->Global()
-              ->Set(context, v8_str("unlock_for_deoptimization"), fun)
-              .FromJust());
-
-    // Optimizes a function f, which will be deoptimized in another
-    // thread.
-    CompileRun(
-        "var obj = { x: 1 };"
-        "function f() { g(); return obj.x;}"
-        "function g() { "
-        "  unlock_for_deoptimization(); }"
-        "%NeverOptimizeFunction(g);"
-        "f(); f(); %OptimizeFunctionOnNextCall(f);");
-
-    // Trigger the unlocking.
-    ready_to_deopt = true;
-    isolate->SetData(1, &ready_to_deopt);
-    Local<Value> v = CompileRun("f();");
-
-    // Once the isolate has been unlocked, the thread will wait for the
-    // other thread to finish its task. Once this happens, this thread
-    // continues with its execution, that is, with the execution of the
-    // function g, which then returns to f. The function f should have
-    // also been deoptimized. Otherwise, the test will fail here.
-    CHECK(v->IsNumber());
-    CHECK_EQ(1, static_cast<int>(v->NumberValue(context).FromJust()));
-  }
-}
-
-TEST(EagerDeoptimizationMultithread) {
-  i::FLAG_allow_natives_syntax = true;
-  v8::Isolate::CreateParams create_params;
-  create_params.array_buffer_allocator = CcTest::array_buffer_allocator();
-  v8::Isolate* isolate = v8::Isolate::New(create_params);
-  {
-    v8::Locker locker(isolate);
-    v8::Isolate::Scope isolate_scope(isolate);
-    v8::HandleScope scope(isolate);
-    v8::Local<v8::Context> context = v8::Context::New(isolate);
-    const char* trigger_deopt = "f({y: 0, x: 1});";
-
-    // We use the isolate to pass arguments to the UnlockForDeoptimization
-    // function. Namely, we pass a pointer to the deoptimizing thread.
-    DeoptimizeCodeThread deoptimize_thread(isolate, context, trigger_deopt);
-    isolate->SetData(0, &deoptimize_thread);
-    bool ready_to_deopt = false;
-    isolate->SetData(1, &ready_to_deopt);
-    v8::Context::Scope context_scope(context);
-
-    // Create the function templace for C++ code that is invoked from
-    // JavaScript code.
-    Local<v8::FunctionTemplate> fun_templ =
-        v8::FunctionTemplate::New(isolate, UnlockForDeoptimizationIfReady);
-    Local<Function> fun = fun_templ->GetFunction(context).ToLocalChecked();
-    CHECK(context->Global()
-              ->Set(context, v8_str("unlock_for_deoptimization"), fun)
-              .FromJust());
-
-    // Optimizes a function f, which will be deoptimized by another thread.
-    CompileRun(
-        "function f(obj) { unlock_for_deoptimization(); return obj.x; }"
-        "f({x: 1}); f({x: 1});"
-        "%OptimizeFunctionOnNextCall(f);"
-        "f({x: 1});");
-
-    // Trigger the unlocking.
-    ready_to_deopt = true;
-    isolate->SetData(1, &ready_to_deopt);
-    Local<Value> v = CompileRun("f({x: 1});");
-
-    // Once the isolate has been unlocked, the thread will wait for the
-    // other thread to finish its task. Once this happens, this thread
-    // continues with its execution, that is, with the execution of the
-    // function g, which then returns to f. The function f should have
-    // also been deoptimized. Otherwise, the test will fail here.
-    CHECK(v->IsNumber());
-    CHECK_EQ(1, static_cast<int>(v->NumberValue(context).FromJust()));
-  }
-}
 
 // Migrating an isolate
 class KangarooThread : public v8::base::Thread {