Skip to content

test(grid): add grid scroll performance benchmarks - #2746

Open
mofojed wants to merge 4 commits into
deephaven:mainfrom
mofojed:grid-perf-benchmark-tooling
Open

test(grid): add grid scroll performance benchmarks#2746
mofojed wants to merge 4 commits into
deephaven:mainfrom
mofojed:grid-perf-benchmark-tooling

Conversation

@mofojed

@mofojed mofojed commented Aug 28, 2026

Copy link
Copy Markdown
Member
  • Adds two Playwright benchmark suites, both skipped unless RUN_PERF_TESTS is set since frame timings are too resource sensitive for CI.
  • grid-performance.spec.ts measures scroll FPS in the main app against real tables from a Deephaven server.
  • grid-perf-app.spec.ts drives a standalone Vite app backed by MockGridModel, so the grid can be benchmarked without a server and at row and column counts the test data does not reach.
  • Will be used as a baseline to measure performance of Grid changes

Adds two Playwright benchmark suites, both skipped unless RUN_PERF_TESTS
is set since frame timings are too resource sensitive for CI.

grid-performance.spec.ts measures scroll FPS in the main app against real
tables from a Deephaven server.

grid-perf-app.spec.ts drives a standalone Vite app backed by MockGridModel,
so the grid can be benchmarked without a server and at row and column counts
the test data does not reach.
@mofojed
mofojed requested a balanced review from Copilot August 28, 2026 17:53
@mofojed mofojed self-assigned this Aug 28, 2026
@codecov

codecov Bot commented Aug 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 52.04%. Comparing base (535936a) to head (3b45e28).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2746      +/-   ##
==========================================
+ Coverage   51.80%   52.04%   +0.23%     
==========================================
  Files         808      808              
  Lines       46321    46321              
  Branches    11849    12037     +188     
==========================================
+ Hits        23996    24107     +111     
+ Misses      22305    22167     -138     
- Partials       20       47      +27     
Flag Coverage Δ
unit 52.04% <ø> (+0.23%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds opt-in Playwright benchmarks for Grid scrolling performance in the main application and a standalone mock-data app.

Changes:

  • Adds real-table and standalone Grid FPS benchmarks.
  • Adds a configurable Vite performance app using MockGridModel.
  • Documents and exposes benchmark commands.

Reviewed changes

Copilot reviewed 11 out of 12 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
tests/grid-performance.spec.ts Adds main-app scroll benchmarks.
tests/grid-perf-app.spec.ts Adds standalone Grid benchmarks.
tests/grid-perf-app/src/App.tsx Renders a configurable mock Grid.
tests/grid-perf-app/src/main.tsx Initializes the React app.
tests/grid-perf-app/vite.config.ts Configures the performance server.
tests/grid-perf-app/tsconfig.json Configures TypeScript.
tests/grid-perf-app/package.json Defines app dependencies and scripts.
tests/grid-perf-app/package-lock.json Locks app dependencies.
tests/grid-perf-app/index.html Provides the app shell and sizing.
tests/grid-perf-app/.gitignore Excludes generated artifacts.
README.md Documents benchmark workflows.
package.json Adds benchmark commands.
Files not reviewed (1)
  • tests/grid-perf-app/package-lock.json: Generated file
Suppressed comments (5)

tests/grid-performance.spec.ts:181

  • all_types is created with only 20 rows (tests/docker-scripts/data/app.d/common_tables.py:4,27), roughly 380px at the Iris Grid row height, so it has no vertical overflow in the normal E2E viewport. Both tests in this block send only vertical wheel deltas, meaning the Grid view does not change and the measured FPS is effectively idle-page FPS. Use a fixture with enough rows or scroll the overflowing column axis.
      // all_types is a table with many different column types
      await openTable(page, 'all_types');

tests/grid-performance.spec.ts:203

  • This benchmark never positions the mouse over the Grid. openTable leaves it at the Panels menu click, while Grid registers its non-passive wheel handler directly on the canvas, so these events can target unrelated page content and measure no Grid work. Hover the canvas before starting the measurement.
      for (let i = 0; i < 50; i += 1) {
        await page.mouse.wheel(0, 200);

tests/grid-performance.spec.ts:239

  • The mouse is still at the control used by openTable, not explicitly over the Grid canvas. Because the wheel listener is attached directly to the canvas, this loop can run for three seconds without scrolling or redrawing the Grid. Hover the canvas before starting timing.
    while (Date.now() - startTime < duration) {
      await page.mouse.wheel(0, 300 * direction);

tests/grid-performance.spec.ts:244

  • Using Math.random() changes the scroll-direction sequence and how long the grid sits at an edge on every run, adding avoidable variance to a performance baseline. Reverse at a deterministic event interval (or use a seeded sequence) so results from two revisions represent the same workload.
      // Reverse direction occasionally
      if (Math.random() < 0.1) {
        direction *= -1;

tests/grid-perf-app.spec.ts:196

  • Using Math.random() gives each benchmark run a different workload and can change how much time is spent at the top boundary, adding avoidable variance to the baseline. Reverse at a deterministic event interval (or use a seeded sequence) so revisions are compared with identical input.
      if (Math.random() < 0.1) {
        direction *= -1;
      }

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread tests/grid-perf-app/vite.config.ts
Comment thread tests/grid-performance.spec.ts Outdated
Comment thread tests/grid-perf-app.spec.ts Outdated
Comment thread package.json Outdated
Adds two Playwright benchmark suites, both skipped unless RUN_PERF_TESTS
is set since frame timings are too resource sensitive for CI.

grid-performance.spec.ts measures scroll FPS in the main app against real
tables from a Deephaven server.

grid-perf-app.spec.ts drives a standalone Vite app backed by MockGridModel,
so the grid can be benchmarked without a server and at row and column counts
the test data does not reach.
Copilot AI review requested due to automatic review settings August 31, 2026 20:10

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 12 out of 13 changed files in this pull request and generated 1 comment.

Files not reviewed (1)
  • tests/grid-perf-app/package-lock.json: Generated file
Suppressed comments (1)

tests/grid-performance.spec.ts:257

  • The random reversals make this baseline non-repeatable. Because simple_table has only 100 rows, one random sequence may spend much of the three seconds pinned at an edge (where wheel events do not redraw), while another keeps moving; the resulting FPS difference can come from the workload rather than a Grid change. Use a fixed or seeded reversal schedule.
      // Reverse direction occasionally
      if (Math.random() < 0.1) {
        direction *= -1;
      }

Comment thread tests/grid-performance.spec.ts Outdated
test.describe('all_types table performance', () => {
test.beforeEach(async ({ page }) => {
// all_types is a table with many different column types
await openTable(page, 'all_types');
- Add a big all_types table that can be scrolled
Copilot AI review requested due to automatic review settings August 31, 2026 20:42

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 14 out of 15 changed files in this pull request and generated 1 comment.

Files not reviewed (1)
  • tests/grid-perf-app/package-lock.json: Generated file
Suppressed comments (6)

Previously missed (4) — in code that hasn't changed since the last review.

tests/grid-performance.spec.ts:168

  • Browser-side failures are not observed here, so a grid exception or console error can leave the page's requestAnimationFrame loop running and produce a successful-looking FPS result. Install the same error listeners used by tests/table-scroll.spec.ts:46-54 before navigating so a broken benchmark fails instead of reporting idle-page FPS.
  test.beforeEach(async ({ page }) => {
    await gotoPage(page, '');
  });

tests/grid-perf-app.spec.ts:139

  • Browser errors do not automatically fail Playwright tests, so this benchmark can keep measuring the page's animation loop after the standalone Grid has crashed. Add error listeners before each test so logged timings are only accepted from a functioning grid.

This issue also appears on line 194 of the same file.

  test.describe.configure({ mode: 'serial' });

tests/grid-performance.spec.ts:213

  • These wheel events are sent at the mouse position left by openTable (the Panels menu item), unlike scrollGrid, which explicitly moves to the grid. Depending on what occupies that coordinate after the menu closes, this test can measure an idle page without scrolling the Grid; hover the target grid before starting the measurement.

This issue also appears on line 253 of the same file.

        await page.mouse.wheel(0, 200);

tests/grid-performance.spec.ts:260

  • Using random direction changes makes the amount and location of scrolling different on every run, so before/after FPS results do not use a repeatable workload and cannot serve as a reliable baseline. Reverse after a fixed number of wheel events instead.
      if (Math.random() < 0.1) {

tests/grid-performance.spec.ts:253

  • This test also sends wheel events without first targeting the newly opened grid, so the events remain at the Panels menu click coordinate and may not reach the canvas wheel listener. Hover the grid before measuring to ensure this is sustained Grid scrolling rather than idle-page FPS.
      await page.mouse.wheel(0, 300 * direction);

tests/grid-perf-app.spec.ts:194

  • Random direction changes make each benchmark run execute a different scroll path, which adds workload variance to the FPS comparison. Use a fixed reversal cadence so this suite provides a reproducible baseline.
      if (Math.random() < 0.1) {

"Short=(short)(i%19==0 ? null : (int)(scale*(i*2-1)))",
"BigDec=(i%21==0 ? null : new java.math.BigDecimal(scale*(i*2-1)))",
"BigInt=(i%22==0 ? null : new java.math.BigInteger(Integer.toString((int)(scale*(i*2-1)))))",
"Byte=(Byte)(i%19==0 ? null : new Byte( Integer.toString((int)(i))))",
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants