[ruff] Migrate cache from bincode to rkyv - #26374
Conversation
Summary `bincode` is unmaintained. This isn't a problem on its own but if a problem comes up with it. As ruff's use of `bincode` is relatively isolated, the risk is low. Options: - defer: reevaluating what to do - use a fork: didn't see one I was confident in - fork: nothing pressing us to do this atm - `bitcode`: faster than `bincode` but blocked on SoftbearStudios/bitcode#51 but how that is being handled doesn't give me much confidence in the maintenance - `rkyv`: almost as fast as `bitcode` and highly respected If it wasn't for `rkyv` looking so promising, I would say we defer this. Test Plan Leveraging existing tests as this should be transparent
|
We use rkyv heavily in uv! |
Typing conformance resultsNo changes detected ✅Current numbersThe percentage of diagnostics emitted that were expected errors held steady at 94.47%. The percentage of expected errors that received a diagnostic held steady at 89.19%. The number of fully passing files held steady at 95/134. |
Memory usage reportMemory usage unchanged ✅ |
|
Are you able to include some (brief) summarized numbers on cache size and performance? More concerned with the former, I trust the latter is similar. |
|
|
Do we have a typical set of use cases we run against? |
|
@ntBre do you remember which project's you used when you analyzed the caching/diagnostic rendering regression recently (I don't remember what it was). I often used airflow and CPython |
|
I don't remember specifically, but I also usually use our cpython benchmark for this kind of thing. |
|
This is the issue I had in mind #18198 (comment), where you used airflow, homeassistant and Cpython |
|
Before $ cargo build --release --bin ruff && hyperfine --warmup 10 "./target/release/ruff check ./crates/ruff_linter/resources/test/airflow/ --no-cache -e -s --isolated"
"./target/release/ruff check ./crates/ruff_linter/resources/test/airflow -e -s --isolated"
warning: ignoring 'build.analysis' config, pass `-Zbuild-analysis` to enable it
Compiling ruff v0.15.19 (/Users/epage/code/oss/ruff/crates/ruff)
Finished `release` profile [optimized] target(s) in 1m 44s
Benchmark 1: ./target/release/ruff check ./crates/ruff_linter/resources/test/airflow/ --no-cache -e -s --isolated
Time (mean ± σ): 161.8 ms ± 10.7 ms [User: 701.6 ms, System: 1186.6 ms]
Range (min … max): 148.8 ms … 187.0 ms 18 runs
Benchmark 2: ./target/release/ruff check ./crates/ruff_linter/resources/test/airflow -e -s --isolated
Time (mean ± σ): 75.6 ms ± 1.9 ms [User: 81.3 ms, System: 578.8 ms]
Range (min … max): 73.6 ms … 80.8 ms 37 runs
Summary
./target/release/ruff check ./crates/ruff_linter/resources/test/airflow -e -s --isolated ran
2.14 ± 0.15 times faster than ./target/release/ruff check ./crates/ruff_linter/resources/test/airflow/ --no-cache -e -s --isolated
$ rm -rf ruff_cache && cargo build --release --bin ruff && ./target/release/ruff check ./crates/ruff_linter/resources/test/airflow/ -e -s --isolated && du -ch .ruff_cache
warning: ignoring 'build.analysis' config, pass `-Zbuild-analysis` to enable it
Finished `release` profile [optimized] target(s) in 0.15s
2.6M .ruff_cache/0.15.19
2.6M .ruff_cache
2.6M totalAfter $ cargo build --release --bin ruff && hyperfine --warmup 10 "./target/release/ruff check ./crates/ruff_linter/resources/test/airflow/ --no-cache -e -s --isolated"
"./target/release/ruff check ./crates/ruff_linter/resources/test/airflow -e -s --isolated"
warning: ignoring 'build.analysis' config, pass `-Zbuild-analysis` to enable it
Finished `release` profile [optimized] target(s) in 0.18s
Benchmark 1: ./target/release/ruff check ./crates/ruff_linter/resources/test/airflow/ --no-cache -e -s --isolated
Time (mean ± σ): 160.5 ms ± 11.5 ms [User: 700.9 ms, System: 1201.8 ms]
Range (min … max): 146.4 ms … 180.1 ms 18 runs
Benchmark 2: ./target/release/ruff check ./crates/ruff_linter/resources/test/airflow -e -s --isolated
Time (mean ± σ): 71.6 ms ± 1.8 ms [User: 81.2 ms, System: 579.2 ms]
Range (min … max): 69.4 ms … 77.2 ms 36 runs
Summary
./target/release/ruff check ./crates/ruff_linter/resources/test/airflow -e -s --isolated ran
2.24 ± 0.17 times faster than ./target/release/ruff check ./crates/ruff_linter/resources/test/airflow/ --no-cache -e -s --isolated
$ rm -rf .ruff_cache/ && cargo build --release --bin ruff && ./target/release/ruff check ./crates/ruff_linter/resources/test/airflow/ -e -s --isolated && du -ch .ruf
f_cache
warning: ignoring 'build.analysis' config, pass `-Zbuild-analysis` to enable it
Compiling ruff v0.15.19 (/Users/epage/code/oss/ruff/crates/ruff)
Finished `release` profile [optimized] target(s) in 1m 46s
2.7M .ruff_cache/0.15.19
2.7M .ruff_cache
2.7M total |
Summary
bincodeis unmaintained.This isn't a problem on its own but if a problem comes up with it. As ruff's use of
bincodeis relatively isolated, the risk is low.Options:
bitcode: faster thanbincodebut blocked on Make unsound code opt-in. SoftbearStudios/bitcode#51 but how that is being handled doesn't give me much confidence in the maintenancerkyv: almost as fast asbitcodeand highly respectedIf it wasn't for
rkyvlooking so promising, I would say we defer this.Fixes #22284
Test Plan
Leveraging existing tests as this should be transparent