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.

Sign up for GitHub

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

Jump to bottom

tidy should skip files based on mtime. #59884

Closed
eddyb opened this issue Apr 11, 2019 · 1 comment
Closed

tidy should skip files based on mtime. #59884

eddyb opened this issue Apr 11, 2019 · 1 comment
Labels
T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Comments

@eddyb
Copy link
Member

eddyb commented Apr 11, 2019

We can have a stamp file that tidy writes on success, and is used to determine, on subsequent runs, which files have changed since, and only read those.
I'm getting these times right now:

Building stage0 tool tidy (x86_64-unknown-linux-gnu)
    Finished release [optimized] target(s) in 0.28s
tidy check
Build completed successfully in 0:00:03

In lieu of running tidy in parallel (which AFAIK is not possible) with the build, I want to run it before the build, but this means the time until I get errors from the compiler is delayed by a few seconds.
Skipping most of the work should allow tidy to complete much faster.

cc @alexcrichton @Mark-Simulacrum
@jonas-schievink jonas-schievink added A-build T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Apr 11, 2019
@jonas-schievink jonas-schievink added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) and removed T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) A-build labels Apr 21, 2019
@Mark-Simulacrum Mark-Simulacrum mentioned this issue Jun 21, 2019
Merged
Centril added a commit to Centril/rust that referenced this issue Jun 23, 2019
@Centril
Rollup merge of rust-lang#62037 - Mark-Simulacrum:tidy-fast, r=eddyb
3d4f15e 
Speed up tidy

master:
  Time (mean ± σ):      3.478 s ±  0.033 s    [User: 3.298 s, System: 0.178 s]
  Range (min … max):    3.425 s …  3.525 s    10 runs

This PR:
  Time (mean ± σ):      1.098 s ±  0.006 s    [User: 783.7 ms, System: 310.2 ms]
  Range (min … max):    1.092 s …  1.113 s    10 runs

Alleviates rust-lang#59884. For the most part each commit stands on its own. Timings are on warm filesystem cache.

r? @eddyb
bors added a commit that referenced this issue Jun 23, 2019
@bors
Auto merge of #62037 - Mark-Simulacrum:tidy-fast, r=eddyb
de7c4e4 
Speed up tidy

master:
  Time (mean ± σ):      3.478 s ±  0.033 s    [User: 3.298 s, System: 0.178 s]
  Range (min … max):    3.425 s …  3.525 s    10 runs

This PR:
  Time (mean ± σ):      1.098 s ±  0.006 s    [User: 783.7 ms, System: 310.2 ms]
  Range (min … max):    1.092 s …  1.113 s    10 runs

Alleviates #59884. For the most part each commit stands on its own. Timings are on warm filesystem cache.

r? @eddyb
@jyn514
Copy link
Member

jyn514 commented May 20, 2023

Since #108772, tidy takes less than a second to run for me locally. I'm going to close this since I think the performance is good enough for now, but I don't mind reopening if it's still slow for you.

@jyn514 jyn514 closed this as not planned Won't fix, can't repro, duplicate, stale May 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@eddyb @jonas-schievink @jyn514