perf(linter/reporter/stylish): compute diagnostic Info once per diagnostic#24525
Merged
Conversation
camc314
approved these changes
Jul 15, 2026
Contributor
Merge activity
|
…ostic (#24525) AI Disclosure: This was generated with Claude Code, Fable 5. It has been tested and reviewed by me for correctness. `format_stylish` recomputed `Info::new` for every diagnostic in multiple passes — inside `sort_by_key` (which re-evaluates its key on every comparison), then again when grouping by filename, measuring the position column width, and rendering. Each `Info::new` call performs two `read_span` calls, which scan the file's source text from offset 0 to compute line/column, so `--format=stylish` output time grew quadratically with the number of diagnostics in a file. This change computes `Info` once per diagnostic into a `Vec<(Info, &Error)>` and reuses it for sorting, grouping, and rendering. Output is byte-identical (all output-formatter snapshot tests pass unchanged). This does have a downside in that it uses more peak memory in storing the `Info` objects in the Vec. I think the time savings are worth it, and it'd only be a problem for the (less likely) case where there are thousands of diagnostics. Though that's also the case where the optimization is most notable, obviously. ## Benchmark Fixture: a single 1.4MB file with 20k `debugger;` statements interleaved with filler lines (and a 10× smaller variant), release build, macOS. | Command | Before | After | |---|---|---| | `--format=stylish`, 20k diagnostics | 98.5s | 18.9s (5.2× faster) | | `--format=stylish`, 2k diagnostics | 1.09s | 0.22s | | `--format=unix`, 20k diagnostics (unchanged reference) | 20.6s | 20.6s | The fixed `stylish` now matches the streaming `unix` formatter on the same input. The remaining ~19s is a separate, shared issue: `Info::new` scans the source linearly per diagnostic, making every formatter that uses it quadratic in diagnostics-per-file — that would need a fix in `oxc_diagnostics` (e.g. computing positions for all of a file's diagnostics in one scan) and is not addressed here.
graphite-app
Bot
force-pushed
the
perf-stylish-formatter
branch
from
July 15, 2026 10:13
a879290 to
90ae040
Compare
camc314
added a commit
that referenced
this pull request
Jul 21, 2026
# Oxlint ### 💥 BREAKING CHANGES - 54cc121 ast: [**BREAKING**] Split `MetaProperty` into `ImportMeta` and `NewTarget` (#24557) (camc314) ### 🚀 Features - 7b045cd minfier: Drop last break from last switch case (#24673) (Armano) - dd18383 linter/node: Implement no-top-level-await rule (#24634) (Connor Shea) - 16a65f2 linter/react: Implement function-component-definition rule (#24471) (Cole Ellison) - 7f1f585 linter: Reuse `jest/padding-around-test-blocks` for `vitest/padding-around-test-blocks` (#24519) (Mikhail Baev) - 99978a8 linter/import/consistent-type-specifier-style: Support `prefer-top-level-if-only-type-imports` option (#24502) (camc314) ### 🐛 Bug Fixes - 0184ad6 linter/unicorn/no-useless-undefined: Preserve valid parameter defaults (#24686) (camc314) - 8694167 linter/eslint/prefer-destructuring: Handle typed declarations (#24616) (camc314) - 477cf0f linter/eslint/no-throw-literal: Handle assigned errors (#24561) (Cole Ellison) - ac9200a linter: Detect React components from returned JSX (#24521) (camc314) - c0a6522 linter/eslint/no-useless-computed-key: Allow TS syntax in computed keys (#24524) (Cole Ellison) ### ⚡ Performance - 346eed1 linter/unicorn/prefer-event-target: Only run on `Class` and `NewExpression` nodes (#24685) (Mikhail Baev) - 7be5cf0 oxlint/lsp: Only invoke lint on code actions when document is not opened (#24676) (Sysix) - d3f07a0 diagnostics: Box OxcDiagnosticInner to reduce binary size (#24665) (Boshen) - 90ae040 linter/reporter/stylish: Compute diagnostic Info once per diagnostic (#24525) (connorshea) ### 📚 Documentation - e6f7174 linter/valid-expect: Fix correct example being identical to incorrect one (#24468) (mkan0141) # Oxfmt ### 💥 BREAKING CHANGES - 54cc121 ast: [**BREAKING**] Split `MetaProperty` into `ImportMeta` and `NewTarget` (#24557) (camc314) ### 🚀 Features - 3d22307 parser: Add `ParseOptions::enable_ident_hashes` (#24491) (Boshen) ### 🐛 Bug Fixes - 6fe866a oxfmt: Keep tailwind classes glued to template expr with `preserveWhitespace` (#24609) (leaysgur) - 33e32d8 formatter_css: Use `line_suffix` for EOL line comment (#24580) (leaysgur) - 5f76998 formatter_graphql: Keep same line comments pending across intervening tokens (#24579) (leaysgur) Co-authored-by: Boshen <[email protected]> Co-authored-by: Cameron <[email protected]>
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
2 participants
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.
AI Disclosure: This was generated with Claude Code, Fable 5. It has been tested and reviewed by me for correctness.
format_stylishrecomputedInfo::newfor every diagnostic in multiple passes — insidesort_by_key(which re-evaluates its key on every comparison), then again when grouping by filename, measuring the position column width, and rendering. EachInfo::newcall performs tworead_spancalls, which scan the file's source text from offset 0 to compute line/column, so--format=stylishoutput time grew quadratically with the number of diagnostics in a file.This change computes
Infoonce per diagnostic into aVec<(Info, &Error)>and reuses it for sorting, grouping, and rendering. Output is byte-identical (all output-formatter snapshot tests pass unchanged).This does have a downside in that it uses more peak memory in storing the
Infoobjects in the Vec. I think the time savings are worth it, and it'd only be a problem for the (less likely) case where there are thousands of diagnostics. Though that's also the case where the optimization is most notable, obviously.Benchmark
Fixture: a single 1.4MB file with 20k
debugger;statements interleaved with filler lines (and a 10× smaller variant), release build, macOS.--format=stylish, 20k diagnostics--format=stylish, 2k diagnostics--format=unix, 20k diagnostics (unchanged reference)The fixed
stylishnow matches the streamingunixformatter on the same input. The remaining ~19s is a separate, shared issue:Info::newscans the source linearly per diagnostic, making every formatter that uses it quadratic in diagnostics-per-file — that would need a fix inoxc_diagnostics(e.g. computing positions for all of a file's diagnostics in one scan) and is not addressed here.