Skip to content

test(tree): Re-enable commented-out table benchmarks#27227

Open
Josmithr wants to merge 3 commits intomicrosoft:mainfrom
Josmithr:tree/table/fix-performance-tests
Open

test(tree): Re-enable commented-out table benchmarks#27227
Josmithr wants to merge 3 commits intomicrosoft:mainfrom
Josmithr:tree/table/fix-performance-tests

Conversation

@Josmithr
Copy link
Copy Markdown
Contributor

@Josmithr Josmithr commented May 4, 2026

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 4, 2026

Hi! Thank you for opening this PR. Want me to review it?

Based on the diff (746 lines, 2 files), I've queued these reviewers:

  • Correctness — logic errors, race conditions, lifecycle issues
  • Security — vulnerabilities, secret exposure, injection
  • API Compatibility — breaking changes, release tags, type design
  • Performance — algorithmic regressions, memory leaks
  • Testing — coverage gaps, hollow tests

How this works

  • Adjust the reviewer set by ticking/unticking boxes above. Reviewer toggles alone don't trigger anything.

  • Tick Start review below to dispatch the review fleet.

  • After review finishes, tick Start review again to request another run — it auto-resets after each dispatch.

  • This comment updates as new commits land; your reviewer selections are preserved.

  • Start review

@Josmithr Josmithr marked this pull request as ready for review May 5, 2026 03:30
@Josmithr Josmithr requested a review from a team as a code owner May 5, 2026 03:30
@Josmithr Josmithr requested review from CraigMacomber and Copilot May 5, 2026 03:30
Copy link
Copy Markdown
Contributor

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 re-enables previously commented-out SharedTree “table” benchmarks (both duration and memory) related to undo/redo scenarios, aligning with AB#43364.

Changes:

  • Re-enabled multiple undo/redo benchmark cases in the execution-time suite for table operations (insert/remove column/row, including combined scenarios).
  • Re-enabled corresponding redo-focused memory-usage benchmarks for similar table operation patterns.

Reviewed changes

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

File Description
packages/dds/tree/src/test/shared-tree/tableTree.bench.ts Re-enables several previously disabled duration benchmarks for undo/redo table operations.
packages/dds/tree/src/test/memory/tableTree.spec.ts Re-enables several previously disabled memory benchmarks for redo (and related) table operations.

Comment on lines +419 to +451
runBenchmark({
title: `Redo: Remove a column and a row in the middle ${count} times`,
tableSize,
initialCellValue,
beforeOperation: (table, undoRedoManager) => {
for (let i = 0; i < count; i++) {
const column = table.columns[Math.floor(table.columns.length / 2)];
const row = table.rows[Math.floor(table.rows.length / 2)];
table.removeColumns([column]);
table.removeRows([row]);
}
for (let i = 0; i < count; i++) {
// Undo remove row
undoRedoManager.undo();
// Undo remove column
undoRedoManager.undo();
}
assert(!undoRedoManager.canUndo);
assert(undoRedoManager.canRedo);
},
operation: (_table, undoRedoManager) => {
for (let i = 0; i < count; i++) {
// Redo remove row
undoRedoManager.redo();
// Redo remove column
undoRedoManager.redo();
}
},
afterOperation: (_table, undoRedoManager) => {
assert(!undoRedoManager.canRedo);
},
maxBenchmarkDurationSeconds,
});
Comment on lines +441 to +444
// Redo remove row
undoRedoManager.redo();
// Redo remove column
undoRedoManager.redo();
undoRedoManager.undo();
undoRedoManager.undo();
}
assert(!undoRedoManager.canUndo);
// Undo column insertion
undoRedoManager.undo();
}
assert(!undoRedoManager.canUndo);
for (let i = 0; i < count; i++) {
undoRedoManager.undo();
}
assert(!undoRedoManager.canUndo);
// Undo column removal
undoRedoManager.undo();
}
assert(!undoRedoManager.canUndo);
// Undo column insertion
undoRedoManager.undo();
}
assert(!undoRedoManager.canUndo);
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