• 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
isolate.cc 182 KB