Skip to content

test(shared-tree): Add execution time performance benchmarks for SharedTree table APIs #24881

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

Merged
merged 1 commit into from
Jun 27, 2025

Conversation

chentong7
Copy link
Contributor

@chentong7 chentong7 commented Jun 20, 2025

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 execution time
spec.js:54
    Column Insertion
spec.js:54
      ✔ @Benchmark @Measurement @ExecutionTime Insert a column in the middle 5 times (135ms)
spec.js:83
      ✔ @Benchmark @Measurement @ExecutionTime Undo insert the middle column 5 times (142ms)
spec.js:83
      ✔ @Benchmark @Measurement @ExecutionTime Redo insert the middle column 5 times (121ms)
spec.js:83
    Row Insertion
spec.js:54
      ✔ @Benchmark @Measurement @ExecutionTime Insert a row in the middle 5 times (93ms)
spec.js:83
      ✔ @Benchmark @Measurement @ExecutionTime Undo insert the middle row 5 times (94ms)
spec.js:83
      ✔ @Benchmark @Measurement @ExecutionTime Redo insert the middle row 5 times (94ms)
spec.js:83
    Column and Row Insertion
spec.js:54
      ✔ @Benchmark @Measurement @ExecutionTime Insert a column and a row in the middle 5 times (88ms)
spec.js:83
      ✔ @Benchmark @Measurement @ExecutionTime Undo insert the middle column and row 5 times (97ms)
spec.js:83
      ✔ @Benchmark @Measurement @ExecutionTime Redo insert the middle column and row 5 times (110ms)
spec.js:83
    Column Removal
spec.js:54
      ✔ @Benchmark @Measurement @ExecutionTime Remove a column in the middle 5 times (105ms)
spec.js:83
      ✔ @Benchmark @Measurement @ExecutionTime Undo remove the middle column 5 times (127ms)
spec.js:83
      ✔ @Benchmark @Measurement @ExecutionTime Redo remove the middle column 5 times (120ms)
spec.js:83
    Row Removal
spec.js:54
      ✔ @Benchmark @Measurement @ExecutionTime Remove a row in the middle 5 times (102ms)
spec.js:83
      ✔ @Benchmark @Measurement @ExecutionTime Undo remove the middle row 5 times (83ms)
spec.js:83
      ✔ @Benchmark @Measurement @ExecutionTime Redo remove the middle row 5 times (94ms)
spec.js:83
    Column and Row Removal
spec.js:54
      ✔ @Benchmark @Measurement @ExecutionTime Remove a column and a row in the middle 5 times (99ms)
spec.js:83
      ✔ @Benchmark @Measurement @ExecutionTime Undo remove the middle column and row 5 times (122ms)
spec.js:83
      ✔ @Benchmark @Measurement @ExecutionTime Redo remove the middle column and row 5 times (129ms)
spec.js:83
    Insert a column and a row and remove right away
spec.js:54
      ✔ @Benchmark @Measurement @ExecutionTime Insert a column and a row and remove them right away 5 times (93ms)
spec.js:83
      ✔ @Benchmark @Measurement @ExecutionTime Undo insert a column and a row and remove them right away 5 times (119ms)
spec.js:83
      ✔ @Benchmark @Measurement @ExecutionTime Redo insert a column and a row and remove them right away 5 times (132ms)
spec.js:83
    Cell Value Setting
spec.js:54
      ✔ @Benchmark @Measurement @ExecutionTime Set a cell value 5 times (87ms)
spec.js:83
      ✔ @Benchmark @Measurement @ExecutionTime Undo set a cell value 5 times (82ms)
spec.js:83
      ✔ @Benchmark @Measurement @ExecutionTime Redo set a cell value 5 times (87ms)
spec.js:83
  24 passing (3s)

@Copilot Copilot AI review requested due to automatic review settings June 20, 2025 01:47
@chentong7 chentong7 requested a review from a team as a code owner June 20, 2025 01:47
@github-actions github-actions bot added base: main PRs targeted against main branch area: dds Issues related to distributed data structures area: dds: tree labels Jun 20, 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-related APIs that measure execution times for various table operations such as insertion and removal.

  • Added benchmark tests for column, row, and combined insertion and removal operations.
  • Updated Mocha configuration to filter performance-relevant tests and included helper functions for table tree initialization.

Reviewed Changes

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

File Description
packages/dds/tree/src/test/time/tableTree.spec.ts Adds performance benchmark tests for table operations.
packages/dds/tree/src/test/time/.mocharc.cjs Configures Mocha for execution time tests.
packages/dds/tree/src/test/testHelper.ts Provides utility functions and type definitions for table trees.

@chentong7 chentong7 requested a review from a team June 20, 2025 20:46
@github-actions github-actions bot added public api change Changes to a public API and removed public api change Changes to a public API labels Jun 20, 2025
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 a couple more comments, but overall looking very good! Thanks!!

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.

matrix changes look fine!

@github-actions github-actions bot added the public api change Changes to a public API label Jun 27, 2025
@github-actions github-actions bot removed the public api change Changes to a public API label Jun 27, 2025
Copy link
Contributor

🔗 No broken links found! ✅

Your attention to detail is admirable.

linkcheck output


> fluid-framework-docs-site@0.0.0 ci:check-links /home/runner/work/FluidFramework/FluidFramework/docs
> start-server-and-test "npm run serve -- --no-open" 3000 check-links

1: starting server using command "npm run serve -- --no-open"
and when url "[ 'http://127.0.0.1:3000' ]" is responding with HTTP status code 200
running tests using command "npm run check-links"


> fluid-framework-docs-site@0.0.0 serve
> docusaurus serve --no-open

[SUCCESS] Serving "build" directory at: http://localhost:3000/

> fluid-framework-docs-site@0.0.0 check-links
> linkcheck http://localhost:3000 --skip-file skipped-urls.txt

Crawling...

Stats:
  231978 links
    1733 destination URLs
    1964 URLs ignored
       0 warnings
       0 errors


table.removeRow(row);
}
for (let i = 0; i < count; i++) {
undoRedoManager.undo();
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: I liked the comments you added in the test above explaining what each undo call is doing. Would you mind adding them here as well (and anywhere else we do multiple undo/redos)? 😁

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice. Added all comments there.

Copy link
Contributor

Choose a reason for hiding this comment

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

@chentong7 did you push the changes before merging? I don't see them here.

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 docs suggestion. Otherwise looks great! Excited to see the comparison between Tree and Matrix!

@chentong7 chentong7 merged commit ff18a8c into microsoft:main Jun 27, 2025
49 checks passed
alexvy86 added a commit that referenced this pull request Jun 30, 2025
…for SharedTree table APIs" (#24940)

Reverts #24881. The perf benchmarks pipeline
started taking a very long time to run SharedTree benchmarks after it,
so opening this revert to have it ready in case we need to merge it to
unblock the pipeline.
MarioJGMsoft pushed a commit to MarioJGMsoft/FluidFramework that referenced this pull request Jul 8, 2025
…edTree table APIs (microsoft#24881)

## 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 execution time
spec.js:54
    Column Insertion
spec.js:54
✔ @benchmark @measurement @ExecutionTime Insert a column in the middle 5
times (135ms)
spec.js:83
✔ @benchmark @measurement @ExecutionTime Undo insert the middle column 5
times (142ms)
spec.js:83
✔ @benchmark @measurement @ExecutionTime Redo insert the middle column 5
times (121ms)
spec.js:83
    Row Insertion
spec.js:54
✔ @benchmark @measurement @ExecutionTime Insert a row in the middle 5
times (93ms)
spec.js:83
✔ @benchmark @measurement @ExecutionTime Undo insert the middle row 5
times (94ms)
spec.js:83
✔ @benchmark @measurement @ExecutionTime Redo insert the middle row 5
times (94ms)
spec.js:83
    Column and Row Insertion
spec.js:54
✔ @benchmark @measurement @ExecutionTime Insert a column and a row in
the middle 5 times (88ms)
spec.js:83
✔ @benchmark @measurement @ExecutionTime Undo insert the middle column
and row 5 times (97ms)
spec.js:83
✔ @benchmark @measurement @ExecutionTime Redo insert the middle column
and row 5 times (110ms)
spec.js:83
    Column Removal
spec.js:54
✔ @benchmark @measurement @ExecutionTime Remove a column in the middle 5
times (105ms)
spec.js:83
✔ @benchmark @measurement @ExecutionTime Undo remove the middle column 5
times (127ms)
spec.js:83
✔ @benchmark @measurement @ExecutionTime Redo remove the middle column 5
times (120ms)
spec.js:83
    Row Removal
spec.js:54
✔ @benchmark @measurement @ExecutionTime Remove a row in the middle 5
times (102ms)
spec.js:83
✔ @benchmark @measurement @ExecutionTime Undo remove the middle row 5
times (83ms)
spec.js:83
✔ @benchmark @measurement @ExecutionTime Redo remove the middle row 5
times (94ms)
spec.js:83
    Column and Row Removal
spec.js:54
✔ @benchmark @measurement @ExecutionTime Remove a column and a row in
the middle 5 times (99ms)
spec.js:83
✔ @benchmark @measurement @ExecutionTime Undo remove the middle column
and row 5 times (122ms)
spec.js:83
✔ @benchmark @measurement @ExecutionTime Redo remove the middle column
and row 5 times (129ms)
spec.js:83
    Insert a column and a row and remove right away
spec.js:54
✔ @benchmark @measurement @ExecutionTime Insert a column and a row and
remove them right away 5 times (93ms)
spec.js:83
✔ @benchmark @measurement @ExecutionTime Undo insert a column and a row
and remove them right away 5 times (119ms)
spec.js:83
✔ @benchmark @measurement @ExecutionTime Redo insert a column and a row
and remove them right away 5 times (132ms)
spec.js:83
    Cell Value Setting
spec.js:54
✔ @benchmark @measurement @ExecutionTime Set a cell value 5 times (87ms)
spec.js:83
✔ @benchmark @measurement @ExecutionTime Undo set a cell value 5 times
(82ms)
spec.js:83
✔ @benchmark @measurement @ExecutionTime Redo set a cell value 5 times
(87ms)
spec.js:83
  24 passing (3s)
~~~
MarioJGMsoft pushed a commit to MarioJGMsoft/FluidFramework that referenced this pull request Jul 8, 2025
…for SharedTree table APIs" (microsoft#24940)

Reverts microsoft#24881. The perf benchmarks pipeline
started taking a very long time to run SharedTree benchmarks after it,
so opening this revert to have it ready in case we need to merge it to
unblock the pipeline.
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.

5 participants