Reland "[RCS] Create thread local runtime call stats tables for worker threads"

This is a reland of 1e5b6d99

Original change's description:
> [RCS] Create thread local runtime call stats tables for worker threads
>
> Creating a runtime call stats table for each worker thread task is expensive.
> Instead we create a single table per thread, and use TLS to get the correct one
> when starting a worker thread task.
>
> In order to correctly initialize the parser, scanner and parse-info's runtime
> call stats fields, we move creation of the scanner and parser onto the
> background tasks for BackgroundCompileTask and UnoptimizedCompilationJob.
>
> Change-Id: I36064c7fb43290968620b1985cc02637b16f4232
> Reviewed-on: https://chromium-review.googlesource.com/1187522
> Reviewed-by: Toon Verwaest <verwaest@chromium.org>
> Commit-Queue: Ross McIlroy <rmcilroy@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#55448}

TBR=verwaest@chromium.org

Change-Id: I7c2f48280651d7632d99ee22d86260c72bc16185
Reviewed-on: https://chromium-review.googlesource.com/1193463
Commit-Queue: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#55456}

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

@@ -73,6 +73,8 @@ UnoptimizedCompileJob::UnoptimizedCompileJob(Isolate* isolate,
       allocator_(isolate->allocator()),
       context_(isolate->global_handles()->Create(isolate->context())),
       shared_(isolate->global_handles()->Create(*shared)),
+      worker_thread_runtime_stats_(
+          isolate->counters()->worker_thread_runtime_call_stats()),
       max_stack_size_(max_stack_size),
       trace_compiler_dispatcher_jobs_(FLAG_trace_compiler_dispatcher_jobs) {
   DCHECK(!shared_->is_toplevel());
@@ -122,10 +124,6 @@ void UnoptimizedCompileJob::PrepareOnMainThread(Isolate* isolate) {
   unicode_cache_.reset(new UnicodeCache());
   parse_info_->set_unicode_cache(unicode_cache_.get());
   parse_info_->set_function_literal_id(shared_->FunctionLiteralId(isolate));
-  if (V8_UNLIKELY(FLAG_runtime_stats)) {
-    parse_info_->set_runtime_call_stats(new (parse_info_->zone())
-                                            RuntimeCallStats());
-  }
 
   Handle<Script> script = parse_info->script();
   HandleScope scope(isolate);
@@ -195,16 +193,17 @@ void UnoptimizedCompileJob::PrepareOnMainThread(Isolate* isolate) {
         ScannerStream::For(isolate, wrapper_, shared_->StartPosition() - offset,
                            shared_->EndPosition() - offset));
     parse_info_->set_character_stream(std::move(stream));
+    // Set script to null in parse_info so that it's not dereferenced on the
+    // background thread.
   }
 
-  parser_.reset(new Parser(parse_info_.get()));
-  parser_->DeserializeScopeChain(isolate, parse_info_.get(),
-                                 parse_info_->maybe_outer_scope_info());
-
   // Initailize the name after setting up the ast_value_factory.
   Handle<String> name(shared_->Name(), isolate);
   parse_info_->set_function_name(
-      parse_info_->ast_value_factory()->GetString(name));
+      parse_info_->GetOrCreateAstValueFactory()->GetString(name));
+  // Clear the parse info's script handle to ensure it's not dereferenced
+  // on the background thread.
+  parse_info->ClearScriptHandle();
 
   set_status(Status::kPrepared);
 }
@@ -223,9 +222,19 @@ void UnoptimizedCompileJob::Compile(bool on_background_thread) {
   DisallowHandleDereference no_deref;
 
   parse_info_->set_on_background_thread(on_background_thread);
+
+  base::Optional<WorkerThreadRuntimeCallStatsScope> runtime_call_stats_scope;
+  if (V8_UNLIKELY(FLAG_runtime_stats && on_background_thread)) {
+    runtime_call_stats_scope.emplace(worker_thread_runtime_stats_);
+    parse_info_->set_runtime_call_stats(runtime_call_stats_scope->Get());
+  }
+
   uintptr_t stack_limit = GetCurrentStackPosition() - max_stack_size_ * KB;
-  parser_->set_stack_limit(stack_limit);
   parse_info_->set_stack_limit(stack_limit);
+
+  parser_.reset(new Parser(parse_info_.get()));
+  parser_->set_stack_limit(stack_limit);
+  parser_->InitializeEmptyScopeChain(parse_info_.get());
   parser_->ParseOnBackground(parse_info_.get());
 
   if (parse_info_->literal() == nullptr) {
@@ -271,10 +280,9 @@ void UnoptimizedCompileJob::FinalizeOnMainThread(Isolate* isolate) {
   }
 
   Handle<Script> script(Script::cast(shared_->script()), isolate);
-  DCHECK_EQ(*parse_info_->script(), shared_->script());
+  parse_info_->set_script(script);
 
   parser_->UpdateStatistics(isolate, script);
-  parse_info_->UpdateBackgroundParseStatisticsOnMainThread(isolate);
   parser_->HandleSourceURLComments(isolate, script);
 
   {

src/compiler-dispatcher/unoptimized-compile-job.h

@@ -28,6 +28,7 @@ class String;
 class UnicodeCache;
 class UnoptimizedCompilationJob;
 class Utf16CharacterStream;
+class WorkerThreadRuntimeCallStats;
 
 class V8_EXPORT_PRIVATE UnoptimizedCompileJob : public CompilerDispatcherJob {
  public:
@@ -67,6 +68,7 @@ class V8_EXPORT_PRIVATE UnoptimizedCompileJob : public CompilerDispatcherJob {
   Handle<SharedFunctionInfo> shared_;  // Global handle.
   Handle<String> source_;              // Global handle.
   Handle<String> wrapper_;             // Global handle.
+  WorkerThreadRuntimeCallStats* worker_thread_runtime_stats_;
   size_t max_stack_size_;
 
   // Members required for parsing.

src/compiler.cc

@@ -942,6 +942,7 @@ class BackgroundCompileTask : public ScriptCompiler::ScriptStreamingTask {
  private:
   ScriptStreamingData* source_;  // Not owned.
   int stack_size_;
+  WorkerThreadRuntimeCallStats* worker_thread_runtime_call_stats_;
   AccountingAllocator* allocator_;
   TimedHistogram* timer_;
 
@@ -952,6 +953,8 @@ BackgroundCompileTask::BackgroundCompileTask(ScriptStreamingData* source,
                                              Isolate* isolate)
     : source_(source),
       stack_size_(i::FLAG_stack_size),
+      worker_thread_runtime_call_stats_(
+          isolate->counters()->worker_thread_runtime_call_stats()),
       timer_(isolate->counters()->compile_script_on_background()) {
   VMState<PARSER> state(isolate);
 
@@ -960,16 +963,7 @@ BackgroundCompileTask::BackgroundCompileTask(ScriptStreamingData* source,
   ParseInfo* info = new ParseInfo(isolate);
   LOG(isolate, ScriptEvent(Logger::ScriptEventType::kStreamingCompile,
                            info->script_id()));
-  if (V8_UNLIKELY(FLAG_runtime_stats)) {
-    info->set_runtime_call_stats(new (info->zone()) RuntimeCallStats());
-  } else {
-    info->set_runtime_call_stats(nullptr);
-  }
   info->set_toplevel();
-  std::unique_ptr<Utf16CharacterStream> stream(
-      ScannerStream::For(source->source_stream.get(), source->encoding,
-                         info->runtime_call_stats()));
-  info->set_character_stream(std::move(stream));
   info->set_unicode_cache(&source_->unicode_cache);
   info->set_allow_lazy_parsing();
   if (V8_UNLIKELY(info->block_coverage_enabled())) {
@@ -981,12 +975,6 @@ BackgroundCompileTask::BackgroundCompileTask(ScriptStreamingData* source,
 
   source->info.reset(info);
   allocator_ = isolate->allocator();
-
-  // Parser needs to stay alive for finalizing the parsing on the main
-  // thread.
-  source_->parser.reset(new Parser(source_->info.get()));
-  source_->parser->DeserializeScopeChain(isolate, source_->info.get(),
-                                         MaybeHandle<ScopeInfo>());
 }
 
 void BackgroundCompileTask::Run() {
@@ -995,12 +983,29 @@ void BackgroundCompileTask::Run() {
 
   source_->info->set_on_background_thread(true);
 
+  // Get a runtime call stats table associated with the current worker thread.
+  WorkerThreadRuntimeCallStatsScope runtime_call_stats_scope(
+      worker_thread_runtime_call_stats_);
+  RuntimeCallStats* old_runtime_call_stats =
+      source_->info->runtime_call_stats();
+  source_->info->set_runtime_call_stats(runtime_call_stats_scope.Get());
+
   // Reset the stack limit of the parser to reflect correctly that we're on a
   // background thread.
   uintptr_t old_stack_limit = source_->info->stack_limit();
   uintptr_t stack_limit = GetCurrentStackPosition() - stack_size_ * KB;
   source_->info->set_stack_limit(stack_limit);
+
+  std::unique_ptr<Utf16CharacterStream> stream(
+      ScannerStream::For(source_->source_stream.get(), source_->encoding,
+                         source_->info->runtime_call_stats()));
+  source_->info->set_character_stream(std::move(stream));
+
+  // Parser needs to stay alive for finalizing the parsing on the main
+  // thread.
+  source_->parser.reset(new Parser(source_->info.get()));
   source_->parser->set_stack_limit(stack_limit);
+  source_->parser->InitializeEmptyScopeChain(source_->info.get());
 
   source_->parser->ParseOnBackground(source_->info.get());
   if (source_->info->literal() != nullptr) {
@@ -1009,10 +1014,9 @@ void BackgroundCompileTask::Run() {
         source_->info.get(), allocator_, &source_->inner_function_jobs);
   }
 
-  source_->info->EmitBackgroundParseStatisticsOnBackgroundThread();
-
-  source_->info->set_on_background_thread(false);
   source_->info->set_stack_limit(old_stack_limit);
+  source_->info->set_runtime_call_stats(old_runtime_call_stats);
+  source_->info->set_on_background_thread(false);
 }
 
 }  // namespace
@@ -1773,8 +1777,6 @@ Compiler::GetSharedFunctionInfoForStreamedScript(
   isolate->counters()->total_compile_size()->Increment(source_length);
 
   ParseInfo* parse_info = streaming_data->info.get();
-  parse_info->UpdateBackgroundParseStatisticsOnMainThread(isolate);
-
   // Check if compile cache already holds the SFI, if so no need to finalize
   // the code compiled on the background thread.
   CompilationCache* compilation_cache = isolate->compilation_cache();

src/counters.cc

@@ -118,7 +118,8 @@ Counters::Counters(Isolate* isolate)
       STATS_COUNTER_TS_LIST(SC)
 #undef SC
       // clang format on
-      runtime_call_stats_() {
+      runtime_call_stats_(),
+      worker_thread_runtime_call_stats_() {
   static const struct {
     Histogram Counters::*member;
     const char* caption;
@@ -529,5 +530,64 @@ void RuntimeCallStats::Dump(v8::tracing::TracedValue* value) {
   in_use_ = false;
 }
 
+WorkerThreadRuntimeCallStats::WorkerThreadRuntimeCallStats()
+    : tls_key_(base::Thread::CreateThreadLocalKey()) {}
+
+WorkerThreadRuntimeCallStats::~WorkerThreadRuntimeCallStats() {
+  base::Thread::DeleteThreadLocalKey(tls_key_);
+}
+
+RuntimeCallStats* WorkerThreadRuntimeCallStats::NewTable() {
+  DCHECK(FLAG_runtime_stats);
+  std::unique_ptr<RuntimeCallStats> new_table =
+      base::make_unique<RuntimeCallStats>();
+  RuntimeCallStats* result = new_table.get();
+
+  base::LockGuard<base::Mutex> lock(&mutex_);
+  tables_.push_back(std::move(new_table));
+  return result;
+}
+
+void WorkerThreadRuntimeCallStats::AddToMainTable(
+    RuntimeCallStats* main_call_stats) {
+  base::LockGuard<base::Mutex> lock(&mutex_);
+  for (auto& worker_stats : tables_) {
+    DCHECK_NE(main_call_stats, worker_stats.get());
+    main_call_stats->Add(worker_stats.get());
+    worker_stats->Reset();
+  }
+}
+
+WorkerThreadRuntimeCallStatsScope::WorkerThreadRuntimeCallStatsScope(
+    WorkerThreadRuntimeCallStats* worker_stats)
+    : table_(nullptr) {
+  if (V8_LIKELY(!FLAG_runtime_stats)) return;
+
+  table_ = reinterpret_cast<RuntimeCallStats*>(
+      base::Thread::GetThreadLocal(worker_stats->GetKey()));
+  if (table_ == nullptr) {
+    table_ = worker_stats->NewTable();
+    base::Thread::SetThreadLocal(worker_stats->GetKey(), table_);
+  }
+
+  if (FLAG_runtime_stats &
+      v8::tracing::TracingCategoryObserver::ENABLED_BY_TRACING) {
+    table_->Reset();
+  }
+}
+
+WorkerThreadRuntimeCallStatsScope::~WorkerThreadRuntimeCallStatsScope() {
+  if (V8_LIKELY(table_ == nullptr)) return;
+
+  if ((FLAG_runtime_stats &
+       v8::tracing::TracingCategoryObserver::ENABLED_BY_TRACING)) {
+    auto value = v8::tracing::TracedValue::Create();
+    table_->Dump(value.get());
+    TRACE_EVENT_INSTANT1(TRACE_DISABLED_BY_DEFAULT("v8.runtime_stats"),
+                         "V8.RuntimeStats", TRACE_EVENT_SCOPE_THREAD,
+                         "runtime-call-stats", std::move(value));
+  }
+}
+
 }  // namespace internal
 }  // namespace v8

src/counters.h

@@ -1023,7 +1023,7 @@ enum RuntimeCallCounterId {
                           kNumberOfCounters
 };
 
-class RuntimeCallStats final : public ZoneObject {
+class RuntimeCallStats final {
  public:
   V8_EXPORT_PRIVATE RuntimeCallStats();
 
@@ -1075,6 +1075,42 @@ class RuntimeCallStats final : public ZoneObject {
   RuntimeCallCounter counters_[kNumberOfCounters];
 };
 
+class WorkerThreadRuntimeCallStats final {
+ public:
+  WorkerThreadRuntimeCallStats();
+  ~WorkerThreadRuntimeCallStats();
+
+  // Returns the TLS key associated with this WorkerThreadRuntimeCallStats.
+  base::Thread::LocalStorageKey GetKey() const { return tls_key_; }
+
+  // Returns a new worker thread runtime call stats table managed by this
+  // WorkerThreadRuntimeCallStats.
+  RuntimeCallStats* NewTable();
+
+  // Adds the counters from the worker thread tables to |main_call_stats|.
+  void AddToMainTable(RuntimeCallStats* main_call_stats);
+
+ private:
+  base::Mutex mutex_;
+  std::vector<std::unique_ptr<RuntimeCallStats>> tables_;
+  base::Thread::LocalStorageKey tls_key_;
+};
+
+// Creating a WorkerThreadRuntimeCallStatsScope will provide a thread-local
+// runtime call stats table, and will dump the table to an immediate trace event
+// when it is destroyed.
+class WorkerThreadRuntimeCallStatsScope final {
+ public:
+  WorkerThreadRuntimeCallStatsScope(
+      WorkerThreadRuntimeCallStats* off_thread_stats);
+  ~WorkerThreadRuntimeCallStatsScope();
+
+  RuntimeCallStats* Get() const { return table_; }
+
+ private:
+  RuntimeCallStats* table_;
+};
+
 #define CHANGE_CURRENT_RUNTIME_COUNTER(runtime_call_stats, counter_id) \
   do {                                                                 \
     if (V8_UNLIKELY(FLAG_runtime_stats) && runtime_call_stats) {       \
@@ -1518,6 +1554,10 @@ class Counters : public std::enable_shared_from_this<Counters> {
 
   RuntimeCallStats* runtime_call_stats() { return &runtime_call_stats_; }
 
+  WorkerThreadRuntimeCallStats* worker_thread_runtime_call_stats() {
+    return &worker_thread_runtime_call_stats_;
+  }
+
  private:
   friend class StatsTable;
   friend class StatsCounterBase;
@@ -1597,6 +1637,7 @@ class Counters : public std::enable_shared_from_this<Counters> {
 #undef SC
 
   RuntimeCallStats runtime_call_stats_;
+  WorkerThreadRuntimeCallStats worker_thread_runtime_call_stats_;
 
   DISALLOW_IMPLICIT_CONSTRUCTORS(Counters);
 };

src/isolate.cc

@@ -3227,6 +3227,8 @@ void Isolate::DumpAndResetStats() {
   }
   if (V8_UNLIKELY(FLAG_runtime_stats ==
                   v8::tracing::TracingCategoryObserver::ENABLED_BY_NATIVE)) {
+    counters()->worker_thread_runtime_call_stats()->AddToMainTable(
+        counters()->runtime_call_stats());
     counters()->runtime_call_stats()->Print();
     counters()->runtime_call_stats()->Reset();
   }

src/parsing/parse-info.cc

@@ -103,34 +103,6 @@ ParseInfo::~ParseInfo() {}
 
 DeclarationScope* ParseInfo::scope() const { return literal()->scope(); }
 
-void ParseInfo::EmitBackgroundParseStatisticsOnBackgroundThread() {
-  // If runtime call stats was enabled by tracing, emit a trace event at the
-  // end of background parsing on the background thread.
-  if (runtime_call_stats_ &&
-      (FLAG_runtime_stats &
-       v8::tracing::TracingCategoryObserver::ENABLED_BY_TRACING)) {
-    auto value = v8::tracing::TracedValue::Create();
-    runtime_call_stats_->Dump(value.get());
-    TRACE_EVENT_INSTANT1(TRACE_DISABLED_BY_DEFAULT("v8.runtime_stats"),
-                         "V8.RuntimeStats", TRACE_EVENT_SCOPE_THREAD,
-                         "runtime-call-stats", std::move(value));
-  }
-}
-
-void ParseInfo::UpdateBackgroundParseStatisticsOnMainThread(Isolate* isolate) {
-  // Copy over the counters from the background thread to the main counters on
-  // the isolate.
-  RuntimeCallStats* main_call_stats = isolate->counters()->runtime_call_stats();
-  if (FLAG_runtime_stats ==
-      v8::tracing::TracingCategoryObserver::ENABLED_BY_NATIVE) {
-    DCHECK_NE(main_call_stats, runtime_call_stats());
-    DCHECK_NOT_NULL(main_call_stats);
-    DCHECK_NOT_NULL(runtime_call_stats());
-    main_call_stats->Add(runtime_call_stats());
-  }
-  set_runtime_call_stats(main_call_stats);
-}
-
 Handle<Script> ParseInfo::CreateScript(Isolate* isolate, Handle<String> source,
                                        ScriptOriginOptions origin_options,
                                        NativesFlag natives) {

src/parsing/parse-info.h

@@ -198,6 +198,8 @@ class V8_EXPORT_PRIVATE ParseInfo {
   // TODO(titzer): these should not be part of ParseInfo.
   //--------------------------------------------------------------------------
   Handle<Script> script() const { return script_; }
+  void set_script(Handle<Script> script);
+  void ClearScriptHandle() { script_ = Handle<Script>(); }
   MaybeHandle<ScopeInfo> maybe_outer_scope_info() const {
     return maybe_outer_scope_info_;
   }
@@ -216,12 +218,8 @@ class V8_EXPORT_PRIVATE ParseInfo {
     set_strict_mode(is_strict(language_mode));
   }
 
-  void EmitBackgroundParseStatisticsOnBackgroundThread();
-  void UpdateBackgroundParseStatisticsOnMainThread(Isolate* isolate);
-
  private:
   void SetScriptForToplevelCompile(Isolate* isolate, Handle<Script> script);
-  void set_script(Handle<Script> script);
 
   // Various configuration flags for parsing.
   enum Flag {

src/parsing/parser.cc

@@ -458,22 +458,27 @@ Parser::Parser(ParseInfo* info)
   }
 }
 
-void Parser::DeserializeScopeChain(
-    Isolate* isolate, ParseInfo* info,
-    MaybeHandle<ScopeInfo> maybe_outer_scope_info) {
+void Parser::InitializeEmptyScopeChain(ParseInfo* info) {
+  DCHECK_NULL(original_scope_);
+  DCHECK_NULL(info->script_scope());
   // TODO(wingo): Add an outer SCRIPT_SCOPE corresponding to the native
   // context, which will have the "this" binding for script scopes.
   DeclarationScope* script_scope = NewScriptScope();
   info->set_script_scope(script_scope);
-  Scope* scope = script_scope;
+  original_scope_ = script_scope;
+}
+
+void Parser::DeserializeScopeChain(
+    Isolate* isolate, ParseInfo* info,
+    MaybeHandle<ScopeInfo> maybe_outer_scope_info) {
+  InitializeEmptyScopeChain(info);
   Handle<ScopeInfo> outer_scope_info;
   if (maybe_outer_scope_info.ToHandle(&outer_scope_info)) {
     DCHECK(ThreadId::Current().Equals(isolate->thread_id()));
-    scope = Scope::DeserializeScopeChain(
-        isolate, zone(), *outer_scope_info, script_scope, ast_value_factory(),
-        Scope::DeserializationMode::kScopesOnly);
+    original_scope_ = Scope::DeserializeScopeChain(
+        isolate, zone(), *outer_scope_info, info->script_scope(),
+        ast_value_factory(), Scope::DeserializationMode::kScopesOnly);
   }
-  original_scope_ = scope;
 }
 
 namespace {

src/parsing/parser.h

@@ -155,6 +155,10 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
 
   void ParseOnBackground(ParseInfo* info);
 
+  // Initializes an empty scope chain for top-level scripts, or scopes which
+  // consist of only the native context.
+  void InitializeEmptyScopeChain(ParseInfo* info);
+
   // Deserialize the scope chain prior to parsing in which the script is going
   // to be executed. If the script is a top-level script, or the scope chain
   // consists of only a native context, maybe_outer_scope_info should be an
@@ -233,8 +237,6 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
                                                            ParseInfo* info,
                                                            Zone* zone);
 
-  void StitchAst(ParseInfo* top_level_parse_info, Isolate* isolate);
-
   PreParser* reusable_preparser() {
     if (reusable_preparser_ == nullptr) {
       reusable_preparser_ =