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

This reverts commit a441cbfb.

Reason for revert: causes DeepEagerCompilationPeakMemory to fail.

https://ci.chromium.org/p/v8/builders/ci/V8%20Linux/36681

Original change's description:
> [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}

TBR=rmcilroy@chromium.org,leszeks@chromium.org

Change-Id: I8716b332b07fe4f394b5a32c986bbe652325582d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:1011762
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2163143Reviewed-by: Bill Budge <bbudge@chromium.org>
Commit-Queue: Bill Budge <bbudge@chromium.org>
Cr-Commit-Position: refs/heads/master@{#67347}

src/codegen/compiler.cc

@@ -42,7 +42,6 @@
 #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,127 +2206,6 @@ 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(
@@ -2399,28 +2277,26 @@ MaybeHandle<SharedFunctionInfo> Compiler::GetSharedFunctionInfoForScript(
 
   if (maybe_result.is_null()) {
     // No cache entry found compile the script.
-    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);
+    UnoptimizedCompileFlags flags = UnoptimizedCompileFlags::ForToplevelCompile(
+        isolate, natives == NOT_NATIVES_CODE, language_mode,
+        script_details.repl_mode);
 
-      flags.set_is_eager(compile_options == ScriptCompiler::kEagerCompile);
-      flags.set_is_module(origin_options.IsModule());
+    flags.set_is_module(origin_options.IsModule());
+    flags.set_is_eager(compile_options == ScriptCompiler::kEagerCompile);
 
-      maybe_result = CompileScriptOnMainThread(
-          flags, source, script_details, origin_options, natives, extension,
-          isolate, &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());
 
-    // Add the result to the isolate cache.
+    // Compile the function and add it to the isolate cache.
+    maybe_result =
+        CompileToplevel(&parse_info, script, isolate, &is_compiled_scope);
     Handle<SharedFunctionInfo> result;
     if (extension == nullptr && maybe_result.ToHandle(&result)) {
       DCHECK(is_compiled_scope.is_compiled());

src/codegen/compiler.h

@@ -383,7 +383,6 @@ 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,6 +452,54 @@ 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());
@@ -542,6 +590,23 @@ 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,
@@ -2846,6 +2911,13 @@ 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,6 +278,7 @@ 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,8 +1091,6 @@ 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,9 +228,7 @@ 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);
-  }
+  impl()->AddToScriptList(script);
 
   LOG(isolate(), ScriptEvent(Logger::ScriptEventType::kCreate, script_id));
   return script;

src/objects/script.h

@@ -22,10 +22,6 @@ 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, Script::kTemporaryScriptId);
+  return UnoptimizedCompileFlags(isolate, -1);
 }
 
 template <typename T>
@@ -235,8 +235,7 @@ 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(flags().script_id() >= 0 ||
-         flags().script_id() == Script::kTemporaryScriptId);
+  DCHECK_GE(flags().script_id(), 0);
   Handle<Script> script =
       isolate->factory()->NewScriptWithId(source, flags().script_id());
   if (isolate->NeedsSourcePositionsForProfiling()) {

test/cctest/test-log.cc

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