Teach compiler jobs how to actually parse

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

 // Copyright 2016 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #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"
 #include "src/parsing/parser.h"
 #include "src/parsing/scanner-character-streams.h"
 #include "src/unicode-cache.h"
 #include "src/zone.h"
 
 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()));
   i::GlobalHandles::Destroy(Handle<Object>::cast(function_).location());
 }
 
 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()));
   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()) {
     can_parse_on_background_thread_ = true;
     character_stream_.reset(new ExternalTwoByteStringUtf16CharacterStream(
         Handle<ExternalTwoByteString>::cast(source), shared->start_position(),
         shared->end_position()));
   } else if (source->IsExternalOneByteString()) {
     can_parse_on_background_thread_ = true;
     character_stream_.reset(new ExternalOneByteStringUtf16CharacterStream(
         Handle<ExternalOneByteString>::cast(source), shared->start_position(),
         shared->end_position()));
   } else {
     can_parse_on_background_thread_ = false;
     character_stream_.reset(new GenericStringUtf16CharacterStream(
         source, shared->start_position(), shared->end_position()));
   }
   parse_info_.reset(new ParseInfo(zone_.get()));
   parse_info_->set_isolate(isolate_);
   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
 }  // namespace v8