Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

analyze: borrowck performance improvements #1111

Merged
merged 3 commits into from
Aug 7, 2024

Conversation

spernsteiner
Copy link
Collaborator

I found some easy borrowck/Polonius performance improvements while investigating a performance regression on a development branch.

  1. Use BufReader/BufWriter when loading/saving from the polonius_cache. Otherwise bincode reads/writes each 8-byte field individually.
  2. Don't dump Polonius facts to inspect/FUNC/*.facts unless requested with C2RUST_ANALYZE_DUMP_POLONIUS_FACTS=1. This takes a nontrivial amount of time for big functions and is only needed when debugging certain borrowck issues.

Improvements compared to master (all measurements taken on the second run, after populating polonius_cache/):

  • cargo test algo_md5: 25s -> 4.6s
  • cargo test --release algo_md5: 16s -> 2.1s

@spernsteiner spernsteiner requested a review from ahomescu August 6, 2024 23:46
@spernsteiner spernsteiner merged commit d762adf into master Aug 7, 2024
9 checks passed
@kkysen kkysen deleted the analyze-borrowck-cache-bufreader branch August 8, 2024 08:01
@@ -383,7 +384,7 @@ fn run_polonius<'tcx>(
fn try_load_cached_output(facts_hash: &str) -> Option<Output> {
let path = format!("polonius_cache/{}.output", facts_hash);

let f = File::open(&path).ok()?;
let f = BufReader::new(File::open(&path).ok()?);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it faster to just read it all at once? Sometimes I've found that to be faster.

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.

3 participants