Reparse closure instead of script for most uses of ScopeIterator

szuend · Commit Bot · commit 2fe1552c5809 · 2019-12-04T09:35:22.000Z
The ScopeIterator only requires accurate information for the whole script during local debug-evaluate, when the accurate scope information is used to build stack local blacklists. Otherwise it is enough to only reparse the closure. This should recover some performance during stepping, especially with large stacks and scripts. Drive-by: Remove unused COLLECT_NON_LOCALS enum option. Bug: chromium:1028093, v8:9938 Change-Id: I6b3a34e9015e564d683e76b88388daabc426e1cb Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1948715 Commit-Queue: Simon Zünd <szuend@chromium.org> Reviewed-by: Yang Guo <yangguo@chromium.org> Cr-Commit-Position: refs/heads/master@{#65318}
diff --git a/src/debug/debug-evaluate.cc b/src/debug/debug-evaluate.cc
@@ -171,7 +171,7 @@ DebugEvaluate::ContextBuilder::ContextBuilder(Isolate* isolate,
     : isolate_(isolate),
       frame_inspector_(frame, inlined_jsframe_index, isolate),
       scope_iterator_(isolate, &frame_inspector_,
-                      ScopeIterator::COLLECT_NON_LOCALS) {
+                      ScopeIterator::ReparseStrategy::kScript) {
   Handle<Context> outer_context(frame_inspector_.GetFunction()->context(),
                                 isolate);
   evaluation_context_ = outer_context;
diff --git a/src/debug/debug-scope-iterator.cc b/src/debug/debug-scope-iterator.cc
@@ -50,7 +50,9 @@ namespace internal {
 
 DebugScopeIterator::DebugScopeIterator(Isolate* isolate,
                                        FrameInspector* frame_inspector)
-    : iterator_(isolate, frame_inspector) {
+    : iterator_(
+          isolate, frame_inspector,
+          ::v8::internal::ScopeIterator::ReparseStrategy::kFunctionLiteral) {
   if (!Done() && ShouldIgnore()) Advance();
 }
 
diff --git a/src/debug/debug-scopes.cc b/src/debug/debug-scopes.cc
@@ -23,7 +23,7 @@ namespace v8 {
 namespace internal {
 
 ScopeIterator::ScopeIterator(Isolate* isolate, FrameInspector* frame_inspector,
-                             ScopeIterator::Option option)
+                             ReparseStrategy strategy)
     : isolate_(isolate),
       frame_inspector_(frame_inspector),
       function_(frame_inspector_->GetFunction()),
@@ -37,7 +37,7 @@ ScopeIterator::ScopeIterator(Isolate* isolate, FrameInspector* frame_inspector,
   // We should not instantiate a ScopeIterator for wasm frames.
   DCHECK_NE(Script::TYPE_WASM, frame_inspector->GetScript()->type());
 
-  TryParseAndRetrieveScopes(option);
+  TryParseAndRetrieveScopes(strategy);
 }
 
 ScopeIterator::~ScopeIterator() { delete info_; }
@@ -72,7 +72,7 @@ ScopeIterator::ScopeIterator(Isolate* isolate,
       context_(generator->context(), isolate),
       script_(Script::cast(function_->shared().script()), isolate) {
   CHECK(function_->shared().IsSubjectToDebugging());
-  TryParseAndRetrieveScopes(DEFAULT);
+  TryParseAndRetrieveScopes(ReparseStrategy::kFunctionLiteral);
 }
 
 void ScopeIterator::Restart() {
@@ -195,7 +195,7 @@ class ScopeChainRetriever {
 
 }  // namespace
 
-void ScopeIterator::TryParseAndRetrieveScopes(ScopeIterator::Option option) {
+void ScopeIterator::TryParseAndRetrieveScopes(ReparseStrategy strategy) {
   // Catch the case when the debugger stops in an internal function.
   Handle<SharedFunctionInfo> shared_info(function_->shared(), isolate_);
   Handle<ScopeInfo> scope_info(shared_info->scope_info(), isolate_);
@@ -233,9 +233,17 @@ void ScopeIterator::TryParseAndRetrieveScopes(ScopeIterator::Option option) {
   }
 
   // Reparse the code and analyze the scopes.
+  // Depending on the choosen strategy, the whole script or just
+  // the closure is re-parsed for function scopes.
   Handle<Script> script(Script::cast(shared_info->script()), isolate_);
-  info_ = new ParseInfo(isolate_, script);
-  info_->set_eager();
+  if (scope_info->scope_type() == FUNCTION_SCOPE &&
+      strategy == ReparseStrategy::kFunctionLiteral) {
+    info_ = new ParseInfo(isolate_, shared_info);
+  } else {
+    info_ = new ParseInfo(isolate_, script);
+    info_->set_eager();
+  }
+
   if (scope_info->scope_type() == EVAL_SCOPE || script->is_wrapped()) {
     info_->set_eval();
     if (!context_->IsNativeContext()) {
diff --git a/src/debug/debug-scopes.h b/src/debug/debug-scopes.h
@@ -41,10 +41,13 @@ class ScopeIterator {
   static const int kScopeDetailsFunctionIndex = 5;
   static const int kScopeDetailsSize = 6;
 
-  enum Option { DEFAULT, COLLECT_NON_LOCALS };
+  enum class ReparseStrategy {
+    kScript,
+    kFunctionLiteral,
+  };
 
   ScopeIterator(Isolate* isolate, FrameInspector* frame_inspector,
-                Option options = DEFAULT);
+                ReparseStrategy strategy);
 
   ScopeIterator(Isolate* isolate, Handle<JSFunction> function);
   ScopeIterator(Isolate* isolate, Handle<JSGeneratorObject> generator);
@@ -129,7 +132,7 @@ class ScopeIterator {
 
   int GetSourcePosition();
 
-  void TryParseAndRetrieveScopes(ScopeIterator::Option option);
+  void TryParseAndRetrieveScopes(ReparseStrategy strategy);
 
   void UnwrapEvaluationContext();
 
diff --git a/src/debug/debug-stack-trace-iterator.cc b/src/debug/debug-stack-trace-iterator.cc
@@ -87,7 +87,9 @@ v8::MaybeLocal<v8::Value> DebugStackTraceIterator::GetReceiver() const {
     // Arrow function defined in top level function without references to
     // variables may have NativeContext as context.
     if (!context->IsFunctionContext()) return v8::MaybeLocal<v8::Value>();
-    ScopeIterator scope_iterator(isolate_, frame_inspector_.get());
+    ScopeIterator scope_iterator(
+        isolate_, frame_inspector_.get(),
+        ScopeIterator::ReparseStrategy::kFunctionLiteral);
     // We lookup this variable in function context only when it is used in arrow
     // function otherwise V8 can optimize it out.
     if (!scope_iterator.ClosureScopeHasThisReference()) {

Original file line number	Diff line number	Diff line change
`@@ -50,7 +50,9 @@ namespace internal {`
`50`	`50`
`51`	`51`	`DebugScopeIterator::DebugScopeIterator(Isolate* isolate,`
`52`	`52`	`FrameInspector* frame_inspector)`
`53`		`- : iterator_(isolate, frame_inspector) {`
	`53`	`+ : iterator_(`
	`54`	`+ isolate, frame_inspector,`
	`55`	`+ ::v8::internal::ScopeIterator::ReparseStrategy::kFunctionLiteral) {`
`54`	`56`	`if (!Done() && ShouldIgnore()) Advance();`
`55`	`57`	`}`
`56`	`58`