rust-analyzer / countme

Apache License 2.0
18 stars 4 forks source link

Optimizations #10

Closed WaffleLapkin closed 3 years ago

WaffleLapkin commented 3 years ago

Hey there! I've watched your video in the "Explaining rust-analyzer" series where you said that this crate can be optimized and decided to look into it (great series btw, thanks :>).

I've implemented some general optimizations (mostly by looking for long functions in flamegraph) as well as the first approach from #9. I've tried to make commits self-contained and write good messages for them, so reviewing commit-by-commit is probably a good idea.

For me in the end, benchmarks (see details below) show these results:

Details on how I've done benchmarking

That may be silly, but I've just run [hyperfine](https://github.com/sharkdp/hyperfine) on pairs of commits with a bash script like this (I'm bad at bash, I know): ```bash #!/usr/bin/bash if [ ! -z "$2" ] then git checkout "$2" 2> /dev/null || echo "couldn't do checkout" fi echo "comparing:" git log -1 --oneline cargo br --example bench --features="enable" 2> /dev/null || echo "compilation failure" cp ./target/release/examples/bench ./bench_current if [ ! -z "$1" ] then git checkout $1 2> /dev/null || echo "couldn't do checkout" else git checkout HEAD~1 2> /dev/null || echo "couldn't do checkout" fi echo "to:" git log -1 --oneline cargo br --example bench --features="enable" 2> /dev/null || echo "compilation failure" cp ./target/release/examples/bench ./bench_prev git switch - 2> /dev/null || echo "couldn't switch back" hyperfine -w 5 -m 20 -c "sleep 20" ./bench_prev ./bench_current ``` Results (multi-thread): ```text comparing: d76b8c3 (HEAD -> optimizations) Fix docs: correctly mention when counting is and isn't enabled by default to: b69e96e (HEAD) Disable at runtime by default Benchmark 1: ./bench_prev Time (mean ± σ): 4.102 s ± 0.214 s [User: 30.658 s, System: 0.021 s] Range (min … max): 3.660 s … 4.463 s 20 runs Benchmark 2: ./bench_current Time (mean ± σ): 604.1 ms ± 48.3 ms [User: 4363.1 ms, System: 10.3 ms] Range (min … max): 541.5 ms … 731.1 ms 20 runs Summary './bench_current' ran 6.79 ± 0.65 times faster than './bench_prev' ``` Results (single-thread): ```text comparing: d76b8c3 (HEAD -> optimizations) Fix docs: correctly mention when counting is and isn't enabled by default to: e95e4e4 (HEAD) Add single thread bench example Benchmark 1: ./bench_prev Time (mean ± σ): 4.679 s ± 0.029 s [User: 4.666 s, System: 0.007 s] Range (min … max): 4.626 s … 4.779 s 20 runs Benchmark 2: ./bench_current Time (mean ± σ): 1.789 s ± 0.023 s [User: 1.780 s, System: 0.006 s] Range (min … max): 1.775 s … 1.872 s 20 runs Warning: Statistical outliers were detected. Consider re-running this benchmark on a quiet PC without any interferences from other programs. It might help to use the '--warmup' or '--prepare' options. Summary './bench_current' ran 2.62 ± 0.04 times faster than './bench_prev' ```

I've also tried to see if running RA_COUNT=1 cargo run -q --release -p rust-analyzer -- analysis-stats . would be faster, but it seems like noise affects performance a lot more than couting.

matklad commented 3 years ago

Could you bump cargo toml version here as well, so that this gets a release automatically?

WaffleLapkin commented 3 years ago

Sure, I've updated version to 3.0.0

matklad commented 3 years ago

bors r+

bors[bot] commented 3 years ago

Build succeeded: