[compiler] Collect eager inner functions for compilation during renumbering.

src/compiler.cc

 // Copyright 2012 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.h"
 
 #include <algorithm>
 #include <memory>
 
 #include "src/asmjs/asm-js.h"
@@ skipping 20 matching lines @@
 #include "src/parsing/parsing.h"
 #include "src/parsing/rewriter.h"
 #include "src/parsing/scanner-character-streams.h"
 #include "src/runtime-profiler.h"
 #include "src/snapshot/code-serializer.h"
 #include "src/vm-state-inl.h"
 
 namespace v8 {
 namespace internal {
 
-
-
 // A wrapper around a CompilationInfo that detaches the Handles from
 // the underlying DeferredHandleScope and stores them in info_ on
 // destruction.
 class CompilationHandleScope final {
  public:
   explicit CompilationHandleScope(CompilationInfo* info)
       : deferred_(info->isolate()), info_(info) {}
   ~CompilationHandleScope() { info_->set_deferred_handles(deferred_.Detach()); }
 
  private:
@@ skipping 317 matching lines @@
   // Checks whether top level functions should be passed by the filter.
   if (shared->is_toplevel()) {
     Vector<const char> filter = CStrVector(FLAG_ignition_filter);
     return (filter.length() == 0) || (filter.length() == 1 && filter[0] == '*');
   }
 
   // Finally respect the filter.
   return shared->PassesFilter(FLAG_ignition_filter);
 }
 
-CompilationJob* GetUnoptimizedCompilationJob(CompilationInfo* info,
-                                             LazyCompilationMode mode) {
+CompilationJob* GetUnoptimizedCompilationJob(CompilationInfo* info) {
   // Function should have been parsed and analyzed before creating a compilation
   // job.
   DCHECK_NOT_NULL(info->literal());
   DCHECK_NOT_NULL(info->scope());
 
   EnsureFeedbackMetadata(info);
   if (ShouldUseIgnition(info)) {
-    return interpreter::Interpreter::NewCompilationJob(info, mode);
+    return interpreter::Interpreter::NewCompilationJob(info);
   } else {
-    return FullCodeGenerator::NewCompilationJob(info, mode);
+    return FullCodeGenerator::NewCompilationJob(info);
   }
 }
 
 void InstallSharedScopeInfo(CompilationInfo* info,
                             Handle<SharedFunctionInfo> shared) {
   Handle<ScopeInfo> scope_info = info->scope()->scope_info();
   shared->set_scope_info(*scope_info);
   Scope* outer_scope = info->scope()->GetOuterScopeWithContext();
   if (outer_scope) {
     shared->set_outer_scope_info(*outer_scope->scope_info());
@@ skipping 28 matching lines @@
 
 CompilationJob::Status FinalizeUnoptimizedCompilationJob(CompilationJob* job) {
   CompilationJob::Status status = job->FinalizeJob();
   if (status == CompilationJob::SUCCEEDED) {
     InstallUnoptimizedCode(job->info());
     job->RecordUnoptimizedCompilationStats();
   }
   return status;
 }
 
+bool Renumber(ParseInfo* parse_info) {
+  if (!AstNumbering::Renumber(parse_info->isolate(), parse_info->zone(),
+                              parse_info->literal(),
+                              parse_info->eager_inner_function_literals())) {
+    return false;
+  }
+  Handle<SharedFunctionInfo> shared_info = parse_info->shared_info();
+  if (!shared_info.is_null()) {
+    FunctionLiteral* lit = parse_info->literal();
+    shared_info->set_ast_node_count(lit->ast_node_count());
+    if (lit->dont_optimize_reason() != kNoReason) {
+      shared_info->DisableOptimization(lit->dont_optimize_reason());
+    }
+    if (lit->flags() & AstProperties::kMustUseIgnitionTurbo) {
+      shared_info->set_must_use_ignition_turbo(true);
+    }
+  }
+  return true;
+}
+
 bool GenerateUnoptimizedCode(CompilationInfo* info) {
   if (FLAG_validate_asm && info->scope()->asm_module() &&
       !info->shared_info()->is_asm_wasm_broken() && !info->is_debug()) {
     EnsureFeedbackMetadata(info);
     MaybeHandle<FixedArray> wasm_data;
     wasm_data = AsmJs::CompileAsmViaWasm(info);
     if (!wasm_data.is_null()) {
       info->shared_info()->set_asm_wasm_data(*wasm_data.ToHandleChecked());
       info->SetCode(info->isolate()->builtins()->InstantiateAsmJs());
       InstallUnoptimizedCode(info);
       return true;
     }
   }
 
-  std::unique_ptr<CompilationJob> job(
-      GetUnoptimizedCompilationJob(info, LazyCompilationMode::kIfRequested));
+  std::unique_ptr<CompilationJob> job(GetUnoptimizedCompilationJob(info));
   if (job->PrepareJob() != CompilationJob::SUCCEEDED) return false;
   if (job->ExecuteJob() != CompilationJob::SUCCEEDED) return false;
   if (FinalizeUnoptimizedCompilationJob(job.get()) !=
       CompilationJob::SUCCEEDED) {
     return false;
   }
   return true;
 }
 
+bool CompileUnoptimizedInnerFunctionsRecursively(
+    ZoneVector<FunctionLiteral*>* literals, CompilationInfo* outer_info) {
+  Isolate* isolate = outer_info->isolate();
+  Handle<Script> script = outer_info->script();
+
+  for (auto it = literals->begin(); it != literals->end(); it++) {
+    FunctionLiteral* literal = *it;
+
+    // Find any previously allocated shared function info for the given literal.
+    Handle<SharedFunctionInfo> shared;
+    MaybeHandle<SharedFunctionInfo> maybe_existing =
+        script->FindSharedFunctionInfo(isolate, literal);
+    if (maybe_existing.ToHandle(&shared)) {
+      DCHECK(!shared->is_toplevel());
+      // If we found an existing shared function info with compiled code,
+      // we are done.
+      if (shared->is_compiled()) continue;
+    } else {
+      shared =
+          isolate->factory()->NewSharedFunctionInfoForLiteral(literal, script);
+      shared->set_is_toplevel(false);
+    }
+
+    Zone zone(isolate->allocator(), ZONE_NAME);
+    ParseInfo parse_info(&zone, script);

[marja, 2017/01/05 19:25:22]

It's weird | surprising that we need to create a new ParseInfo here even if we
don't parse... it's because the renumbering needs one as input, right?

[rmcilroy, 2017/01/06 17:08:14]

Yes I agree this is a bit weird (it is a copy of the code which used to be in
GetSharedFunctionInfo and did the same thing). Yes it's because Renumber needs a
parse_info, and also because the compiler also gets some stuff from parse_info
indirectly via accessors on compile_info (e.g., the literal, script, scope,
shared_info, etc.). 

It would be really nice to disentangle parse_info and compilation_info so that
they don't refer to each other then we could maybe get away without this.

+    parse_info.set_literal(literal);
+    parse_info.set_shared_info(shared);
+    parse_info.set_function_literal_id(shared->function_literal_id());
+    parse_info.set_language_mode(literal->scope()->language_mode());
+    parse_info.set_ast_value_factory(
+        outer_info->parse_info()->ast_value_factory());
+    parse_info.set_ast_value_factory_owned(false);
+
+    CompilationInfo info(&parse_info, Handle<JSFunction>::null());
+    if (outer_info->will_serialize()) info.PrepareForSerializing();
+    if (outer_info->is_debug()) info.MarkAsDebug();
+
+    if (!Renumber(&parse_info) ||
+        !CompileUnoptimizedInnerFunctionsRecursively(
+            parse_info.eager_inner_function_literals(), &info) ||
+        !GenerateUnoptimizedCode(&info)) {
+      if (!isolate->has_pending_exception()) isolate->StackOverflow();
+      return false;
+    }
+
+    DCHECK(!info.code().is_null());
+    RecordFunctionCompilation(CodeEventListener::FUNCTION_TAG, &info);
+    if (literal->should_be_used_once_hint()) {
+      info.code()->MarkToBeExecutedOnce(isolate);
+    }
+  }
+  return true;
+}
+
 bool CompileUnoptimizedCode(CompilationInfo* info) {
-  DCHECK(AllowCompilation::IsAllowed(info->isolate()));
+  Isolate* isolate = info->isolate();
+  DCHECK(AllowCompilation::IsAllowed(isolate));
+
   if (!Compiler::Analyze(info->parse_info()) ||
+      !CompileUnoptimizedInnerFunctionsRecursively(
+          info->parse_info()->eager_inner_function_literals(), info) ||
       !GenerateUnoptimizedCode(info)) {
-    Isolate* isolate = info->isolate();
     if (!isolate->has_pending_exception()) isolate->StackOverflow();
     return false;
   }
+
   return true;
 }
 
 void EnsureSharedFunctionInfosArrayOnScript(ParseInfo* info) {
   DCHECK(info->is_toplevel());
   DCHECK(!info->script().is_null());
   if (info->script()->shared_function_infos()->length() > 0) {
     DCHECK_EQ(info->script()->shared_function_infos()->length(),
               info->max_function_literal_id() + 1);
     return;
@@ skipping 62 matching lines @@
 
   // Cache optimized context-specific code.
   Handle<JSFunction> function = info->closure();
   Handle<SharedFunctionInfo> shared(function->shared());
   Handle<LiteralsArray> literals(function->literals());
   Handle<Context> native_context(function->context()->native_context());
   SharedFunctionInfo::AddToOptimizedCodeMap(shared, native_context, code,
                                             literals, info->osr_ast_id());
 }
 
-bool Renumber(ParseInfo* parse_info) {
-  if (!AstNumbering::Renumber(parse_info->isolate(), parse_info->zone(),
-                              parse_info->literal())) {
-    return false;
-  }
-  Handle<SharedFunctionInfo> shared_info = parse_info->shared_info();
-  if (!shared_info.is_null()) {
-    FunctionLiteral* lit = parse_info->literal();
-    shared_info->set_ast_node_count(lit->ast_node_count());
-    if (lit->dont_optimize_reason() != kNoReason) {
-      shared_info->DisableOptimization(lit->dont_optimize_reason());
-    }
-    if (lit->flags() & AstProperties::kMustUseIgnitionTurbo) {
-      shared_info->set_must_use_ignition_turbo(true);
-    }
-  }
-  return true;
-}
-
 bool GetOptimizedCodeNow(CompilationJob* job) {
   CompilationInfo* info = job->info();
   Isolate* isolate = info->isolate();
 
   // Parsing is not required when optimizing from existing bytecode.
   if (!info->is_optimizing_from_bytecode()) {
     if (!Compiler::ParseAndAnalyze(info->parse_info())) return false;
     EnsureFeedbackMetadata(info);
   }
 
@@ skipping 417 matching lines @@
   Handle<SharedFunctionInfo> result;
 
   { VMState<COMPILER> state(info->isolate());
     if (parse_info->literal() == nullptr &&
         !parsing::ParseProgram(parse_info)) {
       return Handle<SharedFunctionInfo>::null();
     }
 
     EnsureSharedFunctionInfosArrayOnScript(parse_info);
 
-    FunctionLiteral* lit = parse_info->literal();
-
     // Measure how long it takes to do the compilation; only take the
     // rest of the function into account to avoid overlap with the
     // parsing statistics.
     RuntimeCallTimerScope runtimeTimer(
         isolate, parse_info->is_eval() ? &RuntimeCallStats::CompileEval
                                        : &RuntimeCallStats::Compile);
     HistogramTimer* rate = parse_info->is_eval()
                                ? info->isolate()->counters()->compile_eval()
                                : info->isolate()->counters()->compile();
     HistogramTimerScope timer(rate);
     TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("v8.compile"),
                  parse_info->is_eval() ? "V8.CompileEval" : "V8.Compile");
 
     // Allocate a shared function info object.
+    FunctionLiteral* lit = parse_info->literal();
     DCHECK_EQ(kNoSourcePosition, lit->function_token_position());
     result = isolate->factory()->NewSharedFunctionInfoForLiteral(lit, script);
     result->set_is_toplevel(true);
     parse_info->set_shared_info(result);
     parse_info->set_function_literal_id(result->function_literal_id());
 
     // Compile the code.
     if (!CompileUnoptimizedCode(info)) {
       return Handle<SharedFunctionInfo>::null();
     }
@@ skipping 543 matching lines @@
   // The source was parsed lazily, so compiling for debugging is not possible.
   DCHECK(!compile_info.is_debug());
 
   Handle<SharedFunctionInfo> result = CompileToplevel(&compile_info);
   if (!result.is_null()) isolate->debug()->OnAfterCompile(script);
   return result;
 }
 
 Handle<SharedFunctionInfo> Compiler::GetSharedFunctionInfo(
     FunctionLiteral* literal, Handle<Script> script,
-    CompilationInfo* outer_info, LazyCompilationMode mode) {
+    CompilationInfo* outer_info) {
   // Precondition: code has been parsed and scopes have been analyzed.
   Isolate* isolate = outer_info->isolate();
   MaybeHandle<SharedFunctionInfo> maybe_existing;
 
   // Find any previously allocated shared function info for the given literal.
   maybe_existing = script->FindSharedFunctionInfo(isolate, literal);
 
-  // We found an existing shared function info. If it has any sort of code
-  // attached, don't worry about compiling and simply return it. Otherwise,
-  // continue to decide whether to eagerly compile.
-  // Note that we also carry on if we are compiling eager to obtain code for
-  // debugging, unless we already have code with debug break slots.
+  // If we found an existing shared function info, return it.
   Handle<SharedFunctionInfo> existing;
   if (maybe_existing.ToHandle(&existing)) {
     DCHECK(!existing->is_toplevel());
-    if (existing->HasBaselineCode() || existing->HasBytecodeArray()) {
-      if (!outer_info->is_debug() || existing->HasDebugCode()) {
-        return existing;
-      }
-    }
+    return existing;

[jochen (gone - plz use gerrit), 2017/01/05 15:53:12]

this might be wrong if we want debug code, but the existing SFI has only
baseline code

[rmcilroy, 2017/01/05 16:02:42]

The debug code logic didn't do anything any longer - there is a comment below in
the old code saying "When compiling for debug, we are not interested in having
debug break slots in inner functions" and the compilation logic was wrapped in a
if (!result->is_compiled()) so even though we fell through here, we were never
actually recompiling the inner function for debug code anyway.

   }
 
-  // Allocate a shared function info object.
-  Handle<SharedFunctionInfo> result;
-  if (!maybe_existing.ToHandle(&result)) {
-    result =
-        isolate->factory()->NewSharedFunctionInfoForLiteral(literal, script);
-    result->set_is_toplevel(false);
+  // Allocate a shared function info object which will be compiled lazily.
+  Handle<SharedFunctionInfo> result =
+      isolate->factory()->NewSharedFunctionInfoForLiteral(literal, script);

[jochen (gone - plz use gerrit), 2017/01/05 15:53:12]

why don't you need to put the compile lazy builtin here?

[rmcilroy, 2017/01/05 16:02:42]

I was confused by this too, but NewSharedFunctionInfoForLiteral already does it
(the old code only added the lazy builtin to the ComilationInfo (not the new
SFI), presumably so that RecordFunctionCompilation "worked").

+  result->set_is_toplevel(false);
+  Scope* outer_scope = literal->scope()->GetOuterScopeWithContext();
+  if (outer_scope) {
+    result->set_outer_scope_info(*outer_scope->scope_info());
   }
-
-  Zone zone(isolate->allocator(), ZONE_NAME);
-  ParseInfo parse_info(&zone, script);
-  CompilationInfo info(&parse_info, Handle<JSFunction>::null());
-  parse_info.set_literal(literal);
-  parse_info.set_shared_info(result);
-  parse_info.set_function_literal_id(result->function_literal_id());
-  parse_info.set_language_mode(literal->scope()->language_mode());
-  parse_info.set_ast_value_factory(
-      outer_info->parse_info()->ast_value_factory());
-  parse_info.set_ast_value_factory_owned(false);
-
-  if (outer_info->will_serialize()) info.PrepareForSerializing();
-  if (outer_info->is_debug()) info.MarkAsDebug();
-
-  // If this inner function is already compiled, we don't need to compile
-  // again. When compiling for debug, we are not interested in having debug
-  // break slots in inner functions, neither for setting break points nor
-  // for revealing inner functions.
-  // This is especially important for generators. We must not replace the
-  // code for generators, as there may be suspended generator objects.
-  if (!result->is_compiled()) {
-    if (mode == LazyCompilationMode::kAlways ||
-        !literal->ShouldEagerCompile()) {
-      info.SetCode(isolate->builtins()->CompileLazy());
-      Scope* outer_scope = literal->scope()->GetOuterScopeWithContext();
-      if (outer_scope) {
-        result->set_outer_scope_info(*outer_scope->scope_info());
-      }
-    } else {
-      // Generate code
-      TimerEventScope<TimerEventCompileCode> timer(isolate);
-      RuntimeCallTimerScope runtimeTimer(isolate,
-                                         &RuntimeCallStats::CompileCode);
-      TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("v8.compile"), "V8.CompileCode");
-      if (Renumber(info.parse_info()) && GenerateUnoptimizedCode(&info)) {
-        // Code generation will ensure that the feedback vector is present and
-        // appropriately sized.
-        DCHECK(!info.code().is_null());
-        if (literal->should_be_used_once_hint()) {
-          info.code()->MarkToBeExecutedOnce(isolate);
-        }
-      } else {
-        return Handle<SharedFunctionInfo>::null();
-      }
-    }
-  }
-
-  if (maybe_existing.is_null()) {
-    RecordFunctionCompilation(CodeEventListener::FUNCTION_TAG, &info);
-  }
-
   return result;
 }
 
 Handle<SharedFunctionInfo> Compiler::GetSharedFunctionInfoForNative(
     v8::Extension* extension, Handle<String> name) {
   Isolate* isolate = name->GetIsolate();
   v8::Isolate* v8_isolate = reinterpret_cast<v8::Isolate*>(isolate);
 
   // Compute the function template for the native function.
   v8::Local<v8::FunctionTemplate> fun_template =
@@ skipping 24 matching lines @@
 
 MaybeHandle<Code> Compiler::GetOptimizedCodeForOSR(Handle<JSFunction> function,
                                                    BailoutId osr_ast_id,
                                                    JavaScriptFrame* osr_frame) {
   DCHECK(!osr_ast_id.IsNone());
   DCHECK_NOT_NULL(osr_frame);
   return GetOptimizedCode(function, NOT_CONCURRENT, osr_ast_id, osr_frame);
 }
 
 CompilationJob* Compiler::PrepareUnoptimizedCompilationJob(
-    CompilationInfo* info, LazyCompilationMode mode) {
+    CompilationInfo* info) {
   VMState<COMPILER> state(info->isolate());
-  std::unique_ptr<CompilationJob> job(GetUnoptimizedCompilationJob(info, mode));
+  std::unique_ptr<CompilationJob> job(GetUnoptimizedCompilationJob(info));
   if (job->PrepareJob() != CompilationJob::SUCCEEDED) {
     return nullptr;
   }
   return job.release();
 }
 
 bool Compiler::FinalizeCompilationJob(CompilationJob* raw_job) {
   // Take ownership of compilation job.  Deleting job also tears down the zone.
   std::unique_ptr<CompilationJob> job(raw_job);
 
@@ skipping 35 matching lines @@
     DCHECK(shared->is_compiled());
     function->set_literals(cached.literals);
   } else if (shared->is_compiled()) {
     // TODO(mvstanton): pass pretenure flag to EnsureLiterals.
     JSFunction::EnsureLiterals(function);
   }
 }
 
 }  // namespace internal
 }  // namespace v8