Skip to content

perf(visualizer): fix O(n²) chars().nth scan and reduce allocations#344

Merged
Boshen merged 1 commit into
mainfrom
perf/visualizer-and-token-accessors
May 26, 2026
Merged

perf(visualizer): fix O(n²) chars().nth scan and reduce allocations#344
Boshen merged 1 commit into
mainfrom
perf/visualizer-and-token-accessors

Conversation

@Boshen
Copy link
Copy Markdown
Member

@Boshen Boshen commented May 26, 2026

Summary

  • Bug fix in generate_line_utf16_tables: content.chars().nth(i + 1) treated the byte offset i (from char_indices()) as a char index. That made the \r\n check walk the string from the start on every line break (O(n²) over the loop) and return the wrong char whenever earlier content contained multi-byte UTF-8. \n is always a single ASCII byte, so peeking content.as_bytes().get(i + 1) is both O(1) and correct.
  • Replace s.push_str(&format!(...)) in SourcemapVisualizer::get_text with writeln!(s, ...) — formats directly into the output String instead of allocating an intermediate one per token.
  • Mark trivial Token accessors (get_dst_line, get_dst_col, get_src_line, get_src_col, get_source_id, get_name_id) #[inline]. Release+LTO already inlines them through serialize_mappings, but the annotation helps debug builds and downstream crates that don't enable LTO.

No behavior change — visualizer insta snapshots match exactly.

`generate_line_utf16_tables` used `content.chars().nth(i + 1)` where `i`
is a byte offset from `char_indices()`. That treated `i` as a char
index, so each `\r\n` check walked the string from the start (O(n) per
check, O(n²) over the loop) and returned the wrong char whenever
preceding content contained multi-byte UTF-8. `\n` is always a single
ASCII byte, so peeking `content.as_bytes().get(i + 1)` is both O(1) and
correct.

Also replace `s.push_str(&format!(...))` in `get_text` with `writeln!`,
which writes directly into the output `String` instead of allocating an
intermediate one per token.

Add `#[inline]` on `Token::{get_dst_line, get_dst_col, get_src_line,
get_src_col, get_source_id, get_name_id}`. Release+LTO already inlines
these through `serialize_mappings`, but the annotation helps debug
builds and downstream crates that don't enable LTO.

No behavior change — visualizer insta snapshots are unchanged.
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented May 26, 2026

Merging this PR will not alter performance

✅ 16 untouched benchmarks
⏩ 5 skipped benchmarks1


Comparing perf/visualizer-and-token-accessors (6d20ba6) with main (55ebb8e)

Open in CodSpeed

Footnotes

  1. 5 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@Boshen Boshen merged commit abae0f7 into main May 26, 2026
8 checks passed
@Boshen Boshen deleted the perf/visualizer-and-token-accessors branch May 26, 2026 05:33
@oxc-guard oxc-guard Bot mentioned this pull request May 26, 2026
graphite-app Bot pushed a commit to oxc-project/oxc that referenced this pull request May 26, 2026
#22742)

## Summary

Pin `oxc_sourcemap` to https://github.com/oxc-project/oxc-sourcemap rev [`abae0f7`](oxc-project/oxc-sourcemap@abae0f7) (current main) to pick up:

- the `SourceMap<'a>` / `OwnedSourceMap` lifetime API — zero-copy borrow-parsed sourcemaps, plus a lifetime-free owned wrapper for downstream struct fields
- the visualizer perf/correctness fix in [oxc-sourcemap#344](oxc-project/oxc-sourcemap#344) (O(n²) `chars().nth()` scan that also misread CRLF after multi-byte UTF-8)

## Changes

Two production call sites in `oxc_codegen` migrate from the lifetime-free `SourceMap` to `OwnedSourceMap`:

- `CodegenReturn::map: Option<oxc_sourcemap::OwnedSourceMap>`
- `SourcemapBuilder::into_sourcemap() -> oxc_sourcemap::OwnedSourceMap` (via the new `SourceMapBuilder::into_owned_sourcemap` helper)

`OwnedSourceMap` derefs to `SourceMap<'static>`, so existing downstream consumers (`to_data_url`, `to_json_string`, `oxc_sourcemap::napi::SourceMap::from`, `SourcemapVisualizer::new`) keep working through auto-deref. No call-site changes in `napi/`, `examples/`, or `tests/` beyond one test in `sourcemap_builder.rs` whose old assertion compared against `&Arc<str>` and now matches `Option<&str>`.

## Follow-up

Unpin and switch back to a crates.io version once `oxc_sourcemap` cuts the next release.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant