Teach compiler jobs how to actually parse

src/compiler-dispatcher/compiler-dispatcher-job.cc

Index: src/compiler-dispatcher/compiler-dispatcher-job.cc
diff --git a/src/compiler-dispatcher/compiler-dispatcher-job.cc b/src/compiler-dispatcher/compiler-dispatcher-job.cc
index 7f4991a112d96dd9d6ecf56f96c191d96a36b87a..382d94ba6df04c5c27f6ad5a3bb84edeffeb25c5 100644
--- a/src/compiler-dispatcher/compiler-dispatcher-job.cc
+++ b/src/compiler-dispatcher/compiler-dispatcher-job.cc
@@ -4,6 +4,7 @@
 
 #include "src/compiler-dispatcher/compiler-dispatcher-job.h"
 
+#include "src/assert-scope.h"
 #include "src/global-handles.h"
 #include "src/isolate.h"
 #include "src/objects-inl.h"
@@ -16,11 +17,24 @@ namespace v8 {
 namespace internal {
 
 CompilerDispatcherJob::CompilerDispatcherJob(Isolate* isolate,
-                                             Handle<JSFunction> function)
+                                             Handle<JSFunction> function,
+                                             size_t max_stack_size)
     : isolate_(isolate),
       function_(Handle<JSFunction>::cast(
           isolate_->global_handles()->Create(*function))),
-      can_parse_on_background_thread_(false) {}
+      max_stack_size_(max_stack_size) {
+  HandleScope scope(isolate_);
+  Handle<SharedFunctionInfo> shared(function_->shared(), isolate_);
+  Handle<Script> script(Script::cast(shared->script()), isolate_);
+  Handle<String> source(String::cast(script->source()), isolate_);
+  if (source->IsExternalTwoByteString()) {

[vogelheim, 2016/07/29 10:19:56]

nitpick:

can_parse_on_background_thread_ = source->IsExternalOneByteString() ||
source->isExternalTwoByteString();

[jochen (gone - plz use gerrit), 2016/07/29 10:27:45]

will update in my next cl

+    can_parse_on_background_thread_ = true;
+  } else if (source->IsExternalOneByteString()) {
+    can_parse_on_background_thread_ = true;
+  } else {
+    can_parse_on_background_thread_ = false;
+  }
+}
 
 CompilerDispatcherJob::~CompilerDispatcherJob() {
   DCHECK(ThreadId::Current().Equals(isolate_->thread_id()));
@@ -29,7 +43,7 @@ CompilerDispatcherJob::~CompilerDispatcherJob() {
 
 void CompilerDispatcherJob::PrepareToParseOnMainThread() {
   DCHECK(ThreadId::Current().Equals(isolate_->thread_id()));
-  DCHECK(status_ == CompileJobStatus::kInitial);
+  DCHECK(status() == CompileJobStatus::kInitial);
   HandleScope scope(isolate_);
   unicode_cache_.reset(new UnicodeCache());
   zone_.reset(new Zone(isolate_->allocator()));
@@ -56,7 +70,32 @@ void CompilerDispatcherJob::PrepareToParseOnMainThread() {
   parse_info_->set_character_stream(character_stream_.get());
   parse_info_->set_hash_seed(isolate_->heap()->HashSeed());
   parse_info_->set_unicode_cache(unicode_cache_.get());
-  status_ = CompileJobStatus::kReadyToParse;
+  parser_.reset(new Parser(parse_info_.get()));
+  status_.SetValue(CompileJobStatus::kReadyToParse);
+}
+
+void CompilerDispatcherJob::Parse() {
+  DCHECK(can_parse_on_background_thread_ ||
+         ThreadId::Current().Equals(isolate_->thread_id()));
+  DCHECK(status() == CompileJobStatus::kReadyToParse);

[vogelheim, 2016/07/29 10:19:56]

I don't understand the concurrency model.

- You've made status_ an atomic variable, so you expect it to be called without
any external synchronization.
- Nothing else here is lock-guarded or atomic, so clearly non of the rest
expects to be called concurrently, but assumes that the caller has made sure
this object is 'theirs'.
- Presumably, the caller would make some sort of decisions based on status().
-  But.. you only update it at the end, which looks like there might be a race
condition. That is, something has called Parse(), and another is calling
status(), presumably to determine whether it may call Parse() also.

This doesn't seem to fit together... care to explain?

[jochen (gone - plz use gerrit), 2016/07/29 10:27:45]

Yeah, I guess I can make it non-atomic and just introduce a lock to be held once
the dispatcher exists and can invoke stuff on several threads

+
+  DisallowHeapAllocation no_allocation;
+  DisallowHandleAllocation no_handles;
+  DisallowHandleDereference no_deref;
+
+  // Nullify the Isolate temporarily so that the parser doesn't accidentally
+  // use it.
+  parse_info_->set_isolate(nullptr);
+
+  uintptr_t stack_limit =
+      reinterpret_cast<uintptr_t>(&stack_limit) - max_stack_size_ * KB;
+
+  parser_->set_stack_limit(stack_limit);
+  parser_->ParseOnBackground(parse_info_.get());
+
+  parse_info_->set_isolate(isolate_);
+
+  status_.SetValue(CompileJobStatus::kParsed);
 }
 
 }  // namespace internal