Skip to content

Avoid sorting all paths in the format command#8181

Merged
charliermarsh merged 1 commit intomainfrom
charlie/sort
Oct 24, 2023
Merged

Avoid sorting all paths in the format command#8181
charliermarsh merged 1 commit intomainfrom
charlie/sort

Conversation

@charliermarsh
Copy link
Member

@charliermarsh charliermarsh commented Oct 24, 2023

Summary

Related to #8135.

If we're not printing a --diff, or a summary of --check changes, we can avoid sorting the list of results. Further, when sorting, we only need to sort a small subset of the entries, in the common case (i.e., in general, it's much more likely that a file is formatted than not).

Test Plan

Local benchmarks suggest a 5-10% speedup on the cached behavior:

❯ hyperfine --warmup 3 "./target/release/ruff format ../airflow" "./target/release/sort format ../airflow"
Benchmark 1: ./target/release/ruff format ../airflow
  Time (mean ± σ):      70.3 ms ±   5.2 ms    [User: 52.1 ms, System: 59.0 ms]
  Range (min … max):    68.3 ms … 101.7 ms    42 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.

Benchmark 2: ./target/release/sort format ../airflow
  Time (mean ± σ):      66.0 ms ±   1.4 ms    [User: 48.3 ms, System: 58.4 ms]
  Range (min … max):    64.7 ms …  71.8 ms    44 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
  './target/release/sort format ../airflow' ran
    1.07 ± 0.08 times faster than './target/release/ruff format ../airflow'

@charliermarsh charliermarsh requested a review from zanieb October 24, 2023 20:09
@charliermarsh charliermarsh added performance Potential performance improvement formatter Related to the formatter labels Oct 24, 2023
@charliermarsh charliermarsh enabled auto-merge (squash) October 24, 2023 20:51
@charliermarsh charliermarsh merged commit 0236e07 into main Oct 24, 2023
@charliermarsh charliermarsh deleted the charlie/sort branch October 24, 2023 20:54
@github-actions
Copy link
Contributor

github-actions bot commented Oct 24, 2023

PR Check Results

Ecosystem

✅ ecosystem check detected no changes.

@konstin
Copy link
Member

konstin commented Oct 25, 2023

Thanks!

@p-linnane p-linnane mentioned this pull request Oct 26, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zanieb zanieb zanieb approved these changes

Assignees

No one assigned

Labels

formatter Related to the formatter performance Potential performance improvement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@charliermarsh @konstin @zanieb