[offthread] Move stress-background-compile to compiler.cc

Make --stress-background-compile a V8 flag rather than a d8 flag, so
that it also tests unittests/cctests.

Now, with this flag, every top-level script compile (that fulfills a
couple of restrictions) will be both main-thread and background-thread
compiled, taking the result of the background compile. In the future,
we'll probably want to verify that the two results are equivalent.

One of the necessary changes to allow tests to pass was to introduce a
concept of a "temporary" script (with a temporary script id), which
doesn't get added to the script list. This is to avoid the main-thread
compile part of the stress-test having a debugger-visible side-effect,
e.g. in tests that enumerate scripts. We can't just create new ids for
such scripts, as then script-id expectation files no longer match.

Bug: chromium:1011762
Change-Id: I500bbf2cabea762e69aca3dbae247daae71192cb
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2120541
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#67332}

src/codegen/compiler.cc

@@ -42,6 +42,7 @@
 #include "src/objects/map.h"
 #include "src/objects/object-list-macros.h"
 #include "src/objects/shared-function-info.h"
+#include "src/objects/string.h"
 #include "src/parsing/parse-info.h"
 #include "src/parsing/parser.h"
 #include "src/parsing/parsing.h"
@@ -2207,6 +2208,127 @@ void FixUpOffThreadAllocatedScript(Isolate* isolate, Handle<Script> script,
   LOG(isolate, ScriptDetails(*script));
 }
 
+MaybeHandle<SharedFunctionInfo> CompileScriptOnMainThread(
+    const UnoptimizedCompileFlags flags, Handle<String> source,
+    const Compiler::ScriptDetails& script_details,
+    ScriptOriginOptions origin_options, NativesFlag natives,
+    v8::Extension* extension, Isolate* isolate,
+    IsCompiledScope* is_compiled_scope) {
+  UnoptimizedCompileState compile_state(isolate);
+  ParseInfo parse_info(isolate, flags, &compile_state);
+  parse_info.set_extension(extension);
+
+  Handle<Script> script = NewScript(isolate, &parse_info, source,
+                                    script_details, origin_options, natives);
+  DCHECK_IMPLIES(parse_info.flags().collect_type_profile(),
+                 script->IsUserJavaScript());
+  DCHECK_EQ(parse_info.flags().is_repl_mode(), script->is_repl_mode());
+
+  return CompileToplevel(&parse_info, script, isolate, is_compiled_scope);
+}
+
+class StressBackgroundCompileThread : public base::Thread {
+ public:
+  StressBackgroundCompileThread(Isolate* isolate, Handle<String> source)
+      : base::Thread(
+            base::Thread::Options("StressBackgroundCompileThread", 2 * i::MB)),
+        source_(source),
+        streamed_source_(std::make_unique<SourceStream>(source, isolate),
+                         v8::ScriptCompiler::StreamedSource::UTF8) {
+    data()->task = std::make_unique<i::BackgroundCompileTask>(data(), isolate);
+  }
+
+  void Run() override { data()->task->Run(); }
+
+  ScriptStreamingData* data() { return streamed_source_.impl(); }
+
+ private:
+  // Dummy external source stream which returns the whole source in one go.
+  // TODO(leszeks): Also test chunking the data.
+  class SourceStream : public v8::ScriptCompiler::ExternalSourceStream {
+   public:
+    SourceStream(Handle<String> source, Isolate* isolate) : done_(false) {
+      source_buffer_ = source->ToCString(ALLOW_NULLS, FAST_STRING_TRAVERSAL,
+                                         &source_length_);
+    }
+
+    size_t GetMoreData(const uint8_t** src) override {
+      if (done_) {
+        return 0;
+      }
+      *src = reinterpret_cast<uint8_t*>(source_buffer_.release());
+      done_ = true;
+
+      return source_length_;
+    }
+
+   private:
+    int source_length_;
+    std::unique_ptr<char[]> source_buffer_;
+    bool done_;
+  };
+
+  Handle<String> source_;
+  v8::ScriptCompiler::StreamedSource streamed_source_;
+};
+
+bool CanBackgroundCompile(const Compiler::ScriptDetails& script_details,
+                          ScriptOriginOptions origin_options,
+                          v8::Extension* extension,
+                          ScriptCompiler::CompileOptions compile_options,
+                          NativesFlag natives) {
+  // TODO(leszeks): Remove the module check once background compilation of
+  // modules is supported.
+  return !origin_options.IsModule() && !extension &&
+         script_details.repl_mode == REPLMode::kNo &&
+         compile_options == ScriptCompiler::kNoCompileOptions &&
+         natives == NOT_NATIVES_CODE;
+}
+
+MaybeHandle<SharedFunctionInfo> CompileScriptOnBothBackgroundAndMainThread(
+    Handle<String> source, const Compiler::ScriptDetails& script_details,
+    ScriptOriginOptions origin_options, Isolate* isolate,
+    IsCompiledScope* is_compiled_scope) {
+  // Start a background thread compiling the script.
+  StressBackgroundCompileThread background_compile_thread(isolate, source);
+
+  UnoptimizedCompileFlags flags_copy =
+      background_compile_thread.data()->task->flags();
+
+  CHECK(background_compile_thread.Start());
+  MaybeHandle<SharedFunctionInfo> main_thread_maybe_result;
+  // In parallel, compile on the main thread to flush out any data races.
+  {
+    IsCompiledScope inner_is_compiled_scope;
+    // The background thread should also create any relevant exceptions, so we
+    // can ignore the main-thread created ones.
+    // TODO(leszeks): Maybe verify that any thrown (or unthrown) exceptions are
+    // equivalent.
+    TryCatch ignore_try_catch(reinterpret_cast<v8::Isolate*>(isolate));
+    flags_copy.set_script_id(Script::kTemporaryScriptId);
+    main_thread_maybe_result = CompileScriptOnMainThread(
+        flags_copy, source, script_details, origin_options, NOT_NATIVES_CODE,
+        nullptr, isolate, &inner_is_compiled_scope);
+  }
+  // Join with background thread and finalize compilation.
+  background_compile_thread.Join();
+  MaybeHandle<SharedFunctionInfo> maybe_result =
+      Compiler::GetSharedFunctionInfoForStreamedScript(
+          isolate, source, script_details, origin_options,
+          background_compile_thread.data());
+
+  // Either both compiles should succeed, or both should fail.
+  // TODO(leszeks): Compare the contents of the results of the two compiles.
+  CHECK_EQ(maybe_result.is_null(), main_thread_maybe_result.is_null());
+
+  Handle<SharedFunctionInfo> result;
+  if (maybe_result.ToHandle(&result)) {
+    *is_compiled_scope = result->is_compiled_scope();
+  }
+
+  return maybe_result;
+}
+
 }  // namespace
 
 MaybeHandle<SharedFunctionInfo> Compiler::GetSharedFunctionInfoForScript(
@@ -2278,26 +2400,28 @@ MaybeHandle<SharedFunctionInfo> Compiler::GetSharedFunctionInfoForScript(
 
   if (maybe_result.is_null()) {
     // No cache entry found compile the script.
-    UnoptimizedCompileFlags flags = UnoptimizedCompileFlags::ForToplevelCompile(
+    if (FLAG_stress_background_compile &&
+        CanBackgroundCompile(script_details, origin_options, extension,
+                             compile_options, natives)) {
+      // If the --stress-background-compile flag is set, do the actual
+      // compilation on a background thread, and wait for its result.
+      maybe_result = CompileScriptOnBothBackgroundAndMainThread(
+          source, script_details, origin_options, isolate, &is_compiled_scope);
+    } else {
+      UnoptimizedCompileFlags flags =
+          UnoptimizedCompileFlags::ForToplevelCompile(
               isolate, natives == NOT_NATIVES_CODE, language_mode,
               script_details.repl_mode);
 
-    flags.set_is_module(origin_options.IsModule());
       flags.set_is_eager(compile_options == ScriptCompiler::kEagerCompile);
+      flags.set_is_module(origin_options.IsModule());
 
-    UnoptimizedCompileState compile_state(isolate);
-    ParseInfo parse_info(isolate, flags, &compile_state);
-    parse_info.set_extension(extension);
-
-    Handle<Script> script = NewScript(isolate, &parse_info, source,
-                                      script_details, origin_options, natives);
-    DCHECK_IMPLIES(parse_info.flags().collect_type_profile(),
-                   script->IsUserJavaScript());
-    DCHECK_EQ(parse_info.flags().is_repl_mode(), script->is_repl_mode());
+      maybe_result = CompileScriptOnMainThread(
+          flags, source, script_details, origin_options, natives, extension,
+          isolate, &is_compiled_scope);
+    }
 
-    // Compile the function and add it to the isolate cache.
-    maybe_result =
-        CompileToplevel(&parse_info, script, isolate, &is_compiled_scope);
+    // Add the result to the isolate cache.
     Handle<SharedFunctionInfo> result;
     if (extension == nullptr && maybe_result.ToHandle(&result)) {
       DCHECK(is_compiled_scope.is_compiled());

src/codegen/compiler.h

@@ -382,6 +382,7 @@ class V8_EXPORT_PRIVATE BackgroundCompileTask {
   UnoptimizedCompilationJobList* inner_function_jobs() {
     return &inner_function_jobs_;
   }
+  UnoptimizedCompileFlags flags() const { return flags_; }
   LanguageMode language_mode() { return language_mode_; }
   bool collected_source_positions() { return collected_source_positions_; }
   bool finalize_on_background_thread() {

src/d8/d8.cc

@@ -452,54 +452,6 @@ ArrayBuffer::Allocator* Shell::array_buffer_allocator;
 ShellOptions Shell::options;
 base::OnceType Shell::quit_once_ = V8_ONCE_INIT;
 
-// Dummy external source stream which returns the whole source in one go.
-class DummySourceStream : public v8::ScriptCompiler::ExternalSourceStream {
- public:
-  DummySourceStream(Local<String> source, Isolate* isolate) : done_(false) {
-    source_length_ = source->Utf8Length(isolate);
-    source_buffer_.reset(new uint8_t[source_length_]);
-    source->WriteUtf8(isolate, reinterpret_cast<char*>(source_buffer_.get()),
-                      source_length_);
-  }
-
-  size_t GetMoreData(const uint8_t** src) override {
-    if (done_) {
-      return 0;
-    }
-    *src = source_buffer_.release();
-    done_ = true;
-
-    return source_length_;
-  }
-
- private:
-  int source_length_;
-  std::unique_ptr<uint8_t[]> source_buffer_;
-  bool done_;
-};
-
-class BackgroundCompileThread : public base::Thread {
- public:
-  BackgroundCompileThread(Isolate* isolate, Local<String> source)
-      : base::Thread(GetThreadOptions("BackgroundCompileThread")),
-        source_(source),
-        streamed_source_(std::make_unique<DummySourceStream>(source, isolate),
-                         v8::ScriptCompiler::StreamedSource::UTF8),
-        task_(v8::ScriptCompiler::StartStreamingScript(isolate,
-                                                       &streamed_source_)) {}
-
-  void Run() override { task_->Run(); }
-
-  v8::ScriptCompiler::StreamedSource* streamed_source() {
-    return &streamed_source_;
-  }
-
- private:
-  Local<String> source_;
-  v8::ScriptCompiler::StreamedSource streamed_source_;
-  std::unique_ptr<v8::ScriptCompiler::ScriptStreamingTask> task_;
-};
-
 ScriptCompiler::CachedData* Shell::LookupCodeCache(Isolate* isolate,
                                                    Local<Value> source) {
   base::MutexGuard lock_guard(cached_code_mutex_.Pointer());
@@ -590,23 +542,6 @@ bool Shell::ExecuteString(Isolate* isolate, Local<String> source,
         maybe_script = ScriptCompiler::Compile(
             context, &script_source, ScriptCompiler::kNoCompileOptions);
       }
-    } else if (options.stress_background_compile) {
-      // Start a background thread compiling the script.
-      BackgroundCompileThread background_compile_thread(isolate, source);
-      CHECK(background_compile_thread.Start());
-
-      // In parallel, compile on the main thread to flush out any data races.
-      {
-        TryCatch ignore_try_catch(isolate);
-        ScriptCompiler::Source script_source(source, origin);
-        USE(ScriptCompiler::Compile(context, &script_source,
-                                    ScriptCompiler::kNoCompileOptions));
-      }
-
-      // Join with background thread and finalize compilation.
-      background_compile_thread.Join();
-      maybe_script = v8::ScriptCompiler::Compile(
-          context, background_compile_thread.streamed_source(), source, origin);
     } else {
       ScriptCompiler::Source script_source(source, origin);
       maybe_script = ScriptCompiler::Compile(context, &script_source,
@@ -2911,13 +2846,6 @@ bool Shell::SetOptions(int argc, char* argv[]) {
                strcmp(argv[i], "--no-stress-opt") == 0) {
       options.stress_opt = false;
       argv[i] = nullptr;
-    } else if (strcmp(argv[i], "--stress-background-compile") == 0) {
-      options.stress_background_compile = true;
-      argv[i] = nullptr;
-    } else if (strcmp(argv[i], "--nostress-background-compile") == 0 ||
-               strcmp(argv[i], "--no-stress-background-compile") == 0) {
-      options.stress_background_compile = false;
-      argv[i] = nullptr;
     } else if (strcmp(argv[i], "--noalways-opt") == 0 ||
                strcmp(argv[i], "--no-always-opt") == 0) {
       // No support for stressing if we can't use --always-opt.

src/d8/d8.h

@@ -278,7 +278,6 @@ class ShellOptions {
   int num_isolates = 1;
   v8::ScriptCompiler::CompileOptions compile_options =
       v8::ScriptCompiler::kNoCompileOptions;
-  bool stress_background_compile = false;
   CodeCacheOptions code_cache_options = CodeCacheOptions::kNoProduceCache;
   SourceGroup* isolate_sources = nullptr;
   const char* icu_data_file = nullptr;

src/flags/flag-definitions.h

@@ -1091,6 +1091,8 @@ DEFINE_BOOL(enable_regexp_unaligned_accesses, true,
 
 // api.cc
 DEFINE_BOOL(script_streaming, true, "enable parsing on background")
+DEFINE_BOOL(stress_background_compile, false,
+            "stress test parsing on background")
 DEFINE_BOOL(
     finalize_streaming_on_background, false,
     "perform the script streaming finalization on the background thread")

src/heap/factory-base.cc

@@ -228,7 +228,9 @@ Handle<Script> FactoryBase<Impl>::NewScriptWithId(Handle<String> source,
   script->set_flags(0);
   script->set_host_defined_options(roots.empty_fixed_array());
 
+  if (script_id != Script::kTemporaryScriptId) {
     impl()->AddToScriptList(script);
+  }
 
   LOG(isolate(), ScriptEvent(Logger::ScriptEventType::kCreate, script_id));
   return script;

src/objects/script.h

@@ -22,6 +22,10 @@ namespace internal {
 // Script describes a script which has been added to the VM.
 class Script : public Struct {
  public:
+  // Script ID used for temporary scripts, which shouldn't be added to the
+  // script list.
+  static constexpr int kTemporaryScriptId = -2;
+
   NEVER_READ_ONLY_SPACE
   // Script types.
   enum Type {

src/parsing/parse-info.cc

@@ -116,7 +116,7 @@ UnoptimizedCompileFlags UnoptimizedCompileFlags::ForToplevelFunction(
 
 // static
 UnoptimizedCompileFlags UnoptimizedCompileFlags::ForTest(Isolate* isolate) {
-  return UnoptimizedCompileFlags(isolate, -1);
+  return UnoptimizedCompileFlags(isolate, Script::kTemporaryScriptId);
 }
 
 template <typename T>
@@ -235,7 +235,8 @@ Handle<Script> ParseInfo::CreateScript(
     MaybeHandle<FixedArray> maybe_wrapped_arguments,
     ScriptOriginOptions origin_options, NativesFlag natives) {
   // Create a script object describing the script to be compiled.
-  DCHECK_GE(flags().script_id(), 0);
+  DCHECK(flags().script_id() >= 0 ||
+         flags().script_id() == Script::kTemporaryScriptId);
   Handle<Script> script =
       isolate->factory()->NewScriptWithId(source, flags().script_id());
   if (isolate->NeedsSourcePositionsForProfiling()) {

test/cctest/test-log.cc

@@ -721,8 +721,10 @@ UNINITIALIZED_TEST(ExternalCodeEventListenerInnerFunctions) {
     v8::Local<v8::UnboundScript> script =
         v8::ScriptCompiler::CompileUnboundScript(isolate1, &source)
             .ToLocalChecked();
-    CHECK_EQ(code_event_handler.CountLines("Script", "f1"), 1);
-    CHECK_EQ(code_event_handler.CountLines("Script", "f2"), 1);
+    CHECK_EQ(code_event_handler.CountLines("Script", "f1"),
+             i::FLAG_stress_background_compile ? 2 : 1);
+    CHECK_EQ(code_event_handler.CountLines("Script", "f2"),
+             i::FLAG_stress_background_compile ? 2 : 1);
     cache = v8::ScriptCompiler::CreateCodeCache(script);
   }
   isolate1->Dispose();