-
Notifications
You must be signed in to change notification settings - Fork 554
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
Conversation
There was a problem hiding this 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. |
4efc39e
to
c885a49
Compare
There was a problem hiding this 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!!
There was a problem hiding this 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!
…edTree table APIs
087b804
to
0b68773
Compare
🔗 No broken links found! ✅ Your attention to detail is admirable. linkcheck output
|
table.removeRow(row); | ||
} | ||
for (let i = 0; i < count; i++) { | ||
undoRedoManager.undo(); |
There was a problem hiding this comment.
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)? 😁
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this 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!
…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) ~~~
…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.
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