Optimize Source renderer scroll performance [DRAFT]

[bvaughn]

Final check list:

• Smoke test
• Make sure all of our e2e tests pass
• Make sure all of the screenshot tests pass

How did the React components change?

• The experimental column breakpoint UI has been removed. It was complicating a lot of things (search result highlights, execution point marker, syntax highlighting) and it wasn't clearly adding much value to offset this. (Jason and I spoke about this ahead of time.)
• Arguably the most expensive rendering bit– the syntax highlighted tokens– have been moved into a sub component so that they can be memoized after initially rendering. Changes like mouse over highlights will no longer cause this component to be re-rendered.
• Fewer ReactElements (and DOM nodes) are being created per row.
• Memoization has been removed from itemData since "hot" actions (like scroll position, cursor position) were frequently invalidating the memoized values anyway.
• Current line highlights (and the alternate source mapping that goes along with them) have been lifted out of the row rendering components and into the main SourceList.
• Mouse interactions (e.g. token hover, add-log-point button) are ignored while the user is scrolling (to reduce the overall amount of work we have to do during this performance-sensitive time).

How has the DOM changed?

The React components have changed a lot (hopefully in a way that simplifies them and makes them more performant) but the underlying DOM also changes (to become flatter) with this PR. Hopefully this simplifies positioning and helps the browser render/layout a bit faster. Here's a before/after for a single row (the same row)

HTML before

<div class="SourceListRow_Row__UOalW" data-test-hit-counts-state="loaded" data-test-contents-state="parsed" data-test-id="SourceLine-2840" data-test-name="SourceLine" style="...">
  <div class="SourceListRow_LineWithHits__e7G7S">
    <div class="SourceListRow_LineNumber__TbrqD" data-test-id="SourceLine-LineNumber-2840">
      <span data-test-name="SourceRowLineNumber">2840</span>
      <div class="SourceListRow_BreakpointToggle__vCI_n" data-test-name="BreakpointToggle" data-test-state="off">2840</div>
    </div>
    <div class="SourceListRow_LineHitCountBar__7SxRJ SourceListRow_LineHitCountBar3__fGHGC"></div>
    <div class="SourceListRow_LineHitCountLabel__zYkzG SourceListRow_LineHitCountLabel3__nKp99">20k</div>
    <div class="SourceListRow_LineSegmentsAndPointPanel__IoUvD" data-test-name="SourceLine-Contents">
      <pre class="SourceListRow_LineSegment__XUkZ_">
        <span class="" data-column-index="0">  </span>
        <span class="tok-variableName" data-column-index="2" data-inspectable-token="true">a</span>
        <span class="" data-column-index="3"> </span>
        <span class="tok-operator" data-column-index="4">=</span>
        <span class="" data-column-index="5"> </span>
        <span class="tok-variableName" data-column-index="6" data-inspectable-token="true">c</span>
        <span class="tok-operator" data-column-index="7">.</span>
        <span class="tok-propertyName" data-column-index="8" data-inspectable-token="true">ref</span>
        <span class="tok-punctuation" data-column-index="11">;</span>
      </pre>
      <div class="SourceListRow_HoverButtonCompanion__L_PY9"></div>
    </div>
  </div>
</div>

HTML after

<div class="SourceListRow_Row__UOalW" data-test-line-number="2840" data-test-id="SourceLine-2840" data-test-name="SourceLine" style="...">
  <div class="SourceListRow_LineNumber__TbrqD" data-test-name="SourceLine-LineNumber">2840</div>
  <div class="SourceListRow_LineHitCount3__aE7dN" data-test-name="SourceLine-HitCount">20k</div>
  <span data-column-index="0"> </span>
  <span class="tok-variableName" data-column-index="2" data-inspectable-token="true">a</span>
  <span data-column-index="3"> </span>
  <span class="tok-operator" data-column-index="4">=</span>
  <span data-column-index="5"> </span>
  <span class="tok-variableName" data-column-index="6" data-inspectable-token="true">c</span>
  <span class="tok-operator" data-column-index="7">.</span>
  <span class="tok-propertyName" data-column-index="8" data-inspectable-token="true">ref</span>
  <span class="tok-punctuation" data-column-index="11">;</span>
</div>

Is the new Source viewer faster?

I think so, although it's difficult to measure this sort of stuff.

Scrolling before

Rough average scroll handler duration ~12ms maybe?

Scrolling after

Rough average scroll handler duration ~5ms maybe?

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
devtools	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Aug 21, 2023 4:55pm

[replay-io]

E2E Tests

72 replays were recorded for edd0785.

1 Failed

• react_devtools 02: RDT integrations (Chromium)

  locator.textContent: Error: strict mode violation: locator('[data-test-id^=SourceLine]').filter({ has: locator('[data-test-name="ViewSourceHighlight"]').or(locator('[data-testname="ViewSourceHighlight"]')) }).locator('[data-test-name="SourceLine-LineNumber"]').or(locator('[data-test-id^=SourceLine]').filter({ has: locator('[data-test-name="ViewSourceHighlight"]').or(locator('[data-testname="ViewSourceHighlight"]')) }).locator('[data-testname="SourceLine-LineNumber"]')) resolved to 2 elements:
      1) <div data-test-name="SourceLine-LineNumber" class="So…>280</div> aka getByText('280')
      2) <div data-test-name="SourceLine-LineNumber" class="So…>135</div> aka getByText('135')
  =========================== logs ===========================
  waiting for locator('[data-test-id^=SourceLine]').filter({ has: locator('[data-test-name="ViewSourceHighlight"]').or(locator('[data-testname="ViewSourceHighlight"]')) }).locator('[data-test-name="SourceLine-LineNumber"]').or(locator('[data-test-id^=SourceLine]').filter({ has: locator('[data-test-name="ViewSourceHighlight"]').or(locator('[data-testname="ViewSourceHighlight"]')) }).locator('[data-testname="SourceLine-LineNumber"]'))
  ============================================================

71 Passed

• authenticated/test-suites/library-test-runs
• authenticated/test-suites/library-tests-list
• breakpoints-01: Test basic breakpoint functionality
• breakpoints-02: Test unhandled divergence while evaluating at a breakpoint
• breakpoints-03: Test stepping forward through breakpoints when rewound before the first one
• breakpoints-04: catch, finally, generators, and async/await
• breakpoints-05: Test interaction of breakpoints with debugger statements
• breakpoints-06: Test log point in a sourcemapped file
• breakpoints-07: rewind and seek using command bar and console messages
• breakpoints-08: should be temporarily disabled
• console_async: support console evaluations in async frames
• console_dock_node: Should show the correct docking behavior for recordings without video
• console_dock: Should show the correct docking behavior for recordings with video
• console_errors: Test that errors and warnings from various sources are shown in the console
• console_eval: support console evaluations
• console_warp-01: should support warping to console messages
• console_warp-02: support pausing, warping, stepping and evaluating console messages
• console-expressions-01: should cache input eager eval and terminal expressions per instance
• cypress-01: Basic Test Suites panel functionality
• cypress-02: Test Step timeline behavior
• cypress-03: Test Step interactions
• cypress-04: Test Step buttons and menu item
• focus_mode-01: should filter messages as regions based on the active focus mode
• highlighter: element highlighter works everywhere
• inspector-01: Test that scopes are rerendered
• inspector-02: element picker and iframe behavior
• inspector-03: Test that styles for elements can be viewed
• inspector-04: Test that styles for elements can be viewed
• inspector-05: Test that styles for elements can be viewed
• inspector-06: Test that styles for elements can be viewed
• inspector-07: Test that styles for elements can be viewed
• logpoints-01: log-points appear in the correct order and allow time warping
• logpoints-02: conditional log-points
• logpoints-03_chromium: should display event properties in the console
• logpoints-03: should display event properties in the console
• logpoints-04: should display exceptions in the console
• logpoints-05: should auto-complete based on log point location
• logpoints-06: should be temporarily disabled
• logpoints-07: should use the correct scope in auto-complete
• logpoints-08: should support jumping directly to a hit point via the capsule input
• logpoints-09: should support pending edits
• node_console-01: Basic node console behavior
• node_console-02: uncaught exceptions should show up
• node_control_flow: catch, finally, generators, and async/await
• node_logpoint-01: Basic node logpoints
• node_logpoint-02: Node exception logpoints
• node_object_preview: Showing console objects in node
• node_spawn: Basic subprocess spawning
• node_stepping-01: Test stepping in async frames and async call stacks
• node_worker-01: make sure node workers don't cause crashes
• object_preview-01: expressions in the console after time warping
• object_preview-02: should allow objects in scope to be inspected
• object_preview-03: Test previews when switching between frames and stepping
• object_preview-04: Test scope mapping and switching between generated/original sources
• object_preview-05: Should support logging objects as values
• quick_open_modal-01: Test basic searching functionality
• react_devtools 01: Basic RDT behavior (Chromium)
• react_devtools 01: Basic RDT behavior (FF)
• redux_devtools: Test Redux DevTools.
• repaint: repaints the screen screen when stepping over code that modifies the DOM
• resizable-panels-01: Left side Toolbar should be collapsible
• scopes_rerender: Test that scopes are rerendered
• sourcemap_stacktrace: Test that stacktraces are sourcemapped
• stacking: Element highlighter selects the correct element when they overlap
• stepping-01: Test basic step-over/back functionality
• stepping-02: Test fixes for some simple stepping bugs
• stepping-03: Stepping past the beginning or end of a frame should act like a step-out
• stepping-04: Test stepping in a frame other than the top frame
• stepping-05_chromium: Test stepping in pretty-printed code
• stepping-05: Test stepping in pretty-printed code
• stepping-06: Test stepping in async frames and async call stacks

View test run on Replay ↗︎

Snapshot Tests

100 replays were recorded for edd0785.

0 Failed

100 Passed

• should add true/false keywords to the list of suggestions
• should add this keyword to the list of suggestions
• should be able to toggle side filter menu open and closed
• should be filterable on complex content
• should be filterable
• should be searchable on complex content
• should be searchable
• should display list of messages
• should display toggleable stack for errors
• should display toggleable stack for exceptions
• should display toggleable stack for traces
• should display toggleable stack for warnings
• should escape object expressions in terminal expressions
• should evaluate and render invalid terminal expressions
• should evaluate terminal expressions at an execution point
• should evaluate terminal expressions without an execution point
• should expand and inspect arrays
• should expand and inspect objects
• should hide node_modules (and unpkg) if toggled
• should log events in the console
• should re-focus the search input when CMD+F is used again
• should re-focus the terminal input when search input is hidden
• should refine string tokens to avoid unnecessarily escaping quotation marks
• should remember filter toggle preferences between reloads
• should show a button to clear terminal expressions
• should show error object stacks by default
• should show the context menu on top of other messages and the current time indicator
• should suggest type-ahead options from prototype objects as well
• should support seeking to a message execution point
• should support setting focus range via the context menu
• should support terminal expressions with line breaks
• should render nested, recursive values (as ellipses) without crashing
• should copy arrays
• should copy dates
• should copy deep arrays
• should copy deep objects and their nested properties
• should copy errors
• should copy functions
• should copy html elements and text
• should copy maps
• should copy objects
• should copy primitive client values
• should copy regular expressions
• should copy sets
• should properly bucket properties for an overflowing array
• should render and inspect a Promise
• should render and inspect a Proxy
• should render and inspect arrays
• should render and inspect functions
• should render and inspect HTML elements
• should render and inspect maps
• should render and inspect objects
• should render and inspect regular expressions
• should render and inspect sets
• should render dates
• should render errors
• should render getters and setters correctly
• should render simple values
• should render sections correctly
• should show/hide object and array previews when toggled
• scroll position should be restored when switching between sources
• should allow log point badge colors to be toggled
• should auto-suggestion text based on the current expression
• should disable syntax highlight for long lines
• should expand and contract line height when log points are added and removed
• should gracefully handle invalid remote analysis
• should handle too many points to find
• should handle too many points to run analysis
• should hide results when search is closed, and remember previous search string if re-opened
• should include log points in search results
• should include log points when filtering data
• should not allow saving log points with invalid conditional
• should not allow saving log points with invalid content
• should not erase break point when logging is toggled
• should not erase log point content when breaking is toggled
• should overflow long log point text (without wrapping)
• should remember search results count per source
• should render search results properly for lines with multiple breakable column positions
• should run local analysis for log points
• should run remote analysis for log points
• should scroll to line and column
• should show different background color and edit icon when log point disabled
• should show preview popup for instance properties
• should support break points
• should support conditional log points
• should support continue to next and previous functionality
• should support custom badge styles for log points
• should support fuzzy file search
• should support jumping to a line in the active source file
• should support new lines in log points
• should support text search in the active source file
• should update the current time when the next/previous log point buttons are clicked
• should render headers for complex data correctly
• should render primitive data correctly
• display an overflow message
• should expand and collapse sources
• should find matches in multiple sources
• should include or exclude external modules as requested
• should open a source and scroll to the correct line
• should reset collapsed state if search changes

View test run on Replay ↗︎

[bvaughn]

Couple of Delta tests failed. Probably helper methods need to be updated to account for markup changes. I'll pick those up Monday morning. I think this is ready to be reviewed, otherwise.

[markerikson]

No real feedback :) You've obviously put a lot of thought into this, and as usual the changes make sense at first glance based on the description.

[bvaughn]

Thanks Mark!
The only test failing on this branch is react devtools 02 which is failing frequently on main, so I'm going to land this.