1. 15 Feb, 2021 1 commit
    • Benedikt Meurer's avatar
      [stack-traces] Cache source position on StackFrameInfos. · 7b07c779
      Benedikt Meurer authored
      Previously we had cached the source position information on
      JSStackFrame (C++) objects and reused that between calls to
      GetLineNumber() and GetColumnNumber(). The refactoring in
      https://crrev.com/eed0d27c2f774b3adbc85d0a5fb30a8cf0f018a8
      effectively removed that cache, while still making things
      faster though.
      
      This CL puts back the caching on the StackFrameInfo objects
      by reusing the `offset` slot to store the computed source
      position (as indicated by a bit in the `flags`). For promise
      combinator async frames, the bit is always set and the
      `offset_or_source_position` slot thus always contains the source
      position (aka the `promise index` in this case). We also
      added a `StackFrameInfo::ComputeLocation()` method to remove the
      last remaining place where we'd peek into the StackFrameInfo from
      outside stack-frame-info.{cc,h}.
      
      Also-By: kimanh@chromium.org
      Bug: chromium:1077657, v8:8742, chromium:1069425
      Change-Id: I59e26a91965617163776e6cc2610b88e6925452c
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2695386
      Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
      Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
      Reviewed-by: 's avatarYang Guo <yangguo@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#72752}
      7b07c779
  2. 12 Feb, 2021 1 commit
    • Benedikt Meurer's avatar
      [stack-traces] Simplify and speedup stack trace collection. · eed0d27c
      Benedikt Meurer authored
      Following up on https://crrev.com/c/2689185, this CL significantly
      simplifies the whole implementation of the stack trace capturing.
      
      Before this CL, capturing any stack trace (for the purpose of the API or
      Error.stack) would roughly work like this:
      
        1. The CaptureStackTrace() function uses the StackFrameIterator to
           walk the system stack. For each native frame it uses the
           FrameSummary abstraction to get all (including potentially inlined)
           frames. For each of those it appends a record consisting of six
           elements to a FrameArray (this holds pointers to the actual
           closures and receivers).
        2. Afterwards the FrameArray is shrinked to the required size, and a
           new FixedArray is allocated, and initialized with new
           StackTraceFrame objects where each holds a reference to the
           FrameArray, the index of the frame, and an initially uninitialized
           StackFrameInfo reference. This new FixedArray is then returned from
           CaptureStackTrace() and either stored on a message object or
           provided to the API as v8::StackTrace.
      
      The new approach removes a lot of the machinery in between and directly
      creates a FixedArray of StackFrameInfo objects in CaptureStackTrace().
      These StackFrameInfo objects are directly exposed as v8::StackFrame on
      the public API, and they hold the six fields that were previously stored
      flat in the FrameArray. This not only avoids a lot of copying around of
      data and creation of temporary objects and handles, but most importantly
      unifies and simplifies the stack frame function inside StackFrameInfo,
      so you no longer need to wonder which function / object might be
      responsible for a certain API.
      
      There's still a lot of room for improvement. In particular we currently
      don't cache the source position for a given StackFrameInfo (or
      globally), but rather recompute it every time. This is still very fast,
      significantly faster than the previous approach.
      
      There are some notable (potentially user visible) changes:
      
        - The CallSite#GetPosition() method now consistently returns the
          Wasm module relative bytecode offset for all Wasm frames (previously
          it'd return the function relative bytecode offset for non-asm.js
          Wasm frames).
        - The column and line numbers returned from StackFrameInfo methods are
          consistently 1-based now, instead of sometimes being 0-based (Wasm)
          and sometimes being 1-based (JS and asm.js Wasm). The only
          potentially noticable difference is that for
          CallSite#GetLineNumber() no longer returns 0 for Wasm frames, but
          that was wrong and useless anyways.
        - CallSite#GetThis() would sometimes return the_hole, another bug
          flushed out by this CL.
      
      The CL also contains some other not noteworthy drive-by-cleanups.
      
      Fixed: chromium:1057211
      Bug: chromium:1077657, chromium:1069425, v8:8742
      Bug: chromium:1127391, chromium:1098530, chromium:981541
      Change-Id: Iff12f6838a4d99080db8dd96bccc14440affc5a5
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2689183
      Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
      Reviewed-by: 's avatarYang Guo <yangguo@chromium.org>
      Reviewed-by: 's avatarSimon Zünd <szuend@chromium.org>
      Reviewed-by: 's avatarJakob Kummerow <jkummerow@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#72694}
      eed0d27c
  3. 11 Feb, 2021 1 commit
    • Benedikt Meurer's avatar
      [stack-traces] Remove StackFrameInfo. · 11b6f176
      Benedikt Meurer authored
      For a long time, V8 had two distinct ways to capture and store a stack
      trace, one where we'd just collect and symbolize the information for the
      v8::StackTrace API (script id, name, line and colum information mostly),
      and one where V8 would also memorize the closures, receivers, and
      optionally the parameters of the stack frame, which we use for
      Error.stack and the non-standard CallSite APIs. Those two were often out
      of sync and suffered from various different issues. Eventually they were
      refactored into a single captureStackTrace() bottleneck that would
      produce a FrameArray.
      
      This CL is a logical continuation of the refactorings. It repairs a
      regression where we'd compute the method name (as part of the
      cached StackFrameInfo) even if we don't need them (as is the case for
      the inspector and any other use of the v8::StackTrace API).
      
      Everytime a method was invoked on StackTraceFrame, it'd call into
      StackTraceFrame::GetInfo(), which would lazily setup the StackFrameInfo
      like this:
      
        1. Create a FrameArrayIterator and point it to the FrameArray at the
           index stored in the StackTraceFrame.
        2. Invoke FrameArrayIterator::Frame(), which copies the information
           from the FrameArray into a temporary JSStackFrame, AsmJsStackFrame
           or WasmStackFrame C++ object, and use the StackFrameBase virtual
           methods to transfer all information to a newly created
           StackFrameInfo object.
        3. Kill the link to the FrameArray and put a link to the
           StackFrameInfo object into the StackTraceFrame.
      
      This caching turned out to be extremely costly, since beyond other
      things, it'd always invoke JSStackFrame::GetMethodName(), which is
      extremely costly (the execution time is linear in the number of
      properties on the receiver and it's prototype chain). The cost was so
      high that several work-arounds had been added, which would avoid
      triggering the eager construction of the StackFrameInfo object (i.e.
      https://crrev.com/c/2080663, https://crrev.com/c/2550504 or
      https://crrev.com/c/2261736, but also https://crrev.com/c/1688927).
      
      This CL removes the StackFrameInfo caching completely, since neither the
      inspector nor Error.stack benefit from the caching at all. It's only the
      first part in a series of refactorings that will significantly reduce
      the complexity and overhead of the stack trace collection.
      
      Doc: https://bit.ly/2wkbuIy
      Bug: chromium:1057211, chromium:1077657, chromium:1069425, v8:8742
      Bug: chromium:1127391, chromium:1098530, chromium:981541
      Change-Id: I8edb8ff48b620eb3043ae51ab4ea27146ef0a5a2
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2689185
      Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
      Commit-Queue: Yang Guo <yangguo@chromium.org>
      Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
      Reviewed-by: 's avatarSimon Zünd <szuend@chromium.org>
      Reviewed-by: 's avatarYang Guo <yangguo@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#72647}
      11b6f176
  4. 01 Sep, 2020 1 commit
  5. 29 May, 2020 1 commit
  6. 26 May, 2020 1 commit
    • Seth Brenith's avatar
      Revert "[torque][cleanup] Use more precise field types in a few classes" · 16cb2d94
      Seth Brenith authored
      This reverts commit 4e5fabae.
      
      Reason for revert: performance regressions chromium:1085305, chromium:1084978
      
      Original change's description:
      > [torque][cleanup] Use more precise field types in a few classes
      > 
      > This change updates some Torque-defined classes to include more precise
      > field types where possible. It also updates those classes to use
      > @generateCppClass. One field was removed because it's unused
      > (PrototypeInfo::validity_cell), and two fields in StackFrameInfo
      > actually became less precise because they're based on Script::name,
      > which is an embedder-provided untyped Local<Value>. (Automatically
      > generated accessors pointed out this bug easily.)
      > 
      > This change also includes a couple of minor fixes in Torque.
      > 
      > Change-Id: Ib2bc6c7165bb3612b6d344c0686a94165a568277
      > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2199640
      > Commit-Queue: Seth Brenith <seth.brenith@microsoft.com>
      > Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
      > Reviewed-by: Toon Verwaest <verwaest@chromium.org>
      > Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
      > Cr-Commit-Position: refs/heads/master@{#67907}
      
      TBR=ulan@chromium.org,tebbi@chromium.org,verwaest@chromium.org,seth.brenith@microsoft.com
      
      Change-Id: I720821d8dc84ea0d79eb137f1c2507f75df9a107
      No-Presubmit: true
      No-Tree-Checks: true
      No-Try: true
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2211322Reviewed-by: 's avatarUlan Degenbaev <ulan@chromium.org>
      Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#67972}
      16cb2d94
  7. 19 May, 2020 1 commit
  8. 13 May, 2020 1 commit
  9. 11 Nov, 2019 1 commit