[inspector] removed kDebugPromiseCollected event

With recent CLs we always store maximum N async stack traces and when we reach limit we drop half of them.
Current promise collected event requires creating weak handle:
- it takes time,
- it consumes memory.
Since async task id distribution for promises is uniform (each new promise has last_async_task_id + 1 as an id) our hash map is good enough to handle any amount of async task ids, following time of executing 1 000 000 000 of lookups:
- for empty hash map: 1.45 seconds,
- for hash map with one entry: 14.95 seconds
- 1024 entries: 15.03 seconds
- 1024 * 1024 entries: 14.82 seconds
- 1024 * 1024 * 1024: 17.9 seconds

BUG=v8:6189
R=dgozman@chromium.org,yangguo@chromium.org

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

src/debug/debug.cc

@@ -1888,32 +1888,6 @@ void Debug::OnAfterCompile(Handle<Script> script) {
 }
 
 namespace {
-struct CollectedCallbackData {
-  Object** location;
-  int id;
-  Debug* debug;
-  Isolate* isolate;
-
-  CollectedCallbackData(Object** location, int id, Debug* debug,
-                        Isolate* isolate)
-      : location(location), id(id), debug(debug), isolate(isolate) {}
-};
-
-void SendAsyncTaskEventCancel(const v8::WeakCallbackInfo<void>& info) {
-  std::unique_ptr<CollectedCallbackData> data(
-      reinterpret_cast<CollectedCallbackData*>(info.GetParameter()));
-  if (!data->debug->is_active()) return;
-  HandleScope scope(data->isolate);
-  data->debug->OnAsyncTaskEvent(debug::kDebugPromiseCollected, data->id, 0);
-}
-
-void ResetPromiseHandle(const v8::WeakCallbackInfo<void>& info) {
-  CollectedCallbackData* data =
-      reinterpret_cast<CollectedCallbackData*>(info.GetParameter());
-  GlobalHandles::Destroy(data->location);
-  info.SetSecondPassCallback(&SendAsyncTaskEventCancel);
-}
-
 // In an async function, reuse the existing stack related to the outer
 // Promise. Otherwise, e.g. in a direct call to then, save a new stack.
 // Promises with multiple reactions with one or more of them being async
@@ -1982,19 +1956,6 @@ int Debug::NextAsyncTaskId(Handle<JSObject> promise) {
       handle(Smi::FromInt(++thread_local_.async_task_count_), isolate_);
   Object::SetProperty(&it, async_id, SLOPPY, Object::MAY_BE_STORE_FROM_KEYED)
       .ToChecked();
-  Handle<Object> global_handle = isolate_->global_handles()->Create(*promise);
-  // We send EnqueueRecurring async task event when promise is fulfilled or
-  // rejected, WillHandle and DidHandle for every scheduled microtask for this
-  // promise.
-  // We need to send a cancel event when no other microtasks can be
-  // started for this promise and all current microtasks are finished.
-  // Since we holding promise when at least one microtask is scheduled (inside
-  // PromiseReactionJobInfo), we can send cancel event in weak callback.
-  GlobalHandles::MakeWeak(
-      global_handle.location(),
-      new CollectedCallbackData(global_handle.location(), async_id->value(),
-                                this, isolate_),
-      &ResetPromiseHandle, v8::WeakCallbackType::kParameter);
   return async_id->value();
 }
 

src/debug/interface-types.h

@@ -73,7 +73,6 @@ enum PromiseDebugActionType {
   kDebugEnqueueAsyncFunction,
   kDebugEnqueuePromiseResolve,
   kDebugEnqueuePromiseReject,
-  kDebugPromiseCollected,
   kDebugWillHandle,
   kDebugDidHandle,
 };

src/inspector/test-interface.cc

@@ -15,4 +15,10 @@ void SetMaxAsyncTaskStacksForTest(V8Inspector* inspector, int limit) {
       ->setMaxAsyncTaskStacksForTest(limit);
 }
 
-}  //  v8_inspector
+void DumpAsyncTaskStacksStateForTest(V8Inspector* inspector) {
+  static_cast<V8InspectorImpl*>(inspector)
+      ->debugger()
+      ->dumpAsyncTaskStacksStateForTest();
+}
+
+}  // namespace v8_inspector

src/inspector/test-interface.h

@@ -12,6 +12,7 @@ namespace v8_inspector {
 class V8Inspector;
 
 V8_EXPORT void SetMaxAsyncTaskStacksForTest(V8Inspector* inspector, int limit);
+V8_EXPORT void DumpAsyncTaskStacksStateForTest(V8Inspector* inspector);
 
 }  //  v8_inspector
 

src/inspector/v8-debugger.cc

@@ -666,10 +666,6 @@ void V8Debugger::PromiseEventOccurred(v8::debug::PromiseDebugActionType type,
       asyncTaskFinishedForStack(task);
       asyncTaskFinishedForStepping(task);
       break;
-    case v8::debug::kDebugPromiseCollected:
-      asyncTaskCanceledForStack(task);
-      asyncTaskCanceledForStepping(task);
-      break;
   }
 }
 
@@ -1053,4 +1049,14 @@ void V8Debugger::setMaxAsyncTaskStacksForTest(int limit) {
   m_maxAsyncCallStacks = limit;
 }
 
+void V8Debugger::dumpAsyncTaskStacksStateForTest() {
+  fprintf(stdout, "Async stacks count: %d\n", m_asyncStacksCount);
+  fprintf(stdout, "Scheduled async tasks: %zu\n", m_asyncTaskStacks.size());
+  fprintf(stdout, "Created async tasks: %zu\n",
+          m_asyncTaskCreationStacks.size());
+  fprintf(stdout, "Async tasks with parent: %zu\n", m_parentTask.size());
+  fprintf(stdout, "Recurring async tasks: %zu\n", m_recurringTasks.size());
+  fprintf(stdout, "\n");
+}
+
 }  // namespace v8_inspector

src/inspector/v8-debugger.h

@@ -106,6 +106,7 @@ class V8Debugger : public v8::debug::DebugDelegate {
   WasmTranslation* wasmTranslation() { return &m_wasmTranslation; }
 
   void setMaxAsyncTaskStacksForTest(int limit);
+  void dumpAsyncTaskStacksStateForTest();
 
  private:
   void compileDebuggerScript();

test/inspector/debugger/collect-obsolete-async-tasks-expected.txt

+Checks that we collect obsolete async tasks with async stacks.
+Async stacks count: 2
+Scheduled async tasks: 1
+Created async tasks: 1
+Async tasks with parent: 0
+Recurring async tasks: 1
+
+Async stacks count: 0
+Scheduled async tasks: 0
+Created async tasks: 0
+Async tasks with parent: 0
+Recurring async tasks: 0
+
+Async stacks count: 2
+Scheduled async tasks: 0
+Created async tasks: 2
+Async tasks with parent: 2
+Recurring async tasks: 0
+
+Async stacks count: 0
+Scheduled async tasks: 0
+Created async tasks: 0
+Async tasks with parent: 0
+Recurring async tasks: 0
+
+Async stacks count: 1
+Scheduled async tasks: 1
+Created async tasks: 0
+Async tasks with parent: 0
+Recurring async tasks: 0
+
+Async stacks count: 0
+Scheduled async tasks: 0
+Created async tasks: 0
+Async tasks with parent: 0
+Recurring async tasks: 0
+

test/inspector/debugger/collect-obsolete-async-tasks.js

+// Copyright 2017 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.
+
+InspectorTest.log('Checks that we collect obsolete async tasks with async stacks.');
+
+InspectorTest.addScript(`
+function test() {
+  setMaxAsyncTaskStacks(128);
+  var p = Promise.resolve();
+
+  dumpAsyncTaskStacksStateForTest();
+  setMaxAsyncTaskStacks(128);
+  dumpAsyncTaskStacksStateForTest();
+
+  p.then(() => 42).then(() => 239);
+
+  dumpAsyncTaskStacksStateForTest();
+  setMaxAsyncTaskStacks(128);
+  dumpAsyncTaskStacksStateForTest();
+
+  setTimeout(() => 42, 0);
+
+  dumpAsyncTaskStacksStateForTest();
+  setMaxAsyncTaskStacks(128);
+  dumpAsyncTaskStacksStateForTest();
+}
+`);
+
+(async function test() {
+  Protocol.Debugger.enable();
+  Protocol.Debugger.setAsyncCallStackDepth({maxDepth: 128});
+  await Protocol.Runtime.evaluate({expression: 'test()'});
+  InspectorTest.completeTest();
+})()

test/inspector/inspector-test.cc

@@ -439,6 +439,7 @@ class InspectorExtension : public v8::Extension {
                       "native function attachInspector();"
                       "native function detachInspector();"
                       "native function setMaxAsyncTaskStacks();"
+                      "native function dumpAsyncTaskStacksStateForTest();"
                       "native function breakProgram();"
                       "native function createObjectWithStrictCheck();"
                       "native function callWithScheduledBreak();"
@@ -466,6 +467,14 @@ class InspectorExtension : public v8::Extension {
                    .FromJust()) {
       return v8::FunctionTemplate::New(
           isolate, InspectorExtension::SetMaxAsyncTaskStacks);
+    } else if (name->Equals(context,
+                            v8::String::NewFromUtf8(
+                                isolate, "dumpAsyncTaskStacksStateForTest",
+                                v8::NewStringType::kNormal)
+                                .ToLocalChecked())
+                   .FromJust()) {
+      return v8::FunctionTemplate::New(
+          isolate, InspectorExtension::DumpAsyncTaskStacksStateForTest);
     } else if (name->Equals(context,
                             v8::String::NewFromUtf8(isolate, "breakProgram",
                                                     v8::NewStringType::kNormal)
@@ -538,6 +547,19 @@ class InspectorExtension : public v8::Extension {
         inspector, args[0].As<v8::Int32>()->Value());
   }
 
+  static void DumpAsyncTaskStacksStateForTest(
+      const v8::FunctionCallbackInfo<v8::Value>& args) {
+    if (args.Length() != 0) {
+      fprintf(stderr, "Internal error: dumpAsyncTaskStacksStateForTest().");
+      Exit();
+    }
+    v8_inspector::V8Inspector* inspector =
+        InspectorClientImpl::InspectorFromContext(
+            args.GetIsolate()->GetCurrentContext());
+    CHECK(inspector);
+    v8_inspector::DumpAsyncTaskStacksStateForTest(inspector);
+  }
+
   static void BreakProgram(const v8::FunctionCallbackInfo<v8::Value>& args) {
     if (args.Length() != 2 || !args[0]->IsString() || !args[1]->IsString()) {
       fprintf(stderr, "Internal error: breakProgram('reason', 'details').");