Reparse closure instead of script for most uses of ScopeIterator

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}

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;

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();
 }
 

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()) {

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();
 

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()) {