1. 08 Feb, 2022 1 commit
    • Benedikt Meurer's avatar
      [debug] Implement stepping out of async functions in the debugger. · 536e96cc
      Benedikt Meurer authored
      Previously the inspector was trying to handle step-out for async
      functions by annotating the async stacks, but this was merely a
      hack and didn't work reliably
      
      (a) when the async caller that is `await`ing the result of the
          callee was still in the synchronous part (because then there
          was no async task yet in the inspector), or
      (b) not at all when the async stack tracking wasn't enabled or the
          maximum async stack depth was too small.
      
      This CL replaces that hack with a pragmatic solution inside the
      V8 debugger, where upon `await` we memorize the async function
      object of the caller on the outer promise of the callee, and when
      stepping out of the callee we check whether the returned promise
      has a memorized async function object and if so, we schedule that
      to resume.
      
      This CL thereby effectively reverts https://crrev.com/c/1054618
      and replaces it with a V8 debug solution, and thereby further
      reduces the (memory) overhead of an AsyncStackTrace.
      
      Fixed: chromium:1246867
      Bug: v8:6161, v8:7753, chromium:1277451, chromium:1280519
      Change-Id: I6aa79e90f49d204f66bfd37e7a328c7fb8d635b1
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3439865Reviewed-by: 's avatarJaroslav Sevcik <jarin@chromium.org>
      Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
      Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
      Cr-Commit-Position: refs/heads/main@{#78990}
      536e96cc
  2. 04 Jan, 2022 1 commit
    • Benedikt Meurer's avatar
      [inspector] Fix `Runtime.setMaxCallStackSizeToCapture`. · 8f8d2fe4
      Benedikt Meurer authored
      This change fixes the implementation of the previously introduced API
      `Runtime.setMaxCallStackSizeToCapture` to work correctly and also apply
      (consistently) to stack traces captured by V8 when exceptions are
      thrown. It does so in a fully backwards compatible manner.
      
      This change thus makes the previous fix for catapult (which landed in
      http://crrev.com/c/3347789) effective, and therefore ensures that real
      world performance benchmarks aren't affected by the use of the `Runtime`
      domain in the catapult test framework.
      
      Note this is basically a reland of crrev.com/c/3361839, but without
      touching the stack traces for console messages (which led to the
      regressions in crbug/1283516, crbug/1283523, etc.).
      
      Fixed: chromium:1280831
      Bug: chromium:1283162, chromium:1278650, chromium:1258599
      Bug: chromium:1280803, chromium:1280832, chromium:1280818
      Doc: https://bit.ly/v8-cheaper-inspector-stack-traces
      Change-Id: I3dcec7b75d76ca267fac8bd6fcb2cda60d5e60dd
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3364086Reviewed-by: 's avatarYang Guo <yangguo@chromium.org>
      Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
      Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
      Cr-Commit-Position: refs/heads/main@{#78479}
      8f8d2fe4
  3. 03 Jan, 2022 1 commit
    • Benedikt Meurer's avatar
      Revert "[inspector] Fix `Runtime.setMaxCallStackSizeToCapture`." · c51b582d
      Benedikt Meurer authored
      This reverts commit 34f73cc7.
      
      Reason for revert: Performance regressions throughout a lot of
      system health and browsing benchmarks.
      
      Original change's description:
      > [inspector] Fix `Runtime.setMaxCallStackSizeToCapture`.
      >
      > This change fixes the implementation of the previously introduced API
      > `Runtime.setMaxCallStackSizeToCapture` to work correctly and also apply
      > (consistently) to stack traces captured by V8 when exceptions are
      > thrown. It does so in a fully backwards compatible manner.
      >
      > This change thus makes the previous fix for catapult (which landed in
      > http://crrev.com/c/3347789) effective, and therefore ensures that real
      > world performance benchmarks aren't affected by the use of the `Runtime`
      > domain in the catapult test framework.
      >
      > Bug: chromium:1283162, chromium:1278650, chromium:1258599
      > Bug: chromium:1280803, chromium:1280832, chromium:1280818
      > Fixed: chromium:1280831
      > Doc: https://bit.ly/v8-cheaper-inspector-stack-traces
      > Change-Id: I4ec951a858317fa49096cd4023deb0104d92c9c9
      > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3361839
      > Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
      > Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
      > Reviewed-by: Yang Guo <yangguo@chromium.org>
      > Cr-Commit-Position: refs/heads/main@{#78458}
      
      Bug: chromium:1283162, chromium:1278650, chromium:1258599
      Bug: chromium:1280803, chromium:1280832, chromium:1280818
      Bug: chromium:1280831
      Change-Id: Id1efaffa2f7f08c47f833f68b8a297494edee21e
      Fixed: chromium:1283751, chromium:1283749, chromium:1283746
      Fixed: chromium:1283729, chromium:1283700, chromium:1283700
      Fixed: chromium:1283691, chromium:1283687, chromium:1283678
      Fixed: chromium:1283677, chromium:1283676, chromium:1283675
      Fixed: chromium:1283674, chromium:1283618, chromium:1283536
      Fixed: chromium:1283523, chromium:1283516
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3364078
      Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
      Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
      Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
      Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
      Reviewed-by: 's avatarYang Guo <yangguo@chromium.org>
      Cr-Commit-Position: refs/heads/main@{#78462}
      c51b582d
  4. 31 Dec, 2021 1 commit
  5. 24 Nov, 2021 1 commit
  6. 11 Nov, 2021 1 commit
    • Benedikt Meurer's avatar
      [inspector] Cache StackFrames by script, line and column number. · e60dc99e
      Benedikt Meurer authored
      This introduces a stack frame cache on the V8Debugger level, which
      de-duplicates StackFrame instances based on their scriptId, line and
      column number.
      
      This greatly reduces the memory pressure when debugging huge Web
      applications that have a lot of async activity (and potentially
      have scripts with huge URLs). This is guided by the observation
      that even in huge applications, there are only a very limited
      number of call sites that initiate async activity and hence we
      only have a limited number of distinct StackFrames to worry
      about (despite having to maintain a large number of async stack
      traces overall).
      
      As a nice side effect, this CL also greatly reduces the negative
      performance impact of collecting async stack traces in these
      huge applications.
      
      Generally speaking this is mostly duct tape however, and we might
      want to follow up with changes to make capturing (and storing)
      stack frames even cheaper.
      
      Fixed: chromium:1268436
      Change-Id: Ib212b3c97dce2bb7ca47d5875d45cf20b9b97afe
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3272577
      Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
      Reviewed-by: 's avatarSimon Zünd <szuend@chromium.org>
      Cr-Commit-Position: refs/heads/main@{#77835}
      e60dc99e
  7. 30 Sep, 2021 1 commit
    • Benedikt Meurer's avatar
      [inspector] Align async task frame reporting for `await`. · d6c01059
      Benedikt Meurer authored
      The V8 Inspector was sending an additional frame as part of async stack
      traces for async functions, which pointed to the first executed `await`
      in the async function. This is leaking an implementation detail of how
      (and more precisely when) the inspector decides to collect this stack
      trace. From the users perspective the async part of the stack trace is
      supposed to capture what happened _prior to the task_ - meaning in case
      of async functions: What lead to the execution of the async function.
      This is reflected by the fact that the DevTools front-end (and the V8
      Inspector itself) performs post-processing on these async call stacks,
      removing the misleading top frame from it. But this post-processing is
      not applied consistently to all async stack traces (i.e. the Console
      message stack traces don't get this), and potentially also not applied
      consistently across consumers of the Chromium debugger backend.
      
      Instead the V8 Inspector now removes the top frame itself and thus
      reports `await` consistently with how other async tasks are reported to
      debugger front-ends.
      
      Note: This preserves backwards compatibility with old versions of
      devtools-frontend, which do post-processing (for the Call Stack) only on
      async stack traces marked with "async function", while we now mark these
      async stack traces with "await" instead (aligned with what the front-end
      is using as user visibile string anyways in the Call Stack section, and
      this matching will be updated in a separate follow up CL to look for
      "await" instead of "async function").
      
      Before: https://imgur.com/kIrWcIc.png
      After: https://imgur.com/HvZGqiP
      Fixed: chromium:1254259
      Bug: chromium:1229662
      Change-Id: I57ce051a28892177b6b96221f083ae957f967e52
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3193535
      Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
      Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
      Reviewed-by: 's avatarKim-Anh Tran <kimanh@chromium.org>
      Cr-Commit-Position: refs/heads/main@{#77157}
      d6c01059
  8. 24 Aug, 2021 1 commit
    • Dan Elphick's avatar
      Reland "[include] Split out v8.h" · ec06bb6c
      Dan Elphick authored
      This is a reland of d1b27019
      
      Fixes include:
      Adding missing file to bazel build
      Forward-declaring classing before friend-classing them to fix win/gcc
      Add missing v8-isolate.h include for vtune builds
      
      Original change's description:
      > [include] Split out v8.h
      >
      > This moves every single class/function out of include/v8.h into a
      > separate header in include/, which v8.h then includes so that
      > externally nothing appears to have changed.
      >
      > Every include of v8.h from inside v8 has been changed to a more
      > fine-grained include.
      >
      > Previously inline functions defined at the bottom of v8.h would call
      > private non-inline functions in the V8 class. Since that class is now
      > in v8-initialization.h and is rarely included (as that would create
      > dependency cycles), this is not possible and so those methods have been
      > moved out of the V8 class into the namespace v8::api_internal.
      >
      > None of the previous files in include/ now #include v8.h, which means
      > if embedders were relying on this transitive dependency then it will
      > give compile failures.
      >
      > v8-inspector.h does depend on v8-scripts.h for the time being to ensure
      > that Chrome continue to compile but that change will be reverted once
      > those transitive #includes in chrome are changed to include it directly.
      >
      > Full design:
      > https://docs.google.com/document/d/1rTD--I8hCAr-Rho1WTumZzFKaDpEp0IJ8ejZtk4nJdA/edit?usp=sharing
      >
      > Bug: v8:11965
      > Change-Id: I53b84b29581632710edc80eb11f819c2097a2877
      > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3097448
      > Reviewed-by: Yang Guo <yangguo@chromium.org>
      > Reviewed-by: Camillo Bruni <cbruni@chromium.org>
      > Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
      > Reviewed-by: Leszek Swirski <leszeks@chromium.org>
      > Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
      > Commit-Queue: Dan Elphick <delphick@chromium.org>
      > Cr-Commit-Position: refs/heads/main@{#76424}
      
      Cq-Include-Trybots: luci.v8.try:v8_linux_vtunejit
      Bug: v8:11965
      Change-Id: I99f5d3a73bf8fe25b650adfaf9567dc4e44a09e6
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3113629Reviewed-by: 's avatarLeszek Swirski <leszeks@chromium.org>
      Reviewed-by: 's avatarCamillo Bruni <cbruni@chromium.org>
      Reviewed-by: 's avatarMichael Lippautz <mlippautz@chromium.org>
      Reviewed-by: 's avatarJakob Kummerow <jkummerow@chromium.org>
      Reviewed-by: 's avatarSimon Zünd <szuend@chromium.org>
      Commit-Queue: Dan Elphick <delphick@chromium.org>
      Cr-Commit-Position: refs/heads/main@{#76460}
      ec06bb6c
  9. 23 Aug, 2021 2 commits
    • Dan Elphick's avatar
      Revert "[include] Split out v8.h" · 44fe02ce
      Dan Elphick authored
      This reverts commit d1b27019.
      
      Reason for revert: Broke vtune build, tsan build and possibly others
      
      Original change's description:
      > [include] Split out v8.h
      >
      > This moves every single class/function out of include/v8.h into a
      > separate header in include/, which v8.h then includes so that
      > externally nothing appears to have changed.
      >
      > Every include of v8.h from inside v8 has been changed to a more
      > fine-grained include.
      >
      > Previously inline functions defined at the bottom of v8.h would call
      > private non-inline functions in the V8 class. Since that class is now
      > in v8-initialization.h and is rarely included (as that would create
      > dependency cycles), this is not possible and so those methods have been
      > moved out of the V8 class into the namespace v8::api_internal.
      >
      > None of the previous files in include/ now #include v8.h, which means
      > if embedders were relying on this transitive dependency then it will
      > give compile failures.
      >
      > v8-inspector.h does depend on v8-scripts.h for the time being to ensure
      > that Chrome continue to compile but that change will be reverted once
      > those transitive #includes in chrome are changed to include it directly.
      >
      > Full design:
      > https://docs.google.com/document/d/1rTD--I8hCAr-Rho1WTumZzFKaDpEp0IJ8ejZtk4nJdA/edit?usp=sharing
      >
      > Bug: v8:11965
      > Change-Id: I53b84b29581632710edc80eb11f819c2097a2877
      > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3097448
      > Reviewed-by: Yang Guo <yangguo@chromium.org>
      > Reviewed-by: Camillo Bruni <cbruni@chromium.org>
      > Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
      > Reviewed-by: Leszek Swirski <leszeks@chromium.org>
      > Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
      > Commit-Queue: Dan Elphick <delphick@chromium.org>
      > Cr-Commit-Position: refs/heads/main@{#76424}
      
      Bug: v8:11965
      Change-Id: Id57313ae992e720c8b19abc975cd69729e1344aa
      No-Presubmit: true
      No-Tree-Checks: true
      No-Try: true
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3113627
      Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
      Commit-Queue: Leszek Swirski <leszeks@chromium.org>
      Owners-Override: Leszek Swirski <leszeks@chromium.org>
      Cr-Commit-Position: refs/heads/main@{#76428}
      44fe02ce
    • Dan Elphick's avatar
      [include] Split out v8.h · d1b27019
      Dan Elphick authored
      This moves every single class/function out of include/v8.h into a
      separate header in include/, which v8.h then includes so that
      externally nothing appears to have changed.
      
      Every include of v8.h from inside v8 has been changed to a more
      fine-grained include.
      
      Previously inline functions defined at the bottom of v8.h would call
      private non-inline functions in the V8 class. Since that class is now
      in v8-initialization.h and is rarely included (as that would create
      dependency cycles), this is not possible and so those methods have been
      moved out of the V8 class into the namespace v8::api_internal.
      
      None of the previous files in include/ now #include v8.h, which means
      if embedders were relying on this transitive dependency then it will
      give compile failures.
      
      v8-inspector.h does depend on v8-scripts.h for the time being to ensure
      that Chrome continue to compile but that change will be reverted once
      those transitive #includes in chrome are changed to include it directly.
      
      Full design:
      https://docs.google.com/document/d/1rTD--I8hCAr-Rho1WTumZzFKaDpEp0IJ8ejZtk4nJdA/edit?usp=sharing
      
      Bug: v8:11965
      Change-Id: I53b84b29581632710edc80eb11f819c2097a2877
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3097448Reviewed-by: 's avatarYang Guo <yangguo@chromium.org>
      Reviewed-by: 's avatarCamillo Bruni <cbruni@chromium.org>
      Reviewed-by: 's avatarJakob Kummerow <jkummerow@chromium.org>
      Reviewed-by: 's avatarLeszek Swirski <leszeks@chromium.org>
      Reviewed-by: 's avatarMichael Lippautz <mlippautz@chromium.org>
      Commit-Queue: Dan Elphick <delphick@chromium.org>
      Cr-Commit-Position: refs/heads/main@{#76424}
      d1b27019
  10. 23 Jul, 2021 1 commit
  11. 06 May, 2021 1 commit
    • Benedikt Meurer's avatar
      [inspector] Consistently pass around script ID as integer. · 51fe5571
      Benedikt Meurer authored
      Within the inspector we should be consistent about passing the script ID
      always as integer, and only convert to String16 when actually needed.
      That (a) saves memory (and some runtime overhead) when stashing away
      call frames, for example in case of async stack traces, and (b) reduces
      confusion which representation to chose.
      
      Bug: chromium:1162229
      Change-Id: I9591931da0a307779372f36aba6e155ec22bbe3d
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2876856
      Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
      Commit-Queue: Yang Guo <yangguo@chromium.org>
      Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
      Reviewed-by: 's avatarYang Guo <yangguo@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#74410}
      51fe5571
  12. 15 Dec, 2020 1 commit
  13. 18 Nov, 2020 1 commit
  14. 09 Jan, 2020 1 commit
  15. 26 Jun, 2019 1 commit
  16. 18 Oct, 2018 1 commit
  17. 09 Aug, 2018 1 commit
    • Alexey Kozyatinskiy's avatar
      [inspector] added V8InspectorClient::resourceNameToUrl · dbfcc487
      Alexey Kozyatinskiy authored
      Some clients (see Node.js) use platform path as ScriptOrigin.
      Reporting platform path in protocol makes using protocol much harder.
      This CL introduced V8InspectorClient::resourceNameToUrl method that
      is called for any reported using protocol url.
      V8Inspector uses url internally as well so protocol client may generate
      pattern for blackboxing with file urls only and does not need to build
      complicated regexp that covers files urls and platform paths on
      different platforms.
      
      R=lushnikov@chromium.org
      TBR=yangguo@chromium.org
      
      Bug: none
      Cq-Include-Trybots: luci.chromium.try:linux_chromium_headless_rel;luci.chromium.try:linux_chromium_rel_ng;master.tryserver.blink:linux_trusty_blink_rel
      Change-Id: Iff302e7441df922fa5d689fe510f5a9bfd470b9b
      Reviewed-on: https://chromium-review.googlesource.com/1164624
      Commit-Queue: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org>
      Reviewed-by: 's avatarAlexei Filippov <alph@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#55029}
      dbfcc487
  18. 23 Jul, 2018 1 commit
  19. 04 Jun, 2018 1 commit
  20. 31 May, 2018 1 commit
  21. 07 May, 2018 1 commit
  22. 02 Feb, 2018 1 commit
  23. 14 Dec, 2017 1 commit
  24. 23 Nov, 2017 1 commit
  25. 22 Nov, 2017 3 commits
    • Alexey Kozyatinskiy's avatar
      Reland [inspector] introduced stackTraceId and externalAsyncTask API · 885a5669
      Alexey Kozyatinskiy authored
      Sometimes we need to capture stack trace on one debugger and use it
      later as a parent stack on another debugger (e.g. worker.postMessage).
      
      This CL includes following addition to our protocol and v8-inspector.h:
        - added Runtime.StackTraceId, this id represents stack trace captured
          on debugger with given id,
        - protocol client can fetch Runtime.StackTrace by
          Runtime.StacKTraceId using Debugger.getStackTrace method,
        - externalParent field is added to Debugger.paused event, it may
          contain external parent stack trace,
        - V8Inspector::storeCurrentStackTrace captures current stack trace
          and returns V8StackTraceId for embedder this id can be used as
          argument for V8Inspector::externalAsyncTaskStarted and
          V8Inspector::externalAsyncTaskFinished method. Any async stack
          trace captured between these calls will get passed external stack
          trace as external parent. These methods are designed to be called
          on different debuggers. If async task is scheduled and started on
          one debugger user should continue to use asyncTask* API,
        - Debugger.enable methods returns unique debuggerId.
      
      TBR=dgozman@chromium.org,jgruber@chromium.org
      
      Bug: chromium:778796
      Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_chromium_rel_ng
      Change-Id: I2c1a2b2e30ed69ccb61d10f08686f4edb09f50e4
      Reviewed-on: https://chromium-review.googlesource.com/786274
      Commit-Queue: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org>
      Reviewed-by: 's avatarAleksey Kozyatinskiy <kozyatinskiy@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#49591}
      885a5669
    • Clemens Hammacher's avatar
      Revert "[inspector] introduced stackTraceId and externalAsyncTask API" · 4379533c
      Clemens Hammacher authored
      This reverts commit 3a41b697.
      
      Reason for revert: Break msvc: https://build.chromium.org/p/client.v8/builders/V8%20Win64%20-%20msvc/builds/250
      
      Original change's description:
      > [inspector] introduced stackTraceId and externalAsyncTask API
      > 
      > Sometimes we need to capture stack trace on one debugger and use it
      > later as a parent stack on another debugger (e.g. worker.postMessage).
      > 
      > This CL includes following addition to our protocol and v8-inspector.h:
      >   - added Runtime.StackTraceId, this id represents stack trace captured
      >     on debugger with given id,
      >   - protocol client can fetch Runtime.StackTrace by
      >     Runtime.StacKTraceId using Debugger.getStackTrace method,
      >   - externalParent field is added to Debugger.paused event, it may
      >     contain external parent stack trace,
      >   - V8Inspector::storeCurrentStackTrace captures current stack trace
      >     and returns V8StackTraceId for embedder this id can be used as
      >     argument for V8Inspector::externalAsyncTaskStarted and
      >     V8Inspector::externalAsyncTaskFinished method. Any async stack
      >     trace captured between these calls will get passed external stack
      >     trace as external parent. These methods are designed to be called
      >     on different debuggers. If async task is scheduled and started on
      >     one debugger user should continue to use asyncTask* API,
      >   - Debugger.enable methods returns unique debuggerId.
      > 
      > Bug: chromium:778796
      > Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_chromium_rel_ng
      > Change-Id: I16aba0d04bfcea90f3e187e635a0588c92354539
      > Reviewed-on: https://chromium-review.googlesource.com/754183
      > Reviewed-by: Jakob Gruber <jgruber@chromium.org>
      > Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
      > Commit-Queue: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org>
      > Cr-Commit-Position: refs/heads/master@{#49582}
      
      TBR=dgozman@chromium.org,pfeldman@chromium.org,yangguo@chromium.org,kozyatinskiy@chromium.org,jgruber@chromium.org
      
      Change-Id: I9b52354fa0841e5148596cf594317f2e5fe508ea
      No-Presubmit: true
      No-Tree-Checks: true
      No-Try: true
      Bug: chromium:778796
      Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_chromium_rel_ng
      Reviewed-on: https://chromium-review.googlesource.com/786152Reviewed-by: 's avatarClemens Hammacher <clemensh@chromium.org>
      Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#49584}
      4379533c
    • Alexey Kozyatinskiy's avatar
      [inspector] introduced stackTraceId and externalAsyncTask API · 3a41b697
      Alexey Kozyatinskiy authored
      Sometimes we need to capture stack trace on one debugger and use it
      later as a parent stack on another debugger (e.g. worker.postMessage).
      
      This CL includes following addition to our protocol and v8-inspector.h:
        - added Runtime.StackTraceId, this id represents stack trace captured
          on debugger with given id,
        - protocol client can fetch Runtime.StackTrace by
          Runtime.StacKTraceId using Debugger.getStackTrace method,
        - externalParent field is added to Debugger.paused event, it may
          contain external parent stack trace,
        - V8Inspector::storeCurrentStackTrace captures current stack trace
          and returns V8StackTraceId for embedder this id can be used as
          argument for V8Inspector::externalAsyncTaskStarted and
          V8Inspector::externalAsyncTaskFinished method. Any async stack
          trace captured between these calls will get passed external stack
          trace as external parent. These methods are designed to be called
          on different debuggers. If async task is scheduled and started on
          one debugger user should continue to use asyncTask* API,
        - Debugger.enable methods returns unique debuggerId.
      
      Bug: chromium:778796
      Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_chromium_rel_ng
      Change-Id: I16aba0d04bfcea90f3e187e635a0588c92354539
      Reviewed-on: https://chromium-review.googlesource.com/754183Reviewed-by: 's avatarJakob Gruber <jgruber@chromium.org>
      Reviewed-by: 's avatarDmitry Gozman <dgozman@chromium.org>
      Commit-Queue: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#49582}
      3a41b697
  26. 21 Nov, 2017 1 commit
    • Alexey Kozyatinskiy's avatar
      [inspector] reworked async instrumentation for promises · ed9b2072
      Alexey Kozyatinskiy authored
      Old instrumentation was designed to collect promise creation stack and
      promise scheduled stack together. In DevTools for last 6 months we
      show only creation stack for promises. We got strong support from users
      for new model. Now we can drop support for scheduled stacks and
      simplify implementation.
      
      New promise instrumentation is straightforward:
      - we send kDebugPromiseThen when promise is created by .then call,
      - we send kDebugPromiseCatch when promise is created by .catch call,
      - we send kDebugWillHandle before chained callback and kDebugDidHandle
        after chained callback,
      - and we send separate kDebugAsyncFunctionPromiseCreated for internal
        promise inside async await function.
      
      Advantages:
      - we reduce amount of captured stacks (we do not capture stack for
        promise that constructed not by .then or .catch),
      - we can consider async task related to .then and .catch as one shot
        since chained callback is executed once,
      - on V8 side we can implement required instrumentation using only
        promise hooks,
      
      Disadvantage:
      - see await-promise test, sometimes scheduled stack was useful since we
        add catch handler in native code,
      
      Implementation details:
      - on kInit promise hook we need to figure out why promise was created.
        We analyze builtin functions until first user defined function on
        current stack. If there is kAsyncFunctionPromiseCreate function then
        we send kDebugAsyncFunctionPromiseCreated event. If there is
        kPromiseThen or kPromiseCatch then only if this function is bottom
        builtin function we send corresponded event to inspector. We need it
        because Promise.all internally calls .then and in this case we have
        Promise.all and Promise.then on stack at the same time and we do not
        need to report this internally created promise to inspector.
      
      Bug: chromium:778796
      Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
      Change-Id: I53f47ce8c5c4a9897655c3396c249ea59529ae47
      Reviewed-on: https://chromium-review.googlesource.com/765208
      Commit-Queue: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org>
      Reviewed-by: 's avatarSathya Gunasekaran <gsathya@chromium.org>
      Reviewed-by: 's avatarYang Guo <yangguo@chromium.org>
      Reviewed-by: 's avatarDmitry Gozman <dgozman@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#49553}
      ed9b2072
  27. 16 May, 2017 1 commit
  28. 11 May, 2017 1 commit
  29. 10 May, 2017 1 commit
  30. 09 May, 2017 1 commit
  31. 28 Apr, 2017 1 commit
    • kozyatinskiy's avatar
      [inspector] better stacks for promises · f2bd913c
      kozyatinskiy authored
      - we should always set creation async stack if it's available regardless existing of current parent async stack,
      - we should cleanup parent link iff there is no creation and schedule async stack for parent.
      
      Let's consider example: Promise.resolve().then(x => x).then(x => x), there is three promises which will call following instrumentation:
      1) created #1 (Promise.resolve()) - collected stack #1
      2) scheduled #1 - collected stack #2
      3) created #2 with #1 as parent (first .then) - collected stack #3
      4) created #3 with #2 as parent (first .then) - collected stack #4
      5) started #2 - use stack #2 as scheduled
      6) scheduled #2 - collected stack #6
      7) finished #2
      8) started #3 - use stack #6 as scheduled
      9) scheduled #3 - collected stack #7
      10) finished #3
      
      If we collect stacks between step 4 and 5, it's possible to collect scheduled stack #2 but still have creation stack for #2 - stack #3 - so we always need to add creation event if scheduled is collected.
      
      If we collect stacks between created and scheduled we should not remove parent link even if parent was not scheduled yet.
      
      BUG=v8:6189
      R=dgozman@chromium.org
      
      Review-Url: https://codereview.chromium.org/2844753002
      Cr-Commit-Position: refs/heads/master@{#44990}
      f2bd913c
  32. 20 Apr, 2017 1 commit
  33. 18 Apr, 2017 3 commits
  34. 12 Apr, 2017 1 commit
  35. 26 Jan, 2017 1 commit