[compiler-dispatcher] make it so that we can always parse on bg threads

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/compilation-info.h"
 #include "src/compiler-dispatcher/compiler-dispatcher-tracer.h"
 #include "src/compiler.h"
 #include "src/flags.h"
 #include "src/global-handles.h"
 #include "src/isolate.h"
 #include "src/objects-inl.h"
 #include "src/parsing/parse-info.h"
 #include "src/parsing/parser.h"
 #include "src/parsing/scanner-character-streams.h"
 #include "src/unicode-cache.h"
 #include "src/utils.h"
 #include "src/zone/zone.h"
 
 namespace v8 {
 namespace internal {
 
+namespace {
+
+class OneByteWrapper : public v8::String::ExternalOneByteStringResource {
+ public:
+  OneByteWrapper(const void* data, int length) : data_(data), length_(length) {}
+  ~OneByteWrapper() override = default;
+
+  const char* data() const override {
+    return reinterpret_cast<const char*>(data_);
+  }
+
+  size_t length() const override { return static_cast<size_t>(length_); }
+
+ private:
+  const void* data_;
+  int length_;
+
+  DISALLOW_COPY_AND_ASSIGN(OneByteWrapper);
+};
+
+class TwoByteWrapper : public v8::String::ExternalStringResource {
+ public:
+  TwoByteWrapper(const void* data, int length) : data_(data), length_(length) {}
+  ~TwoByteWrapper() override = default;
+
+  const uint16_t* data() const override {
+    return reinterpret_cast<const uint16_t*>(data_);
+  }
+
+  size_t length() const override { return static_cast<size_t>(length_); }
+
+ private:
+  const void* data_;
+  int length_;
+
+  DISALLOW_COPY_AND_ASSIGN(TwoByteWrapper);
+};
+
+}  // namespace
+
 CompilerDispatcherJob::CompilerDispatcherJob(Isolate* isolate,
                                              CompilerDispatcherTracer* tracer,
                                              Handle<SharedFunctionInfo> shared,
                                              size_t max_stack_size)
     : isolate_(isolate),
       tracer_(tracer),
       shared_(Handle<SharedFunctionInfo>::cast(
           isolate_->global_handles()->Create(*shared))),
       max_stack_size_(max_stack_size),
-      can_compile_on_background_thread_(false),
       trace_compiler_dispatcher_jobs_(FLAG_trace_compiler_dispatcher_jobs) {
   HandleScope scope(isolate_);
   DCHECK(!shared_->outer_scope_info()->IsTheHole(isolate_));
   Handle<Script> script(Script::cast(shared_->script()), isolate_);
   Handle<String> source(String::cast(script->source()), isolate_);
-  can_parse_on_background_thread_ =
-      source->IsExternalTwoByteString() || source->IsExternalOneByteString();
   if (trace_compiler_dispatcher_jobs_) {
     PrintF("CompilerDispatcherJob[%p] created for ", static_cast<void*>(this));
     shared_->ShortPrint();
     PrintF("\n");
   }
 }
 
 CompilerDispatcherJob::~CompilerDispatcherJob() {
   DCHECK(ThreadId::Current().Equals(isolate_->thread_id()));
   DCHECK(status_ == CompileJobStatus::kInitial ||
@@ skipping 18 matching lines @@
   unicode_cache_.reset(new UnicodeCache());
   zone_.reset(new Zone(isolate_->allocator(), ZONE_NAME));
   Handle<Script> script(Script::cast(shared_->script()), isolate_);
   DCHECK(script->type() != Script::TYPE_NATIVE);
 
   Handle<String> source(String::cast(script->source()), isolate_);
   if (source->IsExternalTwoByteString() || source->IsExternalOneByteString()) {
     character_stream_.reset(ScannerStream::For(
         source, shared_->start_position(), shared_->end_position()));
   } else {
     source = String::Flatten(source);

[vogelheim, 2017/01/13 15:20:22]

[all below:]

I'm vaguely uncomfortable with this whole logic, since it (potentially)
introduces a new copy, while we already have so many copies of the source string
that we're desperately trying to get rid of some.

I take it this should rarely (if ever) be triggered for 'real' code, since
pretty much anything we get from Blink is an External(One|Two)ByteString? If so,
my concern is still there, but more academic.


Since at the current state I don't really have an alternative I don#t want to
hold this up. I do suspect we can do better by looking at, and I do think we
need to revisit this at some point...

[jochen (gone - plz use gerrit), 2017/01/13 16:14:40]

Right. I'm also trying to reduce the impact by just copying the function, not
the entire string (if I have to copy)

-    // Have to globalize the reference here, so it survives between function
-    // calls.
-    source_ = Handle<String>::cast(isolate_->global_handles()->Create(*source));
-    character_stream_.reset(ScannerStream::For(
-        source_, shared_->start_position(), shared_->end_position()));
+    const void* data;
+    int offset = 0;
+    int length = source->length();
+
+    // Objects in lo_space don't move, so we can just read the contents from
+    // any thread.
+    if (isolate_->heap()->lo_space()->Contains(*source)) {

[jochen (gone - plz use gerrit), 2017/01/13 13:21:39]

Hannes, ptal at this if () {} block: here I'm depending on the fact that strings
in LO space won't move, so I can parse off of them on bg threads.

[Hannes Payer (out of office), 2017/01/13 14:01:04]

Correct, LO don't move.

[rmcilroy, 2017/01/13 15:56:22]

Is it common for the source to be in the LO space? I'm guessing the source is
usually an external string, and would only be on the heap for eval functions (am
I missing other situations?), and therefore would only be in the LO space if
it's a 4KB eval - I'm guessing this isn't super common - is it worth the added
complexity here?

[jochen (gone - plz use gerrit), 2017/01/13 16:14:40]

no, I expect most cases to be external strings, or small evals. However, if
somebody implements a module loading using eval, we might hit this.

I think it's preferable to just always be able to run on bg threads

+      // We need to globalize the handle to the flattened string here, in
+      // case it's not referenced from anywhere else.
+      source_ =
+          Handle<String>::cast(isolate_->global_handles()->Create(*source));
+      DisallowHeapAllocation no_allocation;
+      String::FlatContent content = source->GetFlatContent();
+      DCHECK(content.IsFlat());
+      data =
+          content.IsOneByte()
+              ? reinterpret_cast<const void*>(content.ToOneByteVector().start())
+              : reinterpret_cast<const void*>(content.ToUC16Vector().start());
+    } else {
+      // Otherwise, create a copy of the part of the string we'll parse in the
+      // zone.
+      length = (shared_->end_position() - shared_->start_position());
+      offset = shared_->start_position();
+
+      int byte_len = length * (source->IsOneByteRepresentation() ? 1 : 2);
+      data = zone_->New(byte_len);

[vogelheim, 2017/01/13 15:20:22]

Who will delete this?

As I see it, it get pointed to from an External(One|Two)ByteString, but I
thought those typically don't delete their memory (as they don't normally own
it), so... where does this get deleted?

[jochen (gone - plz use gerrit), 2017/01/13 16:14:40]

the zone_ will delete this

+
+      DisallowHeapAllocation no_allocation;
+      String::FlatContent content = source->GetFlatContent();
+      DCHECK(content.IsFlat());
+      if (content.IsOneByte()) {
+        MemCopy(const_cast<void*>(data),
+                &content.ToOneByteVector().at(shared_->start_position()),
+                byte_len);
+      } else {
+        MemCopy(const_cast<void*>(data),
+                &content.ToUC16Vector().at(shared_->start_position()),
+                byte_len);
+      }
+    }
+    Handle<String> wrapper;
+    if (source->IsOneByteRepresentation()) {
+      ExternalOneByteString::Resource* resource =
+          new OneByteWrapper(data, length);
+      source_wrapper_.reset(resource);
+      wrapper = isolate_->factory()
+                    ->NewExternalStringFromOneByte(resource)
+                    .ToHandleChecked();
+    } else {
+      ExternalTwoByteString::Resource* resource =
+          new TwoByteWrapper(data, length);
+      source_wrapper_.reset(resource);
+      wrapper = isolate_->factory()
+                    ->NewExternalStringFromTwoByte(resource)
+                    .ToHandleChecked();
+    }
+    wrapper_ =
+        Handle<String>::cast(isolate_->global_handles()->Create(*wrapper));
+
+    character_stream_.reset(
+        ScannerStream::For(wrapper_, shared_->start_position() - offset,
+                           shared_->end_position() - offset));
   }
   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_is_named_expression(shared_->is_named_expression());
   parse_info_->set_compiler_hints(shared_->compiler_hints());
   parse_info_->set_start_position(shared_->start_position());
   parse_info_->set_end_position(shared_->end_position());
   parse_info_->set_unicode_cache(unicode_cache_.get());
   parse_info_->set_language_mode(shared_->language_mode());
   parse_info_->set_function_literal_id(shared_->function_literal_id());
 
   parser_.reset(new Parser(parse_info_.get()));
   Handle<ScopeInfo> outer_scope_info(
       handle(ScopeInfo::cast(shared_->outer_scope_info())));
   parser_->DeserializeScopeChain(parse_info_.get(),
                                  outer_scope_info->length() > 0
                                      ? MaybeHandle<ScopeInfo>(outer_scope_info)
                                      : MaybeHandle<ScopeInfo>());
 
   Handle<String> name(String::cast(shared_->name()));
   parse_info_->set_function_name(
       parse_info_->ast_value_factory()->GetString(name));
   status_ = CompileJobStatus::kReadyToParse;
 }
 
 void CompilerDispatcherJob::Parse() {
-  DCHECK(can_parse_on_background_thread_ ||
-         ThreadId::Current().Equals(isolate_->thread_id()));
   DCHECK(status() == CompileJobStatus::kReadyToParse);
   COMPILER_DISPATCHER_TRACE_SCOPE_WITH_NUM(
       tracer_, kParse,
       parse_info_->end_position() - parse_info_->start_position());
   if (trace_compiler_dispatcher_jobs_) {
     PrintF("CompilerDispatcherJob[%p]: Parsing\n", static_cast<void*>(this));
   }
 
   DisallowHeapAllocation no_allocation;
   DisallowHandleAllocation no_handles;
-  std::unique_ptr<DisallowHandleDereference> no_deref;
-  // If we can't parse on a background thread, we need to be able to deref the
-  // source string.
-  if (can_parse_on_background_thread_) {
-    no_deref.reset(new DisallowHandleDereference());
-  }
+  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 = GetCurrentStackPosition() - max_stack_size_ * KB;
 
   parser_->set_stack_limit(stack_limit);
   parser_->ParseOnBackground(parse_info_.get());
 
   parse_info_->set_isolate(isolate_);
 
   status_ = CompileJobStatus::kParsed;
 }
 
 bool CompilerDispatcherJob::FinalizeParsingOnMainThread() {
   DCHECK(ThreadId::Current().Equals(isolate_->thread_id()));
   DCHECK(status() == CompileJobStatus::kParsed);
   COMPILER_DISPATCHER_TRACE_SCOPE(tracer_, kFinalizeParsing);
   if (trace_compiler_dispatcher_jobs_) {
     PrintF("CompilerDispatcherJob[%p]: Finalizing parsing\n",
            static_cast<void*>(this));
   }
 
   if (!source_.is_null()) {
     i::GlobalHandles::Destroy(Handle<Object>::cast(source_).location());
     source_ = Handle<String>::null();
   }
+  if (!wrapper_.is_null()) {
+    i::GlobalHandles::Destroy(Handle<Object>::cast(wrapper_).location());
+    wrapper_ = Handle<String>::null();
+  }
 
   if (parse_info_->literal() == nullptr) {
     status_ = CompileJobStatus::kFailed;
   } else {
     status_ = CompileJobStatus::kReadyToAnalyse;
   }
 
   DeferredHandleScope scope(isolate_);
   {
     Handle<Script> script(Script::cast(shared_->script()), isolate_);
@@ skipping 40 matching lines @@
         Compiler::PrepareUnoptimizedCompilationJob(compile_info_.get()));
   }
   compile_info_->set_deferred_handles(scope.Detach());
 
   if (!compile_job_.get()) {
     if (!isolate_->has_pending_exception()) isolate_->StackOverflow();
     status_ = CompileJobStatus::kFailed;
     return false;
   }
 
-  can_compile_on_background_thread_ =
-      compile_job_->can_execute_on_background_thread();
+  CHECK(compile_job_->can_execute_on_background_thread());
   status_ = CompileJobStatus::kReadyToCompile;
   return true;
 }
 
 void CompilerDispatcherJob::Compile() {
   DCHECK(status() == CompileJobStatus::kReadyToCompile);
-  DCHECK(can_compile_on_background_thread_ ||
-         ThreadId::Current().Equals(isolate_->thread_id()));
   COMPILER_DISPATCHER_TRACE_SCOPE_WITH_NUM(
       tracer_, kCompile, parse_info_->literal()->ast_node_count());
   if (trace_compiler_dispatcher_jobs_) {
     PrintF("CompilerDispatcherJob[%p]: Compiling\n", static_cast<void*>(this));
   }
 
   // Disallowing of handle dereference and heap access dealt with in
   // CompilationJob::ExecuteJob.
 
   uintptr_t stack_limit = GetCurrentStackPosition() - max_stack_size_ * KB;
@@ skipping 46 matching lines @@
   parse_info_.reset();
   handles_from_parsing_.reset();
   compile_info_.reset();
   compile_job_.reset();
   zone_.reset();
 
   if (!source_.is_null()) {
     i::GlobalHandles::Destroy(Handle<Object>::cast(source_).location());
     source_ = Handle<String>::null();
   }
+  if (!wrapper_.is_null()) {
+    i::GlobalHandles::Destroy(Handle<Object>::cast(wrapper_).location());
+    wrapper_ = Handle<String>::null();
+  }
 
   status_ = CompileJobStatus::kInitial;
 }
 
 double CompilerDispatcherJob::EstimateRuntimeOfNextStepInMs() const {
   switch (status_) {
     case CompileJobStatus::kInitial:
       return tracer_->EstimatePrepareToParseInMs();
 
     case CompileJobStatus::kReadyToParse:
@@ skipping 22 matching lines @@
   return 0.0;
 }
 
 void CompilerDispatcherJob::ShortPrint() {
   DCHECK(ThreadId::Current().Equals(isolate_->thread_id()));
   shared_->ShortPrint();
 }
 
 }  // namespace internal
 }  // namespace v8