Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
@blocknote/ariakit
@blocknote/code-block
@blocknote/core
@blocknote/diagram-block
@blocknote/mantine
@blocknote/math-block
@blocknote/react
@blocknote/server-util
@blocknote/shadcn
@blocknote/xl-ai
@blocknote/xl-docx-exporter
@blocknote/xl-email-exporter
@blocknote/xl-multi-column
@blocknote/xl-odt-exporter
@blocknote/xl-pdf-exporter
@blocknote/xl-typst-exporter
commit: |
|
YousefED
force-pushed
the
perf/version-diff-render
branch
from
October 9, 2026 10:57
16ad0f5 to
f250967
Compare
A re-created block (a type change or a move) is shown as the deleted original next to its copy, with the same id. UniqueID ignored the deleted block when it rewrote ids, but still counted it as a duplicate. So when both were new in one transaction, as when a version diff is shown, the copy got a new id, which is in neither version.
getNodeId walked the document from the start for each block marked as deleted, and callers look up every block. In a version diff with many re-created blocks, this was quadratic. The ids of all deleted blocks are now found in one walk and kept for that document.
…attributions A version diff of 2000 blocks is one transaction with 12,000 steps. Tiptap's getChangedRanges maps each range through every later step and back, and compares every range with every other range. A new getChangedRanges gives the same result: it skips steps that cannot move a position, or only shift it, and compares only ranges that can contain each other. UniqueID and autolink also replayed all steps of a single transaction into a new transform, and UniqueID mapped every new node back through all steps. They now use the transaction itself, and UniqueID maps only nodes with a duplicated id.
…nsforms The first version queried a segment tree for each step map, so it was slower than Tiptap's for 5 to 50 steps. It now checks each map directly, and uses the trees (built only when needed) to skip a long run of maps of one kind. It is now faster for 1 step and up, in all measured step orders.
UniqueID and autolink each had their own check for a single transaction. A wrapper now does it for all three callers, which also stops getBlocksChangedByTransaction from replaying the steps of a single transaction.
…hangedRanges The random test now also makes split, join, setBlockType, RemoveNodeMarkStep and deletes across blocks, and checks that all 8 step types occur.
The name now says how it differs from ProseMirror's changedRange() and from getChangedRanges: it also covers attribute-only steps.
AttributionExtension merged the list from getChangedRanges into one range. getChangedRangeWithAttrs gives that range directly, and it also covers node-mark steps, so the special case for AddNodeMarkStep is gone.
YousefED
force-pushed
the
perf/version-diff-render
branch
from
October 9, 2026 11:56
f250967 to
22838e3
Compare
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR does two things:
editor.documentthen showed an id that is in neither version. See change 1.It is a separate PR on top of #3090, so that it can be reviewed separately.
Why it was slow
The y-prosemirror binding renders the diff as one transaction with about 6 steps for each re-created block (12,000 steps for the example above). Several plugins process each step in a way that is quadratic in the number of steps:
getChangedRanges, called by UniqueID, autolink and AttributionExtensioncombineTransactionStepsandmapping.invert()for each node in UniqueIDgetNodeId: a document walk for each deleted blockChanges
editor.documentthen showed an id that is in neither version. The deleted block is now ignored in the duplicate check too, as the existing comment onisMarkedDeletedsays. Nick's test that expected the old behavior is updated.getNodeId: one walk for each document. The ids of all deleted blocks are calculated in one walk and stored for that document. Before, each deleted block walked the document from the start.getChangedRanges: same result, without the quadratic cost. A newapi/getChangedRanges.tsreplaces Tiptap's function in UniqueID and autolink. It gives the same ranges as Tiptap's function. It skips runs of steps that cannot move a position, or that only shift it, and the simplify step compares only ranges that can contain each other.combineTransactionStepsapplied all steps again to a new transform, also when there was only one transaction. A wrapper (api/combineTransactionSteps.ts) now returns that transaction as it is. UniqueID, autolink andgetBlocksChangedByTransactionuse it. UniqueID also mapped every new node back through all steps. It now does this only for nodes with a duplicated id.getChangedRangeWithAttrs(renamed fromgetChangedRange), which also covers node-mark steps, so its special case forAddNodeMarkStepis removed.versionDiffPerformance.test.tschecks that showing a version uses one step. It isit.fails, because the y-prosemirror binding renders the diff as one step per change.Results
Time to show a version (jsdom). Half of the blocks are indented, so they are re-created.
With this PR, the plugins take about 0.25 s of the 2.5 s (before: 6.5 s). Most of the remaining time is node-view rendering, which grows linearly.
The same change, when it comes from a collaborator into an open editor, is one step. So it was not slow before (0.5 s for 2000 blocks), and this PR does not change it.
Small transforms (regular editing)
getChangedRangesalone, with steps that insert text. Time per call for Tiptap's function and for the new one:The new function is faster at every size, so regular editing gets no overhead. It checks each step map directly, and uses segment trees (built only when needed) to skip a long run of maps.
How the
getChangedRangeschange was testedsetBlockTypeand node marks. The test checks that all 8 step types occur (ReplaceStep,ReplaceAroundStep,AddMarkStep,RemoveMarkStep,AttrStep,DocAttrStep,AddNodeMarkStep,RemoveNodeMarkStep). The new function must give exactly the same result as Tiptap's.Not in this PR
getChangedRangeWithAttrs). That would be simpler and faster, and might be a better solution. But it can change which ids are rewritten, so it needs careful investigation first. There is a TODO at the call site.getChangedRangescould be deprecated, because a list of ranges is costly to compute exactly. Its doc comment says so. Its remaining callers (UniqueID and autolink) might work from one range, and autolink perhaps from input events (space, Enter, paste). For both, what that changes needs investigation first.