perf(sourcemap): improve perf of search_original_line_and_column

[camc314]

this seems to show a decent speed up on perf.
the thinking behind this change is:

1. subsequent calls to search_original_line_and_column will never look at back at lines it has already searched
2. binary search is faster in this case as typically idx is only increased by a small amount

Given this, we can skip checking a bunch of lines (that will never return true) improving performance.

This is a very hot path.

$ hyperfine --warmup=3 ./target/release-with-debug/examples/sourcemap-main ./target/release-with-debug/examples/sourcemap
Benchmark 1: ./target/release-with-debug/examples/sourcemap-main
  Time (mean ± σ):      79.5 ms ±   5.5 ms    [User: 65.1 ms, System: 12.2 ms]
  Range (min … max):    76.8 ms … 111.7 ms    37 runs
 
  Warning: Statistical outliers were detected. Consider re-running this benchmark on a quiet system without any interferences from other programs. It might help to use the '--warmup' or '--prepare' options.
 
Benchmark 2: ./target/release-with-debug/examples/sourcemap
  Time (mean ± σ):      72.9 ms ±   0.9 ms    [User: 59.2 ms, System: 12.2 ms]
  Range (min … max):    70.6 ms …  75.0 ms    40 runs
 
Summary
  ./target/release-with-debug/examples/sourcemap ran
    1.09 ± 0.08 times faster than ./target/release-with-debug/examples/sourcemap-main

[camc314]

• perf(sourcemap): improve perf of search_original_line_and_column #7926 👈 (View in Graphite)
• refactor(benchmark): transform code before codegen #7934
• main

---

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

• 0-merge - adds this PR to the back of the merge queue
• hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

[codspeed-hq]

CodSpeed Performance Report

Merging #7926 will improve performances by 8.93%

_{Comparing c/12-15-perf_sourcemap_improve_perf_of_search_original_line_and_column_ (d9b6e31) with main (ff9d1b3)}

Summary

⚡ 1 improvements
✅ 28 untouched benchmarks

Benchmarks breakdown

	Benchmark	main	c/12-15-perf_sourcemap_improve_perf_of_search_original_line_and_column_	Change
⚡	codegen_sourcemap[checker.ts]	74.4 ms	68.3 ms	+8.93%

[camc314]

there's the op for more perf gains here.

SourceMapBuilder::tokens is just a Vec with no initial size. we can probably guess the size based on the length of the text to avoid some allocations - even a rough guess should be ok.

poc:

Summary
  ./target/release-with-debug/examples/sourcemap ran # reserving vec capacity
    1.03 ± 0.04 times faster than ./target/release-with-debug/examples/sourcemap-with-opt1 # this PR 
    1.11 ± 0.05 times faster than ./target/release-with-debug/examples/sourcemap-main  # buiilt against main

https://github.com/oxc-project/oxc-sourcemap/blob/0e5ec37a00d6c563727814b9ec41232b226eff90/src/sourcemap_builder.rs#L19

[overlookmotel]

Very nice!

Did I write this code?? It looks familiar. It also looks embarrassingly messy! I'd probably write it differently if I wrote it today. I think I've got a bit better at coding over past 6 months...

If you've enjoyed this and you'd be interested in pushing for further gains, I have a few thoughts of other optimizations we might be able to make, which I guess I didn't see first time around.

[camc314]

Very nice!

Did I write this code?? It looks familiar. It also looks embarrassingly messy! I'd probably write it differently if I wrote it today. I think I've got a bit better at coding over past 6 months...

yes and no i think? i think @ underfin wrote this originally, then you made some perf improvements in #4641

If you've enjoyed this and you'd be interested in pushing for further gains, I have a few thoughts of other optimizations we might be able to make, which I guess I didn't see first time around.

👀 let me know what you can see

[overlookmotel]

👀 let me know what you can see

I'll open an issue once we have this PR merged.

[nrayburn-tech]

Unrelated to this PR, but I just want to say that the conversations you two have in your PRs/issues are super helpful to somebody unfamiliar with Rust (or any low level languages).

Thanks for the explanations @overlookmotel and @camc314 for always asking questions.

[camc314]

Unrelated to this PR, but I just want to say that the conversations you two have in your PRs/issues are super helpful to somebody unfamiliar with Rust (or any low level languages).

Thanks for the explanations @overlookmotel and @camc314 for always asking questions.

❤️ this is amazing to hear. overlookmotel always has the answers

now just to make rust a part of my day job 🙂

[overlookmotel]

@camc314 I hope you don't mind, but I thought I'd give it a go. I've pushed another commit with an optimized implementation of search_original_line_forwards.

I didn't want to use #[cold], but I couldn't find another way to get the assembly in the shape I wanted - fastest path for "on the same line as last time", followed by the unrolled loop covering next 16 lines.

https://godbolt.org/z/heh1cnYa4 (the unrolled loop is the part where same 3 lines repeat over and over, .LBB0_3 is start of the binary search).

Let's see what CodSpeed says!

(We could get rid of the cold path by adding 16 dummy entries to end of the Vec<Line> in generate_line_offset_tables, so there are always at least 16 lines to search over. I'm not sure if that's a good idea or not.)

[overlookmotel]

OK, the perf gain is back to where it was originally, and we don't have the problem of pathologically bad perf if code has moved a long distance any more.

If you think my code looks OK, would you like to apply the same scheme to the backwards search?