perf(editor): cut per-keystroke work on two rich-markdown paths (#10862)

* perf(editor): cut per-keystroke work on two rich-markdown paths

Doc links: both plugins walked every text node and ran matchAll on each — the
auto-convert appendTransaction once per keystroke, the preview decorations once
per keystroke and again per caret move. A link needs `[[`, so gate on a native
substring check first. The two walks had duplicated their guard sequence; they
now share one predicate. 3.1x-3.8x over the repo's own markdown.

Annotations: resolving a comment's block re-serializes the whole document (every
node, plus every adjacent pair), and both the highlight-range and
comment-at-position paths did that once per comment — O(comments x document).
Build the blocks once and pass them down. On a 12-node fixture with 8 comments
that is 184 serializations down to 23.

* test(editor): pin the one-build serialization baseline

Review feedback, all four points:

- The serialize-count assertions compared many-comments against one-comment, so
  they would have passed if BOTH built blocks twice. Pin the absolute count
  (23 = 12 nodes + 11 adjacent pairs) derived from the fixture size, so a
  regression to per-comment building fails instead of comparing equal. Verified
  by reverting the hoist: 2 tests fail.
- Skip an empty benchmark corpus instead of evaluating `index % 0` and
  dereferencing undefined.
- Build fixture paths with path.join.
- Condense the benchmark header to purpose plus parity guarantee.

Co-authored-by: Orca <help@stably.ai>

* test(editor): harden doc-link performance evidence

Co-authored-by: Orca <help@stably.ai>

---------

Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Neil 2026-07-27 17:18:31 -07:00 committed by GitHub
parent d547e278f9
commit 0edc95fa35
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 362 additions and 43 deletions

View File

@ -0,0 +1,202 @@
#!/usr/bin/env node
// Measures the complete ProseMirror doc traversal used by the two doc-link plugins.
import { execFileSync } from 'node:child_process'
import { readFileSync } from 'node:fs'
import { join } from 'node:path'
import { performance } from 'node:perf_hooks'
import { fileURLToPath } from 'node:url'
import { Schema } from '@tiptap/pm/model'
import {
canHoldDocLink,
DOC_LINK_PATTERN,
isDocLinkLiteralCodeTextNode
} from '../../src/renderer/src/components/editor/rich-markdown-doc-link-scan.ts'
const REPO_ROOT = fileURLToPath(new URL('../..', import.meta.url))
const ITERATIONS = Number(process.env.ORCA_DOC_LINK_BENCH_ITERATIONS ?? '41')
const WARMUP_ITERATIONS = Math.min(9, ITERATIONS)
if (!Number.isSafeInteger(ITERATIONS) || ITERATIONS <= 0) {
throw new Error(`ORCA_DOC_LINK_BENCH_ITERATIONS must be a positive integer, got ${ITERATIONS}`)
}
const schema = new Schema({
nodes: {
doc: { content: 'paragraph+' },
paragraph: { content: 'text*' },
text: { group: 'inline' }
}
})
function walkUngated(doc) {
let matches = 0
let visited = 0
doc.descendants((node, _pos, parent) => {
visited += 1
if (node.type.name !== 'text' || !node.text || isDocLinkLiteralCodeTextNode(node, parent)) {
return
}
for (const _match of node.text.matchAll(DOC_LINK_PATTERN)) {
matches += 1
}
})
return { matches, visited }
}
function walkGated(doc) {
let matches = 0
let visited = 0
doc.descendants((node, _pos, parent) => {
visited += 1
if (!canHoldDocLink(node, parent)) {
return
}
for (const _match of node.text.matchAll(DOC_LINK_PATTERN)) {
matches += 1
}
})
return { matches, visited }
}
function countMatches(source) {
let matches = 0
for (const _match of source.matchAll(DOC_LINK_PATTERN)) {
matches += 1
}
return matches
}
function loadDocs() {
const files = execFileSync('git', ['ls-files', '*.md', 'docs/*.md'], {
cwd: REPO_ROOT,
maxBuffer: 256 * 1024 * 1024
})
.toString()
.split('\n')
.filter(Boolean)
const docs = []
for (const file of files) {
try {
const source = readFileSync(join(REPO_ROOT, file), 'utf8')
const lines = source.split('\n').filter(Boolean)
if (lines.length > 0) {
docs.push({ file, lines, size: source.length, matches: countMatches(source) })
}
} catch {
// Indexed paths can disappear while the benchmark is running.
}
}
return docs
}
function createFixture(doc, nonce) {
const paragraphs = doc.lines.map((line, index) => {
const text = index === 0 ? `${line} bench-${nonce}` : line
return schema.node('paragraph', null, text ? schema.text(text) : undefined)
})
return schema.node('doc', null, paragraphs)
}
function median(samples) {
const sorted = [...samples].sort((a, b) => a - b)
return sorted[Math.floor(sorted.length / 2)]
}
function measureCorpus(docs) {
const samples = { ungated: [], gated: [] }
const totals = {
ungated: { matches: 0, visited: 0 },
gated: { matches: 0, visited: 0 }
}
const seenFixtures = new WeakSet()
let expectedMatches = 0
let expectedVisited = 0
const measuredOrder = []
for (let round = -WARMUP_ITERATIONS; round < ITERATIONS; round += 1) {
const doc = docs[(round + WARMUP_ITERATIONS) % docs.length]
const measured = round >= 0
const order =
(round + WARMUP_ITERATIONS) % 2 === 0 ? ['ungated', 'gated'] : ['gated', 'ungated']
const fixtures = {
ungated: createFixture(doc, String(round)),
gated: createFixture(doc, String(round))
}
for (const arm of order) {
const fixture = fixtures[arm]
if (seenFixtures.has(fixture)) {
throw new Error('timed fixture was reused')
}
seenFixtures.add(fixture)
const start = performance.now()
const result = arm === 'ungated' ? walkUngated(fixture) : walkGated(fixture)
const elapsed = performance.now() - start
if (measured) {
samples[arm].push(elapsed)
totals[arm].matches += result.matches
totals[arm].visited += result.visited
measuredOrder.push(arm)
}
}
if (measured) {
expectedMatches += doc.matches
expectedVisited += doc.lines.length * 2
}
}
if (totals.ungated.matches !== expectedMatches || totals.gated.matches !== expectedMatches) {
throw new Error(
`gate changed matches: expected ${expectedMatches}, ungated ${totals.ungated.matches}, gated ${totals.gated.matches}`
)
}
if (totals.ungated.visited !== expectedVisited || totals.gated.visited !== expectedVisited) {
throw new Error(
`full traversal result was not consumed: expected ${expectedVisited}, ungated ${totals.ungated.visited}, gated ${totals.gated.visited}`
)
}
for (let index = 0; index < measuredOrder.length; index += 2) {
const pair = measuredOrder.slice(index, index + 2).join(',')
const previousPair = index === 0 ? null : measuredOrder.slice(index - 2, index).join(',')
if (!['ungated,gated', 'gated,ungated'].includes(pair) || pair === previousPair) {
throw new Error('benchmark arms were not interleaved in alternating order')
}
}
return { ungated: median(samples.ungated), gated: median(samples.gated) }
}
const docs = loadDocs()
if (docs.length === 0) {
throw new Error('no markdown files found in the index')
}
const large = docs.filter((doc) => doc.size > 3000)
const biggest = docs.reduce((a, b) => (b.size > a.size ? b : a))
const pad = (value, width) => String(value).padStart(width)
console.log('Doc-link ProseMirror traversal, per editor transaction. Lower is better.')
console.log(
`docs=${docs.length} (>3KB: ${large.length}) iterations=${ITERATIONS} (interleaved median)`
)
console.log(
`${pad('corpus', 26)} ${pad('ungated', 11)} ${pad('gated', 11)} ${pad('delta', 11)} ${pad('speedup', 9)}`
)
for (const [label, set] of [
['all repo markdown', docs],
['docs over 3 KB', large],
[`biggest (${biggest.file.split('/').pop()})`, [biggest]]
]) {
if (set.length === 0) {
console.log(`${pad(label, 26)} ${pad('no docs in this corpus — skipped', 44)}`)
continue
}
const { ungated, gated } = measureCorpus(set)
const delta = ungated - gated
console.log(
`${pad(label, 26)} ${pad(`${(ungated * 1000).toFixed(1)} us`, 11)} ${pad(`${(gated * 1000).toFixed(1)} us`, 11)} ${pad(`${(delta * 1000).toFixed(1)} us`, 11)} ${pad(`${(ungated / gated).toFixed(2)}x`, 9)}`
)
}
console.log(
'\nAuto-conversion pays this once per keystroke; preview decorations pay it once\nper keystroke and once per caret move.'
)

View File

@ -1,16 +0,0 @@
type DocLinkTextNodeContext = {
marks: readonly { type: { name: string } }[]
}
type DocLinkParentContext = {
type: { spec: { code?: boolean } }
}
// Why: inline/fenced code content is literal markdown; auto-converting it would
// corrupt examples and commands that intentionally contain [[...]] text.
export function isDocLinkLiteralCodeTextNode(
node: DocLinkTextNodeContext,
parent: DocLinkParentContext | null
): boolean {
return parent?.type.spec.code === true || node.marks.some((mark) => mark.type.name === 'code')
}

View File

@ -0,0 +1,32 @@
export const DOC_LINK_PATTERN = /\[\[([^[\]\r\n]+)\]\]/g
const DOC_LINK_OPEN = '[['
type DocLinkTextNodeContext = {
type: { name: string }
text?: string
marks: readonly { type: { name: string } }[]
}
type DocLinkParentContext = {
type: { spec: { code?: boolean } }
}
export function isDocLinkLiteralCodeTextNode(
node: Pick<DocLinkTextNodeContext, 'marks'>,
parent: DocLinkParentContext | null
): boolean {
return parent?.type.spec.code === true || node.marks.some((mark) => mark.type.name === 'code')
}
// A link match requires the exact ASCII opener, including for non-ASCII targets.
export function canHoldDocLink(
node: DocLinkTextNodeContext,
parent: DocLinkParentContext | null
): node is DocLinkTextNodeContext & { text: string } {
return (
node.type.name === 'text' &&
!!node.text &&
node.text.includes(DOC_LINK_OPEN) &&
!isDocLinkLiteralCodeTextNode(node, parent)
)
}

View File

@ -1,8 +1,10 @@
import { describe, expect, it } from 'vitest'
import { isDocLinkLiteralCodeTextNode } from './rich-markdown-doc-link-code-context'
import { canHoldDocLink, DOC_LINK_PATTERN } from './rich-markdown-doc-link-scan'
function textContext(markNames: string[] = []) {
function textContext(text: string, markNames: string[] = []) {
return {
type: { name: 'text' },
text,
marks: markNames.map((name) => ({ type: { name } }))
}
}
@ -15,16 +17,30 @@ function parentContext(code: boolean) {
}
}
describe('isDocLinkLiteralCodeTextNode', () => {
it('allows doc link conversion in ordinary prose text', () => {
expect(isDocLinkLiteralCodeTextNode(textContext(), parentContext(false))).toBe(false)
function scan(text: string, markNames: string[] = [], parentCode = false): string[] {
const node = textContext(text, markNames)
if (!canHoldDocLink(node, parentContext(parentCode))) {
return []
}
return [...node.text.matchAll(DOC_LINK_PATTERN)].map((match) => match[1])
}
describe('production doc-link scan predicate', () => {
it('finds links in ordinary prose, including non-ASCII targets', () => {
expect(scan('See [[Guide]] and [[文档/😀]].')).toEqual(['Guide', '文档/😀'])
})
it('skips doc link conversion for inline code marks', () => {
expect(isDocLinkLiteralCodeTextNode(textContext(['code']), parentContext(false))).toBe(true)
it('rejects ordinary prose without the exact opener', () => {
expect(scan('See [Guide], plain prose, and closed brackets]].')).toEqual([])
})
it('skips doc link conversion for fenced code block text', () => {
expect(isDocLinkLiteralCodeTextNode(textContext(), parentContext(true))).toBe(true)
it('skips inline and fenced code', () => {
expect(scan('`[[inline]]`', ['code'])).toEqual([])
expect(scan('[[fenced]]', [], true)).toEqual([])
})
it('does not match links across CR or LF boundaries', () => {
expect(scan('[[carriage\rreturn]]')).toEqual([])
expect(scan('[[line\nfeed]]')).toEqual([])
})
})

View File

@ -9,16 +9,12 @@ import {
parseMarkdownDocLink,
resolveMarkdownDocLink
} from './markdown-doc-links'
import { isDocLinkLiteralCodeTextNode } from './rich-markdown-doc-link-code-context'
import {
isReservedRichMarkdownTransportBody,
type RichMarkdownSourceTransport
} from './rich-markdown-source-transport'
import { renderRichMarkdownDocLinkHtml } from './rich-markdown-doc-link-dom'
// Why: `.matchAll()` at each call site creates a fresh iterator so the shared
// `/g` regex never leaks `lastIndex` state across nested or concurrent scans.
const DOC_LINK_PATTERN = /\[\[([^[\]\r\n]+)\]\]/g
import { canHoldDocLink, DOC_LINK_PATTERN } from './rich-markdown-doc-link-scan'
const docLinkDissolveKey = new PluginKey('docLinkDissolve')
const docLinkAutoConvertKey = new PluginKey('docLinkAutoConvert')
@ -50,10 +46,7 @@ function buildPreviewDecorations(state: EditorState, storage: DocLinkStorage): D
const index = getDocIndex(storage)
const cursor = state.selection.from
state.doc.descendants((node, pos, parent) => {
if (node.type.name !== 'text' || !node.text) {
return
}
if (isDocLinkLiteralCodeTextNode(node, parent)) {
if (!canHoldDocLink(node, parent)) {
return
}
for (const match of node.text.matchAll(DOC_LINK_PATTERN)) {
@ -279,10 +272,7 @@ export function createMarkdownDocLink(transport: RichMarkdownSourceTransport) {
let modified = false
newState.doc.descendants((node, pos, parent) => {
if (node.type.name !== 'text' || !node.text) {
return
}
if (isDocLinkLiteralCodeTextNode(node, parent)) {
if (!canHoldDocLink(node, parent)) {
return
}

View File

@ -1,8 +1,12 @@
import { afterEach, describe, expect, it, vi } from 'vitest'
import type { Editor } from '@tiptap/core'
import type { DiffComment } from '../../../../shared/types'
import {
countRichMarkdownReviewMarkdownLines,
getRichMarkdownAnnotationButtonLeft,
getRichMarkdownAnnotationButtonTop
getRichMarkdownAnnotationButtonTop,
getRichMarkdownAnnotationHighlightRanges,
getRichMarkdownCommentAtPos
} from './rich-markdown-review-annotations'
afterEach(() => {
@ -47,3 +51,77 @@ describe('getRichMarkdownAnnotationButtonLeft', () => {
expect(getRichMarkdownAnnotationButtonLeft(72)).toBe(40)
})
})
// Why count serializes: resolving a comment's block re-serializes the document,
// so doing it per comment made these O(comments x document). A call count is
// deterministic where a wall-clock threshold would be flaky.
describe('rich markdown annotation block reuse', () => {
function makeEditor(nodeCount: number): { editor: Editor; serializeCalls: () => number } {
let serializeCalls = 0
const content = Array.from({ length: nodeCount }, (_value, index) => ({
type: 'paragraph',
content: [{ type: 'text', text: `paragraph ${index}` }]
}))
const doc = {
forEach(callback: (node: unknown, offset: number, index: number) => void): void {
content.forEach((node, index) => callback(node, index * 10, index))
},
// The text-range search walks the doc to locate the selected text; these
// fixtures never match, so it only needs to be traversable.
nodesBetween(): void {},
content: { size: nodeCount * 10 }
}
const editor = {
getJSON: () => ({ content }),
state: { doc },
markdown: {
serialize: (value: { content?: unknown[] }) => {
serializeCalls += 1
return (value.content ?? []).map((_node, index) => `line ${index}`).join('\n')
}
}
} as unknown as Editor
return { editor, serializeCalls: () => serializeCalls }
}
function makeComments(count: number): DiffComment[] {
return Array.from(
{ length: count },
(_value, index) => ({ lineNumber: index + 1, selectedText: 'nothing-matches' }) as DiffComment
)
}
// One block build over NODE_COUNT nodes: each node serialized alone, plus each
// adjacent pair. Pinned absolutely so "both arms build twice" can't pass as equal.
const NODE_COUNT = 12
const ONE_BUILD_SERIALIZE_CALLS = NODE_COUNT + (NODE_COUNT - 1)
it('serializes the document once regardless of comment count', () => {
const single = makeEditor(NODE_COUNT)
getRichMarkdownAnnotationHighlightRanges(single.editor, makeComments(1), 0)
const many = makeEditor(NODE_COUNT)
getRichMarkdownAnnotationHighlightRanges(many.editor, makeComments(8), 0)
expect(single.serializeCalls()).toBe(ONE_BUILD_SERIALIZE_CALLS)
expect(many.serializeCalls()).toBe(ONE_BUILD_SERIALIZE_CALLS)
})
it('serializes the document once when locating the comment at a position', () => {
const single = makeEditor(NODE_COUNT)
getRichMarkdownCommentAtPos(single.editor, makeComments(1), 0, 5)
const many = makeEditor(NODE_COUNT)
getRichMarkdownCommentAtPos(many.editor, makeComments(8), 0, 5)
expect(single.serializeCalls()).toBe(ONE_BUILD_SERIALIZE_CALLS)
expect(many.serializeCalls()).toBe(ONE_BUILD_SERIALIZE_CALLS)
})
it('does no work at all with no comments', () => {
const none = makeEditor(12)
expect(getRichMarkdownAnnotationHighlightRanges(none.editor, [], 0)).toEqual([])
expect(getRichMarkdownCommentAtPos(none.editor, [], 0, 5)).toBeNull()
expect(none.serializeCalls()).toBe(0)
})
})

View File

@ -119,17 +119,29 @@ export function getRichMarkdownAnnotationHighlightRanges(
comments: readonly DiffComment[],
markdownSourceLineOffset: number
): RichMarkdownAnnotationHighlightRange[] {
if (comments.length === 0) {
return []
}
// Why once: block resolution re-serializes the doc; per comment it was O(n*doc).
const blocks = buildRichMarkdownCommentBlocks(editor)
return comments.flatMap((comment) =>
getRichMarkdownAnnotationHighlightRangesForComment(editor, comment, markdownSourceLineOffset)
getRichMarkdownAnnotationHighlightRangesForComment(
editor,
comment,
markdownSourceLineOffset,
blocks
)
)
}
export function getRichMarkdownAnnotationHighlightRangesForComment(
editor: Editor,
comment: DiffComment,
markdownSourceLineOffset: number
markdownSourceLineOffset: number,
// Why optional: callers looping over comments pass one shared build.
prebuiltBlocks?: RichMarkdownCommentBlock[]
): RichMarkdownAnnotationHighlightRange[] {
const blocks = buildRichMarkdownCommentBlocks(editor)
const blocks = prebuiltBlocks ?? buildRichMarkdownCommentBlocks(editor)
const selectedText = comment.selectedText?.trim()
if (!selectedText) {
return []
@ -158,12 +170,17 @@ export function getRichMarkdownCommentAtPos(
markdownSourceLineOffset: number,
pos: number
): DiffComment | null {
if (comments.length === 0) {
return null
}
const blocks = buildRichMarkdownCommentBlocks(editor)
return (
comments.find((comment) =>
getRichMarkdownAnnotationHighlightRangesForComment(
editor,
comment,
markdownSourceLineOffset
markdownSourceLineOffset,
blocks
).some((range) => range.from <= pos && pos <= range.to)
) ?? null
)