Skip to content

Conversation

@noencke
Copy link
Contributor

@noencke noencke commented Jan 30, 2026

  • Removes rebaser usages of the sorted-btree library's defaultComparator, which is slow (and came up in performance profiling).
  • Updates TupleBTrees to require comparators for their elements, and adds some additional compile time safety around the typing.
  • Fixes a bug in the RevisionTag comparator which compared "root" equal to every number.
  • Adds and removes core comparators in our utils folder and updated them with consistent naming
  • Adds unit tests for the comparators

Copilot AI review requested due to automatic review settings January 30, 2026 20:18
@noencke noencke requested a review from a team as a code owner January 30, 2026 20:18
Copy link
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 improves comparator functions in the tree package by fixing a critical bug and enhancing performance:

Changes:

  • Fixes a bug in comparePartialRevisions (formerly compareRevisions) that incorrectly compared "root" string equal to all numbers
  • Adds new comparator utility functions (compareNumbers, comparePartialNumbers, comparePartialStrings) with proper handling of edge cases (NaN, Infinity, undefined)
  • Removes defaultComparator usage from sorted-btree library (performance improvement)
  • Introduces newChangeAtomIdBTree helper to replace manual newTupleBTree calls with proper comparators
  • Updates newTupleBTree and mergeTupleBTrees signatures for better API design
  • Adds comprehensive unit tests for all comparators

Reviewed changes

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

Show a summary per file
File Description
packages/dds/tree/src/util/utils.ts Removes Named interface and compareNamed function; adds new comparator functions (compareNumbers, comparePartialNumbers, comparePartialStrings) with proper edge case handling
packages/dds/tree/src/util/index.ts Updates exports to include new comparator functions and remove deleted ones
packages/dds/tree/src/util/bTreeUtils.ts Updates newTupleBTree to accept comparator parameter and mergeTupleBTrees to require non-undefined first parameter; improves tuple comparison implementation
packages/dds/tree/src/test/util/utils.spec.ts Adds comprehensive unit tests for all new comparator functions including edge cases
packages/dds/tree/src/test/shared-tree/sharedTreeChangeCodec.spec.ts Updates to use newChangeAtomIdBTree instead of newTupleBTree
packages/dds/tree/src/test/feature-libraries/modular-schema/modularChangesetUtil.ts Updates to use newChangeAtomIdBTree instead of newTupleBTree
packages/dds/tree/src/test/feature-libraries/modular-schema/modularChangeFamily.spec.ts Updates all usages to use newChangeAtomIdBTree with proper typed comparators
packages/dds/tree/src/feature-libraries/modular-schema/modularChangeFamily.ts Implements newFieldIdKeyBTree with proper comparators; updates all BTree creation to use typed functions
packages/dds/tree/src/feature-libraries/modular-schema/modularChangeCodecV1.ts Updates to use newChangeAtomIdBTree for decoding
packages/dds/tree/src/feature-libraries/modular-schema/index.ts Changes from export type to export to also export newCrossFieldKeyTable function
packages/dds/tree/src/feature-libraries/index.ts Adds export for newChangeAtomIdBTree
packages/dds/tree/src/feature-libraries/changeAtomIdBTree.ts Adds newChangeAtomIdBTree helper function with proper comparators for ChangeAtomId tuples
packages/dds/tree/src/core/rebase/types.ts Renames compareRevisions to comparePartialRevisions and fixes bug where string "root" compared equal to all numbers
packages/dds/tree/src/core/rebase/index.ts Updates export name to comparePartialRevisions
packages/dds/tree/src/core/index.ts Exports comparePartialRevisions as compareRevisions for backward compatibility
.gitignore Adds .worktrees/ entry (appears unrelated to PR purpose)

@noencke noencke changed the title Improve comparator functions Improve SharedTree comparator functions Jan 30, 2026
@noencke noencke force-pushed the optimize-comparators branch from bc3c4a1 to a13a938 Compare January 30, 2026 21:14
@noencke noencke merged commit 8fe5725 into microsoft:main Jan 31, 2026
28 checks passed
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