perf: scan markdown review lines with native newline search (#20245)

This commit is contained in:
Neil
2026-09-12 18:16:12 -07:00
committed by GitHub
parent dbac4e6ed2
commit b8f1849ba8
3 changed files with 122 additions and 4 deletions
@@ -0,0 +1,81 @@
import assert from 'node:assert/strict'
import { execFileSync } from 'node:child_process'
import { readFileSync } from 'node:fs'
import { dirname, resolve } from 'node:path'
import { performance } from 'node:perf_hooks'
import { build } from 'esbuild'
import { buildCounterbalancedSchedule } from './counterbalanced-benchmark-schedule.mjs'
const sourcePath = 'src/renderer/src/lib/markdown-review-notes.ts'
const baseline = process.argv[2] ?? '20ab9950654'
async function load(contents) {
const result = await build({
stdin: { contents, loader: 'ts', resolveDir: dirname(resolve(sourcePath)) },
bundle: true,
platform: 'node',
format: 'esm',
write: false
})
return import(
`data:text/javascript;base64,${Buffer.from(result.outputFiles[0].text).toString('base64')}`
)
}
const before = await load(
execFileSync('git', ['show', `${baseline}:${sourcePath}`], { encoding: 'utf8' })
)
const after = await load(readFileSync(sourcePath, 'utf8'))
const results = []
for (const [name, lineCount, width, count, iterations] of [
['small', 20, 40, 1, 1000],
['long-lines', 1000, 1000, 20, 3],
['many-lines', 20000, 80, 20, 3],
['early-note', 20000, 80, 1, 1000]
]) {
const content = Array.from({ length: lineCount }, (_, i) => `${i}: ${'x'.repeat(width)}`).join(
'\r\n'
)
const notes = Array.from({ length: count }, (_, i) => ({
id: `${i}`,
worktreeId: 'bench',
filePath: 'README.md',
source: 'markdown',
lineNumber: name === 'early-note' ? 2 : lineCount - i,
body: 'Clarify this line',
createdAt: i,
side: 'modified'
}))
assert.equal(
after.formatMarkdownReviewNotes(notes, content),
before.formatMarkdownReviewNotes(notes, content)
)
const arms = { before, after }
const samples = { before: [], after: [] }
function run(arm) {
const start = performance.now()
for (let i = 0; i < iterations; i++) {
arms[arm].formatMarkdownReviewNotes(notes, content)
}
return (performance.now() - start) / iterations
}
for (let i = 0; i < 6; i++) {
run('before')
run('after')
}
for (const pair of buildCounterbalancedSchedule(12, 'before', 'after')) {
for (const arm of pair) {
samples[arm].push(run(arm))
}
}
const median = (xs) => xs.sort((a, b) => a - b)[Math.floor(xs.length / 2)]
results.push({
name,
lineCount,
width,
count,
beforeMs: median(samples.before),
afterMs: median(samples.after)
})
}
console.log(
JSON.stringify({ node: process.version, platform: process.platform, baseline, results }, null, 2)
)
@@ -94,6 +94,44 @@ describe('markdown review notes', () => {
expect(charCodeAt.mock.calls.length).toBeLessThan(64)
})
it('preserves empty, trailing, and mixed line endings across excerpt ranges', () => {
for (const content of [
'',
'one',
'\n',
'one\n',
'one\r\n\r\ntwo\r',
'one\rtwo',
Array.from({ length: 15 }, (_, i) => `line ${i}`).join('\n')
]) {
const sourceLines = content
.split('\n')
.map((line) => (line.endsWith('\r') ? line.slice(0, -1) : line))
for (let start = -1; start <= sourceLines.length + 1; start += 1) {
for (let end = start; end <= sourceLines.length + 1; end += 1) {
const first = Math.max(1, start)
const selected = sourceLines.slice(first - 1, Math.max(first, end))
const bounded =
selected.length <= 8
? selected
: [...selected.slice(0, 4), '...', ...selected.slice(-4)]
expect(
getMarkdownReviewExcerpt(content, note({ startLine: start, lineNumber: end }))
).toBe(bounded.map((line) => `> ${line}`).join('\n'))
}
}
}
})
it('finds late excerpts without checking every prefix character in JavaScript', () => {
const content = `${'x'.repeat(100_000)}\r\ntarget\nignored`
const charCodeAt = vi.spyOn(String.prototype, 'charCodeAt')
const excerpt = getMarkdownReviewExcerpt(content, note({ lineNumber: 2 }))
const checks = charCodeAt.mock.calls.length
expect(excerpt).toBe('> target')
expect(checks).toBeLessThanOrEqual(2)
})
it('prefers exact selected text for card highlights', () => {
const highlighted = getMarkdownReviewHighlightedText(
'one\ntwo broad line\nthree',
@@ -93,10 +93,9 @@ function forEachMarkdownReviewLine(
): void {
let lineStart = 0
let lineNumber = 1
for (let index = 0; index <= content.length; index += 1) {
if (index < content.length && content.charCodeAt(index) !== 10) {
continue
}
while (lineStart <= content.length) {
const newline = content.indexOf('\n', lineStart)
const index = newline === -1 ? content.length : newline
const lineEnd = index > lineStart && content.charCodeAt(index - 1) === 13 ? index - 1 : index
if (visit(content.slice(lineStart, lineEnd), lineNumber) === false) {
return