More work for background compilation; move pending_functions_ from ObjectStore to Thread.

runtime/vm/compiler.cc

 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include "vm/compiler.h"
 
 #include "vm/assembler.h"
 
 #include "vm/ast_printer.h"
 #include "vm/block_scheduler.h"
@@ skipping 1145 matching lines @@
 
 
 RawError* Compiler::CompileOptimizedFunction(Thread* thread,
                                              const Function& function,
                                              intptr_t osr_id) {
   VMTagScope tagScope(thread, VMTag::kCompileOptimizedTagId);
   TIMELINE_FUNCTION_COMPILATION_DURATION(thread,
                                          "OptimizedFunction", function);
 
   // Optimization must happen in non-mutator/Dart thread if background
-  // compilation is on.
-  ASSERT(!FLAG_background_compilation ||
+  // compilation is on. OSR compilation still occura in the main thread.
+  ASSERT((osr_id != Thread::kNoDeoptId) || !FLAG_background_compilation ||
          !thread->isolate()->MutatorThreadIsCurrentThread());
   CompilationPipeline* pipeline =
       CompilationPipeline::New(thread->zone(), function);
   return CompileFunctionHelper(pipeline, function, true, osr_id);
 }
 
 
 // This is only used from unit tests.
 RawError* Compiler::CompileParsedFunction(
     ParsedFunction* parsed_function) {
@@ skipping 235 matching lines @@
 // PushFront and PopBack to add and read from queue.
 // TODO(srdjan): Write a more efficient implementation.
 class CompilationWorkQueue : public ValueObject {
  public:
   explicit CompilationWorkQueue(Isolate* isolate) :
       data_(GrowableObjectArray::Handle()) {
     data_ = isolate->background_compilation_queue();
   }
 
   intptr_t IsEmpty() const { return data_.Length() == 0; }
+  intptr_t Length() const { return data_.Length(); }
 
   // Adds to the queue only if 'function' is not already in there.
   void PushFront(const Function& function) {
     for (intptr_t i = 0; i < data_.Length(); i++) {
       if (data_.At(i) == function.raw()) {
         return;
       }
     }
     // Insert new element in front.
     Object& f = Object::Handle();
     data_.Add(f);
     for (intptr_t i = data_.Length() - 1; i > 0; i--) {
       f = data_.At(i - 1);
       data_.SetAt(i, f);
     }
     data_.SetAt(0, function);
   }
 
   RawFunction* PopBack() {
     ASSERT(!IsEmpty());
     Object& result = Object::Handle();
     result = data_.At(data_.Length() - 1);
     data_.SetLength(data_.Length() - 1);
     return Function::Cast(result).raw();
   }
 
+  RawFunction* Last() {
+    ASSERT(!IsEmpty());
+    Object& result = Object::Handle();
+    result = data_.At(data_.Length() - 1);
+    return Function::Cast(result).raw();
+  }
+
  private:
   GrowableObjectArray& data_;
 
   DISALLOW_IMPLICIT_CONSTRUCTORS(CompilationWorkQueue);
 };
 
 
 BackgroundCompiler::BackgroundCompiler(Isolate* isolate)
     : isolate_(isolate), running_(true), done_(new bool()),
-      monitor_(new Monitor()), done_monitor_(new Monitor())  {
+      queue_monitor_(new Monitor()), done_monitor_(new Monitor()),
+      queue_length_(0) {
   *done_ = false;
 }
 
 
 void BackgroundCompiler::Run() {
   while (running_) {

[siva, 2015/10/16 23:29:40]

running_ is being accessed here without holding the queue_monitor lock?

[srdjan, 2015/10/16 23:50:17]

Yes, running_ is not guarded by queue lock. It can go only from true -> false,
therefore I do not think one is needed when reading it.

-    {
-      // Wait to be notified when the work queue is not empty.
-      MonitorLocker ml(monitor_);
-      ml.Wait();
-    }
-
+    // Maybe something is already in the queue, check first before waiting
+    // to be notified.
     Thread::EnterIsolateAsHelper(isolate_);
     {
       Thread* thread = Thread::Current();
       StackZone stack_zone(thread);
       HANDLESCOPE(thread);
-      Function& function = Function::Handle();
-      function = RemoveOrNull();
-      while (!function.IsNull()) {
-        const Error& error = Error::Handle(
+      Zone* zone = stack_zone.GetZone();
+      Function& function = Function::Handle(zone);
+      Function& temp_function = Function::Handle(zone);
+      function = LastOrNull();
+      while (!function.IsNull() && running_) {

[siva, 2015/10/16 23:29:40]

Ditto question about running_ accessed here without holding the lock.

[srdjan, 2015/10/16 23:50:17]

ditto.

+        const Error& error = Error::Handle(zone,
             Compiler::CompileOptimizedFunction(thread, function));
         // TODO(srdjan): We do not expect errors while compiling optimized
         // code, any errors should have been caught when compiling
         // unoptimized code.
         // If it still happens mark function as not optimizable.
         ASSERT(error.IsNull());
-        function = RemoveOrNull();
+        temp_function = RemoveOrNull();
+        ASSERT(temp_function.raw() == function.raw());
+        function = LastOrNull();
       }
     }
     Thread::ExitIsolateAsHelper();
+    {
+      // Wait to be notified when the work queue is not empty.
+      MonitorLocker ml(queue_monitor_);
+      while (queue_length() == 0) {

[siva, 2015/10/16 23:29:40]

while ((queue_length() == 0) && running_) {
}

[srdjan, 2015/10/16 23:50:17]

Done.

+        ml.Wait();
+      }
+    }
   }
   {
     // Notify that the thread is done.
     MonitorLocker ml_done(done_monitor_);
     *done_ = true;
     ml_done.Notify();
   }
 }
 
 
 void BackgroundCompiler::CompileOptimized(const Function& function) {
   Add(function);
 }
 
 
-void BackgroundCompiler::Add(const Function& f) const {
-  MonitorLocker ml(monitor_);
+void BackgroundCompiler::Add(const Function& f) {
+  MonitorLocker ml(queue_monitor_);
   CompilationWorkQueue queue(isolate_);
   queue.PushFront(f);
+  set_queue_length(queue.Length());
   ml.Notify();
 }
 
 
-RawFunction* BackgroundCompiler::RemoveOrNull() const {
-  MonitorLocker ml(monitor_);
+RawFunction* BackgroundCompiler::RemoveOrNull() {
+  MonitorLocker ml(queue_monitor_);
   CompilationWorkQueue queue(isolate_);
-  return queue.IsEmpty() ? Function::null() : queue.PopBack();
+  if (queue.IsEmpty()) return Function::null();
+  set_queue_length(queue.Length() - 1);
+  return queue.PopBack();
 }
 
 
+RawFunction* BackgroundCompiler::LastOrNull() const {
+  MonitorLocker ml(queue_monitor_);
+  CompilationWorkQueue queue(isolate_);
+  return queue.IsEmpty() ? Function::null() : queue.Last();
+}
+
+
 void BackgroundCompiler::Stop(BackgroundCompiler* task) {
   if (task == NULL) {
     return;
   }
-  Monitor* monitor = task->monitor_;
+  Monitor* monitor = task->queue_monitor_;
   Monitor* done_monitor = task->done_monitor_;
   bool* task_done = task->done_;
   // Wake up compiler task and stop it.
   {
-    MonitorLocker ml(task->monitor_);
+    MonitorLocker ml(task->queue_monitor_);
     task->running_ = false;
     // 'task' will be deleted by thread pool.
     task = NULL;
-    ml.Notify();
+    ml.Notify();   // Stop waiting for the queue.
   }
 
   {
     MonitorLocker ml_done(done_monitor);
     while (!(*task_done)) {
       ml_done.Wait();
     }
   }
   delete task_done;
   delete done_monitor;
@@ skipping 12 matching lines @@
           isolate->current_zone(), GrowableObjectArray::New()));
       start_task = true;
     }
   }
   if (start_task) {
     Dart::thread_pool()->Run(isolate->background_compiler());
   }
 }
 
 }  // namespace dart