Run Crankshaft on a separate thread.

src/crankshaft-thread.cc

Index: src/crankshaft-thread.cc
diff --git a/src/crankshaft-thread.cc b/src/crankshaft-thread.cc
new file mode 100644
index 0000000000000000000000000000000000000000..f55e53fe3eb1ec097b35cbd69216e0f150ed330d
--- /dev/null
+++ b/src/crankshaft-thread.cc
@@ -0,0 +1,259 @@
+// Copyright 2012 the V8 project authors. All rights reserved.
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+//       notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+//       copyright notice, this list of conditions and the following
+//       disclaimer in the documentation and/or other materials provided
+//       with the distribution.
+//     * Neither the name of Google Inc. nor the names of its
+//       contributors may be used to endorse or promote products derived
+//       from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+#include "crankshaft-thread.h"
+
+#include <set>
+
+#include "debug.h"
+#include "flags.h"
+#include "lithium.h"
+#include "v8threads.h"
+
+namespace v8 {
+namespace internal {
+
+
+static void FreeDeletedIsolates(std::set<Isolate*> *deleted_isolates) {
+  for (std::set<Isolate*>::iterator i = deleted_isolates->begin(),

[danno, 2012/05/22 10:32:19]

We don't use STL in V8, you'll have to find another way to store the set. Better
yet, get rid of this deferred disposal (see below).

+                                    e = deleted_isolates->end();
+       i != e; ++i)
+    // We simply call free on the Isolates since the destructor for
+    // each of these instances has already been called.
+    free(*i);
+}
+
+void CrankshaftThread::WorkerThread::Run() {
+  std::set<i::Isolate *> deleted_isolates;
+  // The loop lock is given up at two sites.  For this reason,
+  // successfully acquiring the lock from another thread guarantees
+  // that this thread is one of those two places.
+  loop_lock_->Lock();
+
+  while (true) {
+    // Whenever loop_lock_ is unlocked isolate_ is set to the Isolate
+    // currently being processed.
+    Acquire_Store(&isolate_, static_cast<AtomicWord>(0));
+
+    loop_lock_->Unlock();

[danno, 2012/05/22 10:32:19]

I don't think you need the loop lock at all. Just dequeue elements over and over
again.

queue_semaphore_->Wait();
bool to_stop = Release_Load(&stop_loop_);
if (to_stop) return;
{
  Locker locker(&queue_lock);
  if (work_queue.IsEmpty()) continue;
  WorkElement w_element;
  work_queue_.Dequeue(&w_element);
}

// compile w_element...

[sanjoy, 2012/05/22 11:38:55]

I needed a place in the code where I'm assured isolate_ is set to the Isolate
currently being compiled under, so that I can unlock that isolate from
StopThread if it is locked by the current thread.

Isolates have to be locked before they're deleted to prevent the optimizing
compiler thread from compiling functions in an isolate while it is being torn
down.  The above deadlocked situation arises when a thread destroys the last
isolate and tries to stop the compiler queue.

But I think I can still do without the loop locking, I'll try to be more clever
in the second iteration.  Things should become simpler once the optimizing
compiler thread starts doing only the parts it can do in parallel.

+    queue_semaphore_->Wait();
+    loop_lock_->Lock();
+
+    if (Release_Load(&stop_loop_)) {
+      FreeDeletedIsolates(&deleted_isolates);
+      // We need to unlock the loop lock since it is possible the
+      // thread stopping the worker thread set stop_loop_ after we
+      // locked loop_lock_ and is currently waiting on loop_lock_.
+      loop_lock_->Unlock();
+      return;
+    }
+
+    WorkElement w_element;
+    work_queue_.Dequeue(&w_element);
+
+    if (!w_element.dummy) {
+      i::Isolate* isolate = w_element.isolate;
+      if (deleted_isolates.find(isolate) != deleted_isolates.end())
+        continue;
+
+      Acquire_Store(&isolate_, reinterpret_cast<AtomicWord>(isolate));

[danno, 2012/05/22 10:32:19]

I think you can get rid of both isolate_ and loop_lock if you follow my
suggestion above.

+      loop_lock_->Unlock();
+
+      {
+        // Can't use the Locker object here since we can't unlock if
+        // this isolate has been marked for deletion.
+        isolate->thread_manager()->Lock();
+        loop_lock_->Lock();
+
+        bool to_stop = Release_Load(&stop_loop_);
+
+        if (isolate->to_delete()) {
+          // We don't delete the Isolate lest the same memory gets
+          // allocated to another Isolate and the crankshaft thread
+          // gets confused.  We still invoke the destructor to free
+          // the Isolate's members.
+          isolate->~Isolate();

[danno, 2012/05/22 10:32:19]

What happens if two functions from the isolate are queued? You'll dispose it
twice, and access the memory from it after the first dispose is done. I think it
is better to have a queue lock, and when a isolate is disposed, flush the queue
of all of the isolates. You will need to be able to deal with dequeueing empty
elements, but that's probably not hard. 

+          deleted_isolates.insert(isolate);
+          if (to_stop)
+            return;
+          else
+            continue;
+        }
+
+        if (to_stop) {
+          FreeDeletedIsolates(&deleted_isolates);

[danno, 2012/05/22 10:32:19]

How big are the isolates? Can we really afford to only dispose them when the
compilation thread exits?

+          loop_lock_->Unlock();
+          return;
+        }
+
+        bool top_level = true;
+        if (isolate->thread_manager()->RestoreThread()) {
+          top_level = false;
+        } else {
+          i::ExecutionAccess access(isolate);
+          isolate->stack_guard()->ClearThread(access);
+          isolate->stack_guard()->InitThread(access);
+        }

[danno, 2012/05/22 10:32:19]

Don't you have to do this after the Enter to ensure that nobody steps on the
stack guard you set up?

[sanjoy, 2012/05/22 11:38:55]

I don't think so, I've already acquired the isolate lock by now.  When running
with --concurrent_crankshaft, Enter()ing and Exit()ing isolates always acquire /
release the lock.

+
+        isolate->Enter();
+        {
+          HandleScope scope(isolate);
+          if (!w_element.function->IsOptimized()) {
+            i::Handle<i::Context> ctx(w_element.function->context());
+            isolate->handle_scope_implementer()->EnterContext(ctx);
+            isolate->handle_scope_implementer()->SaveContext(
+                isolate->context());
+            isolate->set_context(*ctx);
+
+            while (CompileJSFunction(isolate,

[danno, 2012/05/22 10:32:19]

This won't work. You will have to create a handle version of CompileJSFunction
in factory.cc that does the GC->retry cycle. This logic will just end up in an
endless loop. 

+                                     w_element.function)->IsFailure()) {
+            }
+
+            isolate->handle_scope_implementer()->LeaveLastContext();
+            i::Context* last_context =
+                isolate->handle_scope_implementer()->RestoreContext();
+            isolate->set_context(last_context);
+            isolate->set_context_exit_happened(true);
+#ifdef DEBUG
+            functions_compiled_++;
+#endif
+          }
+        }
+        isolate->global_handles()->Destroy(
+            reinterpret_cast<Object **>(w_element.function.location()));
+        isolate->Exit();
+
+        if (top_level)
+          isolate->thread_manager()->ArchiveThread();
+        else
+          isolate->thread_manager()->FreeThreadResources();
+
+        isolate->thread_manager()->Unlock();
+      }
+    }
+  }
+}
+
+
+void CrankshaftThread::WorkerThread::AddElement(const WorkElement &we) {
+  {
+    // We need the queue_lock_ since UnboundQueue is single producer /
+    // single consumer, while we have a multiple producer / single
+    // consumer situation here.
+    ScopedLock lock(queue_lock_);
+    work_queue_.Enqueue(we);
+  }
+  queue_semaphore_->Signal();
+}
+
+
+void CrankshaftThread::WorkerThread::StopThread() {
+  ASSERT(!stop_loop_);
+  Acquire_Store(&stop_loop_, static_cast<AtomicWord>(true));
+
+  {
+    ScopedLock lock(loop_lock_);
+    // Once we have the loop_lock_, we're assured that the worker
+    // thread is in one of the two sites where it gives it up.
+    i::Isolate *isolate =
+        reinterpret_cast<i::Isolate *>(NoBarrier_Load(&isolate_));
+
+    if (!isolate) {
+      // If isolate is NULL, we the worker thread it is possible the
+      // worker thread waiting on queue_semaphore_.  Insert a dummy
+      // element to get it past the semaphore.
+      WorkElement element;
+      element.dummy = true;
+      AddElement(element);

[danno, 2012/05/22 10:32:19]

Why do you need the explicit dummy element? You set stop_loop_ already. Instead
of AddElement, just queue_semaphore_->Signal() ?

[sanjoy, 2012/05/22 11:38:55]

Right, it is not needed.  I'll remove it in the next iteration.

+    } else if (isolate->thread_manager() &&
+               isolate->thread_manager()->IsLockedByCurrentThread()) {
+      isolate->thread_manager()->ArchiveThread();
+      isolate->thread_manager()->Unlock();
+    } else {
+      ASSERT(!isolate->thread_manager()->IsLockedByCurrentThread() ||
+             isolate->to_delete());
+    }
+  }
+
+  Join();
+
+  ElementsAccessor::TearDown();

[danno, 2012/05/22 10:32:19]

Why are these here? Shouldn't they be in V8 teardown (they are not specific to
the compilation thread).

+  LOperand::TearDownCaches();
+  RegisteredExtension::UnregisterAll();
+
+#ifdef DEBUG
+  PrintF(stderr, "   > %d functions compiled concurrently!\n",
+         functions_compiled_);

[danno, 2012/05/22 10:32:19]

use v8-counters for this

+#endif
+}
+
+
+void CrankshaftThread::StartThread() {
+  ASSERT(FLAG_concurrent_crankshaft);
+  ASSERT(!worker_instance_);
+
+#ifdef DEBUG
+  PrintF(stderr, "    > Starting Crankshaft Thread\n");

[danno, 2012/05/22 10:32:19]

Remove the print, please

+#endif

[danno, 2012/05/22 10:32:19]

Remove the print, please

+
+  Thread::Options o("CShaftThread");
+  worker_instance_ = new WorkerThread(o);
+  worker_instance_->Start();
+}
+
+
+void CrankshaftThread::StopThread() {
+  ASSERT(FLAG_concurrent_crankshaft);
+  ASSERT(worker_instance_);
+
+  worker_instance_->StopThread();
+}
+
+
+void CrankshaftThread::AddJob(Isolate *isolate,
+                              i::Handle<JSFunction> function) {
+  ASSERT(FLAG_concurrent_crankshaft);
+  ASSERT(worker_instance_);
+  ASSERT(isolate->thread_manager()->IsLockedByCurrentThread());
+
+  WorkElement we;
+  we.function = function;
+  we.isolate = isolate;
+
+  worker_instance_->AddElement(we);
+
+  if (!isolate->debugger()->IsDebuggerActive()) {
+    Unlocker unlock(reinterpret_cast<v8::Isolate *>(isolate));
+    Thread::YieldCPU();
+  }
+}
+
+
+CrankshaftThread::WorkerThread *CrankshaftThread::worker_instance_ = NULL;
+
+} }  // namespace v8::internal
+