Skip to content

test(shared-tree): Add memory usage performance benchmarks for SharedTree table APIs #24900

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

chentong7
Copy link
Contributor

Description

This PR introduces performance benchmark tests for SharedTree's table-related APIs, focusing on standard operations. The benchmarks cover:

Insertion of rows, columns, and cells

Includes insertion at the middle of the table for rows and columns. Similarly, tests removal for rows and columns.

Test Result

/home/tong/workspace/FluidFramework/node_modules/.bin/mocha ./tableTree.spec.js --no-timeouts --exit
Writing test results relative to package to nyc/junit-report.xml
mocharc-common.cjs:73
Warning: Cannot find any files matching pattern "lib/test"
collect-files.js:102

spec.js:54
  SharedTree memory usage
spec.js:54
    Size of 10*10 SharedTree
spec.js:54
      Column Insertion
spec.js:54
        ✔ @Benchmark @Measurement @MemoryUsage Insert a column in the middle 5 times (149ms)
spec.js:83
        ✔ @Benchmark @Measurement @MemoryUsage Undo insert column in the middle 5 times (96ms)
spec.js:83
        ✔ @Benchmark @Measurement @MemoryUsage Redo insert column in the middle 5 times (89ms)
spec.js:83
      Column Insertion
spec.js:54
        ✔ @Benchmark @Measurement @MemoryUsage Insert a column in the middle 5 times (77ms)
spec.js:83
        ✔ @Benchmark @Measurement @MemoryUsage Undo insert column in the middle 5 times (78ms)
spec.js:83
        ✔ @Benchmark @Measurement @MemoryUsage Redo insert column in the middle 5 times (82ms)
spec.js:83
      Column and Row Insertion
spec.js:54
        ✔ @Benchmark @Measurement @MemoryUsage Insert a column and a row in the middle 5 times (74ms)
spec.js:83
        ✔ @Benchmark @Measurement @MemoryUsage Undo insert column and row in the middle 5 times (73ms)
spec.js:83
        ✔ @Benchmark @Measurement @MemoryUsage Redo insert column and row in the middle 5 times (77ms)
spec.js:83
      Column Removal
spec.js:54
        ✔ @Benchmark @Measurement @MemoryUsage Remove a column in the middle 5 times (88ms)
spec.js:83
        ✔ @Benchmark @Measurement @MemoryUsage Undo remove column in the middle 5 times (96ms)
spec.js:83
        ✔ @Benchmark @Measurement @MemoryUsage Redo remove column in the middle 5 times (91ms)
spec.js:83
      Row Removal
spec.js:54
        ✔ @Benchmark @Measurement @MemoryUsage Remove a row in the middle 5 times (61ms)
spec.js:83
        ✔ @Benchmark @Measurement @MemoryUsage Undo remove row in the middle 5 times (63ms)
spec.js:83
        ✔ @Benchmark @Measurement @MemoryUsage Redo remove row in the middle 5 times (63ms)
spec.js:83
      Column and Row Removal
spec.js:54
        ✔ @Benchmark @Measurement @MemoryUsage Remove a column and a row in the middle 5 times (77ms)
spec.js:83
        ✔ @Benchmark @Measurement @MemoryUsage Undo remove column and row in the middle 5 times (78ms)
spec.js:83
        ✔ @Benchmark @Measurement @MemoryUsage Redo remove column and row in the middle 5 times (78ms)
spec.js:83
      Insert a column and a row and remove right away
spec.js:54
        ✔ @Benchmark @Measurement @MemoryUsage Insert a column and a row in the middle and remove right away 5 times (77ms)
spec.js:83
        ✔ @Benchmark @Measurement @MemoryUsage Undo insert and remove column and row in the middle 5 times (85ms)
spec.js:83
        ✔ @Benchmark @Measurement @MemoryUsage Redo insert and remove column and row in the middle 5 times (83ms)
spec.js:83
      Cell Value Setting
spec.js:54
        ✔ @Benchmark @Measurement @MemoryUsage Set cell value in the middle 5 times (54ms)
spec.js:83
        ✔ @Benchmark @Measurement @MemoryUsage Undo set cell value in the middle 5 times (59ms)
spec.js:83
        ✔ @Benchmark @Measurement @MemoryUsage Redo set cell value in the middle 5 times (95ms)
spec.js:83
  24 passing (2s)

@Copilot Copilot AI review requested due to automatic review settings June 24, 2025 00:28
@chentong7 chentong7 requested review from a team as code owners June 24, 2025 00:28
@github-actions github-actions bot added area: dds Issues related to distributed data structures area: dds: tree labels Jun 24, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces performance benchmark tests for SharedTree’s table APIs and aligns performance documentation between SharedTree and SharedMatrix. Key changes include:

  • New performance test utilities for table operations (insertions, removals, undo/redo) in SharedTree.
  • Added documentation notes in the SharedMatrix benchmarks to ensure consistency with SharedTree.

Reviewed Changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.

File Description
packages/dds/tree/src/test/tablePerformanceTestUtilities.ts Adds table schema definitions and utility functions for performance benchmarking.
packages/dds/matrix/src/test/memory/sharedMatrix.spec.ts Updates benchmark documentation to reference SharedTree benchmarks for consistency.

@github-actions github-actions bot added the base: main PRs targeted against main branch label Jun 24, 2025
@chentong7 chentong7 requested a review from a team June 24, 2025 00:40
Copy link
Contributor

@jzaffiro jzaffiro left a comment

Choose a reason for hiding this comment

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

approved for matrix changes!

Copy link
Contributor

@Josmithr Josmithr left a comment

Choose a reason for hiding this comment

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

Left one more style comment. Otherwise, looking good! Happy we were able to make these tests look more similar to the runtime ones 🙂

Copy link
Contributor

@Josmithr Josmithr left a comment

Choose a reason for hiding this comment

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

Code changes look good, but could you include the memory usage metrics in the PR description? It currently shows the list of tests passing, but not the reported metrics.

We should also probably hold off on merging this until we've sorted out the CI time issues we discovered with the other PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: dds: tree area: dds Issues related to distributed data structures base: main PRs targeted against main branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants