perf(diff): add timing logs and bail out of LCS on long lines to prevent freezes - #599
Conversation
Add performance instrumentation to debug slow/freezing diffs when viewing branches with long lines. Logs are added at every stage: - Frontend: getDiffFiles/getFileDiff call timing, file/line stats, max line lengths, syntax highlighting duration - Frontend: inline diff computation timing, large line pair warnings for O(m*n) LCS operations (similarity, computeCharHighlights) - Rust backend: get_diff_files/get_file_diff timing with line counts and max line lengths per file - Rust git-diff crate: file loading, hunk computation, and alignment timing breakdown - Rust diff_cache: remote collection script exec, JSON parsing, and processing timing Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Skip O(m*n) LCS operations in similarity() and computeCharHighlights() when the product of input lengths exceeds 1,000,000. For similarity, returns 0 (lines treated as unrelated). For char highlights, marks the entire line as changed. Uses the same threshold that was previously only used for logging. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a798e6b57e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| afterLines: string[], | ||
| ): LineDiffResult { | ||
| const t0 = performance.now(); | ||
| const beforeMaxLen = Math.max(0, ...beforeLines.map(l => l.length)); |
There was a problem hiding this comment.
Using Math.max(0, ...beforeLines.map(...)) expands one argument per line, which throws a RangeError in V8 once the file has a large number of lines (roughly 100k+). Because this runs at the start of computeLineDiff, a large but otherwise valid text diff can fail before any fallback logic, so the inline diff view breaks for exactly the large inputs this change is trying to protect.
Useful? React with 👍 / 👎.
| const diff = await commands.getFileDiff(state.branchId, state.commitSha, state.scope, path); | ||
| const beforeLineCount = diff.before?.content?.type === 'Text' ? diff.before.content.lines.length : 0; | ||
| const afterLineCount = diff.after?.content?.type === 'Text' ? diff.after.content.lines.length : 0; | ||
| const beforeMaxLen = diff.before?.content?.type === 'Text' ? Math.max(0, ...diff.before.content.lines.map((l: string) => l.length)) : 0; |
There was a problem hiding this comment.
The new logging computes max line length via Math.max(0, ...lines.map(...)), which can also raise RangeError: Maximum call stack size exceeded on large files due to argument explosion. In this function the exception is caught and treated as a diff load failure, so users can get null diffs for large files even when backend diff generation succeeded.
Useful? React with 👍 / 👎.
Summary
Test plan
🤖 Generated with Claude Code