[debugger] Refactor LiveEdit function info collection

This moves collection of function information from its previous spot in
the standard compiler pipeline (GetSharedFunctionInfo() and
CompileTopLevel()) to its new location in CompileForLiveEdit. Nesting
information is reconstructed by traversing the AST.

R=yangguo@chromium.org
BUG=

Review-Url: https://codereview.chromium.org/1971683002
Cr-Commit-Position: refs/heads/master@{#36306}

src/compiler.cc

@@ -1019,7 +1019,6 @@ Handle<SharedFunctionInfo> CompileToplevel(CompilationInfo* info) {
     DCHECK(!info->is_debug() || !parse_info->allow_lazy_parsing());
 
     FunctionLiteral* lit = parse_info->literal();
-    LiveEditFunctionTracker live_edit_tracker(isolate, lit);
 
     // Measure how long it takes to do the compilation; only take the
     // rest of the function into account to avoid overlap with the
@@ -1068,8 +1067,6 @@ Handle<SharedFunctionInfo> CompileToplevel(CompilationInfo* info) {
 
     if (!script.is_null())
       script->set_compilation_state(Script::COMPILATION_STATE_COMPILED);
-
-    live_edit_tracker.RecordFunctionInfo(result, lit, info->zone());
   }
 
   return result;
@@ -1228,7 +1225,7 @@ bool Compiler::CompileDebugCode(Handle<SharedFunctionInfo> shared) {
   return true;
 }
 
-bool Compiler::CompileForLiveEdit(Handle<Script> script) {
+MaybeHandle<JSArray> Compiler::CompileForLiveEdit(Handle<Script> script) {
   Isolate* isolate = script->GetIsolate();
   DCHECK(AllowCompilation::IsAllowed(isolate));
 
@@ -1244,22 +1241,23 @@ bool Compiler::CompileForLiveEdit(Handle<Script> script) {
   CompilationInfo info(&parse_info, Handle<JSFunction>::null());
   parse_info.set_global();
   info.MarkAsDebug();
+
   // TODO(635): support extensions.
   const bool compilation_succeeded = !CompileToplevel(&info).is_null();
+  Handle<JSArray> infos;
+  if (compilation_succeeded) {
+    // Check postconditions on success.
+    DCHECK(!isolate->has_pending_exception());
+    infos = LiveEditFunctionTracker::Collect(parse_info.literal(), script,
+                                             &zone, isolate);
+  }
 
   // Restore the original function info list in order to remain side-effect
   // free as much as possible, since some code expects the old shared function
   // infos to stick around.
   script->set_shared_function_infos(*old_function_infos);
 
-  if (!compilation_succeeded) {
-    return false;
-  }
-
-  // Check postconditions on success.
-  DCHECK(!isolate->has_pending_exception());
-
-  return compilation_succeeded;
+  return infos;
 }
 
 // TODO(turbofan): In the future, unoptimized code with deopt support could
@@ -1589,7 +1587,6 @@ Handle<SharedFunctionInfo> Compiler::GetSharedFunctionInfo(
   if (outer_info->will_serialize()) info.PrepareForSerializing();
   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
   // allow some of our builtin JS files to be lazily compiled. These
   // builtins cannot be handled lazily by the parser, since we have to know
@@ -1632,7 +1629,6 @@ Handle<SharedFunctionInfo> Compiler::GetSharedFunctionInfo(
 
   if (maybe_existing.is_null()) {
     RecordFunctionCompilation(Logger::FUNCTION_TAG, &info);
-    live_edit_tracker.RecordFunctionInfo(result, literal, info.zone());
   }
 
   return result;

src/compiler.h

@@ -48,7 +48,7 @@ class Compiler : public AllStatic {
   static bool CompileOptimized(Handle<JSFunction> function, ConcurrencyMode);
   static bool CompileDebugCode(Handle<JSFunction> function);
   static bool CompileDebugCode(Handle<SharedFunctionInfo> shared);
-  static bool CompileForLiveEdit(Handle<Script> script);
+  static MaybeHandle<JSArray> CompileForLiveEdit(Handle<Script> script);
 
   // Generate and install code from previously queued compilation job.
   static void FinalizeCompilationJob(CompilationJob* job);

src/debug/liveedit.cc

@@ -691,105 +691,6 @@ Handle<SharedFunctionInfo> SharedInfoWrapper::GetInfo() {
 }
 
 
-class FunctionInfoListener {
- public:
-  explicit FunctionInfoListener(Isolate* isolate) {
-    current_parent_index_ = -1;
-    len_ = 0;
-    result_ = isolate->factory()->NewJSArray(10);
-  }
-
-  void FunctionStarted(FunctionLiteral* fun) {
-    HandleScope scope(isolate());
-    FunctionInfoWrapper info = FunctionInfoWrapper::Create(isolate());
-    info.SetInitialProperties(fun->name(), fun->start_position(),
-                              fun->end_position(), fun->parameter_count(),
-                              fun->materialized_literal_count(),
-                              current_parent_index_);
-    current_parent_index_ = len_;
-    SetElementSloppy(result_, len_, info.GetJSArray());
-    len_++;
-  }
-
-  void FunctionDone() {
-    HandleScope scope(isolate());
-    FunctionInfoWrapper info = FunctionInfoWrapper::cast(
-        *JSReceiver::GetElement(isolate(), result_, current_parent_index_)
-             .ToHandleChecked());
-    current_parent_index_ = info.GetParentIndex();
-  }
-
-  // Saves full information about a function: its code, its scope info
-  // and a SharedFunctionInfo object.
-  void FunctionInfo(Handle<SharedFunctionInfo> shared, Scope* scope,
-                    Zone* zone) {
-    if (!shared->IsSharedFunctionInfo()) {
-      return;
-    }
-    FunctionInfoWrapper info = FunctionInfoWrapper::cast(
-        *JSReceiver::GetElement(isolate(), result_, current_parent_index_)
-             .ToHandleChecked());
-    info.SetFunctionCode(Handle<Code>(shared->code()),
-                         Handle<HeapObject>(shared->scope_info()));
-    info.SetSharedFunctionInfo(shared);
-
-    Handle<Object> scope_info_list = SerializeFunctionScope(scope, zone);
-    info.SetFunctionScopeInfo(scope_info_list);
-  }
-
-  Handle<JSArray> GetResult() { return result_; }
-
- private:
-  Isolate* isolate() const { return result_->GetIsolate(); }
-
-  Handle<Object> SerializeFunctionScope(Scope* scope, Zone* zone) {
-    Handle<JSArray> scope_info_list = isolate()->factory()->NewJSArray(10);
-    int scope_info_length = 0;
-
-    // Saves some description of scope. It stores name and indexes of
-    // variables in the whole scope chain. Null-named slots delimit
-    // scopes of this chain.
-    Scope* current_scope = scope;
-    while (current_scope != NULL) {
-      HandleScope handle_scope(isolate());
-      ZoneList<Variable*> stack_list(current_scope->StackLocalCount(), zone);
-      ZoneList<Variable*> context_list(
-          current_scope->ContextLocalCount(), zone);
-      ZoneList<Variable*> globals_list(current_scope->ContextGlobalCount(),
-                                       zone);
-      current_scope->CollectStackAndContextLocals(&stack_list, &context_list,
-                                                  &globals_list);
-      context_list.Sort(&Variable::CompareIndex);
-
-      for (int i = 0; i < context_list.length(); i++) {
-        SetElementSloppy(scope_info_list,
-                         scope_info_length,
-                         context_list[i]->name());
-        scope_info_length++;
-        SetElementSloppy(
-            scope_info_list,
-            scope_info_length,
-            Handle<Smi>(Smi::FromInt(context_list[i]->index()), isolate()));
-        scope_info_length++;
-      }
-      SetElementSloppy(scope_info_list,
-                       scope_info_length,
-                       Handle<Object>(isolate()->heap()->null_value(),
-                                      isolate()));
-      scope_info_length++;
-
-      current_scope = current_scope->outer_scope();
-    }
-
-    return scope_info_list;
-  }
-
-  Handle<JSArray> result_;
-  int len_;
-  int current_parent_index_;
-};
-
-
 void LiveEdit::InitializeThreadLocal(Debug* debug) {
   debug->thread_local_.frame_drop_mode_ = LiveEdit::FRAMES_UNTOUCHED;
 }
@@ -825,11 +726,10 @@ MaybeHandle<JSArray> LiveEdit::GatherCompileInfo(Handle<Script> script,
                                                  Handle<String> source) {
   Isolate* isolate = script->GetIsolate();
 
-  FunctionInfoListener listener(isolate);
+  MaybeHandle<JSArray> infos;
   Handle<Object> original_source =
       Handle<Object>(script->source(), isolate);
   script->set_source(*source);
-  isolate->set_active_function_info_listener(&listener);
 
   {
     // Creating verbose TryCatch from public API is currently the only way to
@@ -838,7 +738,7 @@ MaybeHandle<JSArray> LiveEdit::GatherCompileInfo(Handle<Script> script,
     try_catch.SetVerbose(true);
 
     // A logical 'try' section.
-    Compiler::CompileForLiveEdit(script);
+    infos = Compiler::CompileForLiveEdit(script);
   }
 
   // A logical 'catch' section.
@@ -876,11 +776,10 @@ MaybeHandle<JSArray> LiveEdit::GatherCompileInfo(Handle<Script> script,
   }
 
   // A logical 'finally' section.
-  isolate->set_active_function_info_listener(NULL);
   script->set_source(*original_source);
 
   if (rethrow_exception.is_null()) {
-    return listener.GetResult();
+    return infos.ToHandleChecked();
   } else {
     return isolate->Throw<JSArray>(rethrow_exception);
   }
@@ -2038,35 +1937,107 @@ const char* LiveEdit::RestartFrame(JavaScriptFrame* frame) {
   return NULL;
 }
 
+Handle<JSArray> LiveEditFunctionTracker::Collect(FunctionLiteral* node,
+                                                 Handle<Script> script,
+                                                 Zone* zone, Isolate* isolate) {
+  LiveEditFunctionTracker visitor(script, zone, isolate);
+  visitor.VisitFunctionLiteral(node);
+  return visitor.result_;
+}
 
-LiveEditFunctionTracker::LiveEditFunctionTracker(Isolate* isolate,
-                                                 FunctionLiteral* fun)
-    : isolate_(isolate) {
-  if (isolate_->active_function_info_listener() != NULL) {
-    isolate_->active_function_info_listener()->FunctionStarted(fun);
-  }
+LiveEditFunctionTracker::LiveEditFunctionTracker(Handle<Script> script,
+                                                 Zone* zone, Isolate* isolate)
+    : AstTraversalVisitor(isolate) {
+  current_parent_index_ = -1;
+  isolate_ = isolate;
+  len_ = 0;
+  result_ = isolate->factory()->NewJSArray(10);
+  script_ = script;
+  zone_ = zone;
 }
 
+void LiveEditFunctionTracker::VisitFunctionLiteral(FunctionLiteral* node) {
+  Scope* scope = node->scope();
 
-LiveEditFunctionTracker::~LiveEditFunctionTracker() {
-  if (isolate_->active_function_info_listener() != NULL) {
-    isolate_->active_function_info_listener()->FunctionDone();
-  }
+  // FunctionStarted is called in pre-order.
+  FunctionStarted(node);
+
+  VisitDeclarations(scope->declarations());
+  VisitStatements(node->body());
+
+  // FunctionDone are called in post-order.
+  // TODO(jgruber): If required, replace the (linear cost)
+  // FindSharedFunctionInfo call with a more efficient implementation.
+  Handle<SharedFunctionInfo> info =
+      script_->FindSharedFunctionInfo(node).ToHandleChecked();
+  FunctionDone(info, scope);
 }
 
+void LiveEditFunctionTracker::FunctionStarted(FunctionLiteral* fun) {
+  HandleScope handle_scope(isolate_);
+  FunctionInfoWrapper info = FunctionInfoWrapper::Create(isolate_);
+  info.SetInitialProperties(fun->name(), fun->start_position(),
+                            fun->end_position(), fun->parameter_count(),
+                            fun->materialized_literal_count(),
+                            current_parent_index_);
+  current_parent_index_ = len_;
+  SetElementSloppy(result_, len_, info.GetJSArray());
+  len_++;
+}
 
-void LiveEditFunctionTracker::RecordFunctionInfo(
-    Handle<SharedFunctionInfo> info, FunctionLiteral* lit,
-    Zone* zone) {
-  if (isolate_->active_function_info_listener() != NULL) {
-    isolate_->active_function_info_listener()->FunctionInfo(info, lit->scope(),
-                                                            zone);
-  }
+// Saves full information about a function: its code, its scope info
+// and a SharedFunctionInfo object.
+void LiveEditFunctionTracker::FunctionDone(Handle<SharedFunctionInfo> shared,
+                                           Scope* scope) {
+  HandleScope handle_scope(isolate_);
+  FunctionInfoWrapper info = FunctionInfoWrapper::cast(
+      *JSReceiver::GetElement(isolate_, result_, current_parent_index_)
+           .ToHandleChecked());
+  info.SetFunctionCode(Handle<Code>(shared->code()),
+                       Handle<HeapObject>(shared->scope_info()));
+  info.SetSharedFunctionInfo(shared);
+
+  Handle<Object> scope_info_list = SerializeFunctionScope(scope);
+  info.SetFunctionScopeInfo(scope_info_list);
+
+  current_parent_index_ = info.GetParentIndex();
 }
 
+Handle<Object> LiveEditFunctionTracker::SerializeFunctionScope(Scope* scope) {
+  Handle<JSArray> scope_info_list = isolate_->factory()->NewJSArray(10);
+  int scope_info_length = 0;
+
+  // Saves some description of scope. It stores name and indexes of
+  // variables in the whole scope chain. Null-named slots delimit
+  // scopes of this chain.
+  Scope* current_scope = scope;
+  while (current_scope != NULL) {
+    HandleScope handle_scope(isolate_);
+    ZoneList<Variable*> stack_list(current_scope->StackLocalCount(), zone_);
+    ZoneList<Variable*> context_list(current_scope->ContextLocalCount(), zone_);
+    ZoneList<Variable*> globals_list(current_scope->ContextGlobalCount(),
+                                     zone_);
+    current_scope->CollectStackAndContextLocals(&stack_list, &context_list,
+                                                &globals_list);
+    context_list.Sort(&Variable::CompareIndex);
+
+    for (int i = 0; i < context_list.length(); i++) {
+      SetElementSloppy(scope_info_list, scope_info_length,
+                       context_list[i]->name());
+      scope_info_length++;
+      SetElementSloppy(
+          scope_info_list, scope_info_length,
+          Handle<Smi>(Smi::FromInt(context_list[i]->index()), isolate_));
+      scope_info_length++;
+    }
+    SetElementSloppy(scope_info_list, scope_info_length,
+                     Handle<Object>(isolate_->heap()->null_value(), isolate_));
+    scope_info_length++;
+
+    current_scope = current_scope->outer_scope();
+  }
 
-bool LiveEditFunctionTracker::IsActive(Isolate* isolate) {
-  return isolate->active_function_info_listener() != NULL;
+  return scope_info_list;
 }
 
 }  // namespace internal

src/debug/liveedit.h

@@ -32,25 +32,39 @@ namespace v8 {
 namespace internal {
 
 // This class collects some specific information on structure of functions
-// in a particular script. It gets called from compiler all the time, but
-// actually records any data only when liveedit operation is in process;
-// in any other time this class is very cheap.
+// in a particular script.
 //
 // The primary interest of the Tracker is to record function scope structures
-// in order to analyze whether function code maybe safely patched (with new
+// in order to analyze whether function code may be safely patched (with new
 // code successfully reading existing data from function scopes). The Tracker
 // also collects compiled function codes.
-class LiveEditFunctionTracker {
+class LiveEditFunctionTracker : public AstTraversalVisitor {
  public:
-  explicit LiveEditFunctionTracker(Isolate* isolate, FunctionLiteral* fun);
-  ~LiveEditFunctionTracker();
-  void RecordFunctionInfo(Handle<SharedFunctionInfo> info,
-                          FunctionLiteral* lit, Zone* zone);
+  // Traverses the entire AST, and records information about all
+  // FunctionLiterals for further use by LiveEdit code patching. The collected
+  // information is returned as a serialized array.
+  static Handle<JSArray> Collect(FunctionLiteral* node, Handle<Script> script,
+                                 Zone* zone, Isolate* isolate);
 
-  static bool IsActive(Isolate* isolate);
+  virtual ~LiveEditFunctionTracker() {}
+  void VisitFunctionLiteral(FunctionLiteral* node) override;
 
  private:
+  LiveEditFunctionTracker(Handle<Script> script, Zone* zone, Isolate* isolate);
+
+  void FunctionStarted(FunctionLiteral* fun);
+  void FunctionDone(Handle<SharedFunctionInfo> shared, Scope* scope);
+  Handle<Object> SerializeFunctionScope(Scope* scope);
+
+  Handle<Script> script_;
+  Zone* zone_;
   Isolate* isolate_;
+
+  Handle<JSArray> result_;
+  int len_;
+  int current_parent_index_;
+
+  DISALLOW_COPY_AND_ASSIGN(LiveEditFunctionTracker);
 };
 
 

src/isolate.h

@@ -58,7 +58,6 @@ class EmptyStatement;
 class ExternalCallbackScope;
 class ExternalReferenceTable;
 class Factory;
-class FunctionInfoListener;
 class HandleScopeImplementer;
 class HeapProfiler;
 class HStatistics;
@@ -383,8 +382,6 @@ typedef List<HeapObject*> DebugObjectCache;
   /* function cache of the native context. */                                  \
   V(int, next_serial_number, 0)                                                \
   V(ExternalReferenceRedirectorPointer*, external_reference_redirector, NULL)  \
-  /* Part of the state of liveedit. */                                         \
-  V(FunctionInfoListener*, active_function_info_listener, NULL)                \
   /* State for Relocatable. */                                                 \
   V(Relocatable*, relocatable_top, NULL)                                       \
   V(DebugObjectCache*, string_stream_debug_object_cache, NULL)                 \