Debugger: prepare code for debugging on a per-function basis.

src/compiler.cc

Index: src/compiler.cc
diff --git a/src/compiler.cc b/src/compiler.cc
index ab0a620bedc007000931dd8b43529baaec61d2c0..56434c951bc81c9763f3a2f439cfb5fd2fa33004 100644
--- a/src/compiler.cc
+++ b/src/compiler.cc
@@ -81,6 +81,9 @@ class CompilationInfoWithZone : public CompilationInfo {
   explicit CompilationInfoWithZone(Handle<JSFunction> function)
       : CompilationInfo(new ParseInfo(&zone_, function)) {}
 
+  explicit CompilationInfoWithZone(Handle<SharedFunctionInfo> shared)

[Michael Starzinger, 2015/07/20 11:29:40]

See comment below.

[Yang, 2015/07/20 12:13:47]

Acknowledged.

+      : CompilationInfo(new ParseInfo(&zone_, shared)) {}
+
   // Virtual destructor because a CompilationInfoWithZone has to exit the
   // zone scope and get rid of dependent maps even when the destructor is
   // called when cast as a CompilationInfo.
@@ -338,11 +341,10 @@ class HOptimizedGraphBuilderWithPositions: public HOptimizedGraphBuilder {
 
 OptimizedCompileJob::Status OptimizedCompileJob::CreateGraph() {
   DCHECK(info()->IsOptimizing());
-  DCHECK(!info()->IsCompilingForDebugging());
 
   // Do not use Crankshaft/TurboFan if we need to be able to set break points.
-  if (isolate()->debug()->has_break_points()) {
-    return RetryOptimization(kDebuggerHasBreakPoints);
+  if (info()->shared_info()->HasDebugInfo()) {
+    return AbortOptimization(kFunctionBeingDebugged);
   }
 
   // Limit the number of times we try to optimize functions.
@@ -905,18 +907,6 @@ MaybeHandle<Code> Compiler::GetLazyCode(Handle<JSFunction> function) {
 }
 
 
-MaybeHandle<Code> Compiler::GetUnoptimizedCode(
-    Handle<SharedFunctionInfo> shared) {
-  DCHECK(!shared->GetIsolate()->has_pending_exception());
-  DCHECK(!shared->is_compiled());
-
-  Zone zone;
-  ParseInfo parse_info(&zone, shared);
-  CompilationInfo info(&parse_info);
-  return GetUnoptimizedCodeCommon(&info);
-}
-
-
 bool Compiler::EnsureCompiled(Handle<JSFunction> function,
                               ClearExceptionFlag flag) {
   if (function->is_compiled()) return true;
@@ -979,45 +969,40 @@ bool Compiler::EnsureDeoptimizationSupport(CompilationInfo* info) {
 }
 
 
-// Compile full code for debugging. This code will have debug break slots
-// and deoptimization information. Deoptimization information is required
-// in case that an optimized version of this function is still activated on
-// the stack. It will also make sure that the full code is compiled with
-// the same flags as the previous version, that is flags which can change
-// the code generated. The current method of mapping from already compiled
-// full code without debug break slots to full code with debug break slots
-// depends on the generated code is otherwise exactly the same.
-// If compilation fails, just keep the existing code.
-MaybeHandle<Code> Compiler::GetDebugCode(Handle<JSFunction> function) {
-  CompilationInfoWithZone info(function);
-  Isolate* isolate = info.isolate();
-  VMState<COMPILER> state(isolate);
-
-  info.MarkAsDebug();
-
-  DCHECK(!isolate->has_pending_exception());
-  Handle<Code> old_code(function->shared()->code());
-  DCHECK(old_code->kind() == Code::FUNCTION);
-  DCHECK(!old_code->has_debug_break_slots());
-
-  info.MarkCompilingForDebugging();
-  if (old_code->is_compiled_optimizable()) {
-    info.EnableDeoptimizationSupport();
-  } else {
-    info.MarkNonOptimizable();
+MaybeHandle<Code> CompileForDebuggingImpl(CompilationInfo* info) {
+  info->MarkAsDebug();
+  VMState<COMPILER> state(info->isolate());
+  if (info->shared_info()->is_compiled()) {
+    if (info->shared_info()->code()->is_compiled_optimizable()) {
+      info->EnableDeoptimizationSupport();
+    } else {
+      info->MarkNonOptimizable();
+    }
   }
-  MaybeHandle<Code> maybe_new_code = GetUnoptimizedCodeCommon(&info);
+  MaybeHandle<Code> maybe_new_code = GetUnoptimizedCodeCommon(info);
   Handle<Code> new_code;
   if (!maybe_new_code.ToHandle(&new_code)) {
-    isolate->clear_pending_exception();
-  } else {
-    DCHECK_EQ(old_code->is_compiled_optimizable(),
-              new_code->is_compiled_optimizable());
+    info->isolate()->clear_pending_exception();
   }
   return maybe_new_code;
 }
 
 
+MaybeHandle<Code> Compiler::CompileForDebugging(Handle<JSFunction> function) {
+  CompilationInfoWithZone info(function);
+  VMState<COMPILER> state(info.isolate());
+  return CompileForDebuggingImpl(&info);
+}
+
+
+MaybeHandle<Code> Compiler::CompileForDebugging(
+    Handle<SharedFunctionInfo> shared) {
+  DCHECK(shared->allows_lazy_compilation_without_context());
+  CompilationInfoWithZone info(shared);

[Michael Starzinger, 2015/07/20 11:29:39]

There is a constant back-and-forth refactoring around CompilationInfoWithZone.
Can we make the following lines explicit here to avoid adding a new constructor:

    Zone zone;
    ParseInfo parse_info(&zone, shared);
    CompilationInfo info(&parse_info);

[Yang, 2015/07/20 12:13:47]

Done.

+  return CompileForDebuggingImpl(&info);
+}
+
+
 void Compiler::CompileForLiveEdit(Handle<Script> script) {
   // TODO(635): support extensions.
   Zone zone;
@@ -1065,12 +1050,13 @@ static Handle<SharedFunctionInfo> CompileToplevel(CompilationInfo* info) {
   { VMState<COMPILER> state(info->isolate());
     if (parse_info->literal() == NULL) {
       // Parse the script if needed (if it's already parsed, function() is
-      // non-NULL).
+      // non-NULL). If compiling for debugging, we may eagerly compile inner
+      // functions, so do not parse lazily in that case.
       ScriptCompiler::CompileOptions options = parse_info->compile_options();
       bool parse_allow_lazy = (options == ScriptCompiler::kConsumeParserCache ||
                                String::cast(script->source())->length() >
                                    FLAG_min_preparse_length) &&
-                              !Compiler::DebuggerWantsEagerCompilation(isolate);
+                              !info->is_debug();
 
       parse_info->set_allow_lazy_parsing(parse_allow_lazy);
       if (!parse_allow_lazy &&
@@ -1180,6 +1166,8 @@ MaybeHandle<JSFunction> Compiler::GetFunctionFromEval(
     parse_info.set_parse_restriction(restriction);
     parse_info.set_context(context);
 
+    // If we eval from debug code, compile for debugging as well.
+    if (outer_info->HasDebugCode()) info.MarkAsDebug();
     Debug::RecordEvalCaller(script);
 
     shared_info = CompileToplevel(&info);
@@ -1371,9 +1359,13 @@ Handle<SharedFunctionInfo> Compiler::GetSharedFunctionInfo(
   // We found an existing shared function info. If it's already compiled,
   // don't worry about compiling it, and simply return it. If it's not yet
   // compiled, continue to decide whether to eagerly compile.
+  // Carry on if we are compiling eager to obtain code for debugging,
+  // unless we already have code with debut break slots.
   Handle<SharedFunctionInfo> existing;
   if (maybe_existing.ToHandle(&existing) && existing->is_compiled()) {
-    return existing;
+    if (!outer_info->is_debug() || existing->HasDebugCode()) {
+      return existing;
+    }
   }
 
   Zone zone;
@@ -1384,6 +1376,7 @@ Handle<SharedFunctionInfo> Compiler::GetSharedFunctionInfo(
   parse_info.set_language_mode(literal->scope()->language_mode());
   if (outer_info->will_serialize()) info.PrepareForSerializing();
   if (outer_info->is_first_compile()) info.MarkAsFirstCompile();
+  if (outer_info->is_debug()) info.MarkAsDebug();
 
   LiveEditFunctionTracker live_edit_tracker(isolate, literal);
   // Determine if the function can be lazily compiled. This is necessary to
@@ -1396,9 +1389,12 @@ Handle<SharedFunctionInfo> Compiler::GetSharedFunctionInfo(
   // of functions without an outer context when setting a breakpoint through
   // Debug::FindSharedFunctionInfoInScript.
   bool allow_lazy_without_ctx = literal->AllowsLazyCompilationWithoutContext();
-  bool allow_lazy =
-      literal->AllowsLazyCompilation() &&
-      !DebuggerWantsEagerCompilation(isolate, allow_lazy_without_ctx);
+  bool allow_lazy = literal->AllowsLazyCompilation();
+  // Compile eagerly for live edit.
+  allow_lazy &= !LiveEditFunctionTracker::IsActive(isolate);

[Michael Starzinger, 2015/07/20 11:29:39]

nit: Can we combine this into one big conjunction instead?

[Yang, 2015/07/20 12:13:47]

Done.

+  // When compiling debug code, eagerly compile unless we can lazily compile
+  // without the context (SFI without a JSFunction).
+  allow_lazy &= !info.is_debug() || allow_lazy_without_ctx;
 
   if (outer_info->parse_info()->is_toplevel() && outer_info->will_serialize()) {
     // Make sure that if the toplevel code (possibly to be serialized),
@@ -1463,8 +1459,10 @@ Handle<SharedFunctionInfo> Compiler::GetSharedFunctionInfo(
     live_edit_tracker.RecordFunctionInfo(result, literal, info.zone());
     return result;
   } else if (!lazy) {
-    // We have additional data from compilation now.
-    DCHECK(!existing->is_compiled());
+    // We have additional data from compilation now. Upgrading from
+    // non-compiled code or in case of debugging, from non-debug code.
+    DCHECK(info.is_debug() ? !existing->HasDebugCode()

[Michael Starzinger, 2015/07/20 11:29:39]

nit: Not sure exactly what this assertion is guarding against, I would be fine
with dropping it entirely.

[Yang, 2015/07/20 12:13:47]

This is supposed to guard against replacing existing debug code with a new copy.
In that case we would lose break points, as the new code is not patched.

+                           : !existing->is_compiled());
     existing->ReplaceCode(*info.code());
     existing->set_scope_info(*scope_info);
     existing->set_feedback_vector(*info.feedback_vector());
@@ -1478,6 +1476,10 @@ MaybeHandle<Code> Compiler::GetOptimizedCode(Handle<JSFunction> function,
                                              ConcurrencyMode mode,
                                              BailoutId osr_ast_id,
                                              JavaScriptFrame* osr_frame) {
+  Isolate* isolate = function->GetIsolate();
+  Handle<SharedFunctionInfo> shared(function->shared(), isolate);
+  if (shared->HasDebugInfo()) return MaybeHandle<Code>();
+
   Handle<Code> cached_code;
   if (GetCodeFromOptimizedCodeMap(
           function, osr_ast_id).ToHandle(&cached_code)) {
@@ -1492,10 +1494,8 @@ MaybeHandle<Code> Compiler::GetOptimizedCode(Handle<JSFunction> function,
     return cached_code;
   }
 
-  Isolate* isolate = function->GetIsolate();
   DCHECK(AllowCompilation::IsAllowed(isolate));
 
-  Handle<SharedFunctionInfo> shared(function->shared(), isolate);
   if (!shared->is_compiled() ||
       shared->scope_info() == ScopeInfo::Empty(isolate)) {
     // The function was never compiled. Compile it unoptimized first.
@@ -1557,19 +1557,18 @@ Handle<Code> Compiler::GetConcurrentlyOptimizedCode(OptimizedCompileJob* job) {
   Handle<SharedFunctionInfo> shared = info->shared_info();
   shared->code()->set_profiler_ticks(0);
 
+  DCHECK(!shared->HasDebugInfo());

[Michael Starzinger, 2015/07/20 11:29:39]

What happens of a concurrent compile job for a function has been enqueued and
the debugger sets a breakpoint inside that function while it's on the queue.
Does the debugger flush the queue somehow? Or is there another mechanism in
place to prevent this?

[Yang, 2015/07/20 12:13:47]

Done. We flush the queue when preparing the function for break points.

+
   // 1) Optimization on the concurrent thread may have failed.
   // 2) The function may have already been optimized by OSR.  Simply continue.
   //    Except when OSR already disabled optimization for some reason.
   // 3) The code may have already been invalidated due to dependency change.
-  // 4) Debugger may have been activated.
-  // 5) Code generation may have failed.
+  // 4) Code generation may have failed.
   if (job->last_status() == OptimizedCompileJob::SUCCEEDED) {
     if (shared->optimization_disabled()) {
       job->RetryOptimization(kOptimizationDisabled);
     } else if (info->dependencies()->HasAborted()) {
       job->RetryOptimization(kBailedOutDueToDependencyChange);
-    } else if (isolate->debug()->has_break_points()) {
-      job->RetryOptimization(kDebuggerHasBreakPoints);
     } else if (job->GenerateCode() == OptimizedCompileJob::SUCCEEDED) {
       RecordFunctionCompilation(Logger::LAZY_COMPILE_TAG, info.get(), shared);
       if (shared->SearchOptimizedCodeMap(info->context()->native_context(),
@@ -1595,15 +1594,6 @@ Handle<Code> Compiler::GetConcurrentlyOptimizedCode(OptimizedCompileJob* job) {
 }
 
 
-bool Compiler::DebuggerWantsEagerCompilation(Isolate* isolate,
-                                             bool allow_lazy_without_ctx) {
-  if (LiveEditFunctionTracker::IsActive(isolate)) return true;
-  Debug* debug = isolate->debug();
-  bool debugging = debug->is_active() || debug->has_break_points();
-  return debugging && !allow_lazy_without_ctx;
-}
-
-
 CompilationPhase::CompilationPhase(const char* name, CompilationInfo* info)
     : name_(name), info_(info) {
   if (FLAG_hydrogen_stats) {