Speed up tidy #105829
Speed up tidy #105829
Conversation
|
r? @jyn514 (rustbot has picked a reviewer for you, use r? to override) |
rustbot
added
T-bootstrap
| if line.is_empty() { | ||
| if i == 0 { | ||
| leading_new_lines = true; | ||
| } | ||
| trailing_new_lines += 1; | ||
| continue; | ||
| } else { | ||
| trailing_new_lines = 0; | ||
| } | ||
|
|
||
| let trimmed = line.trim(); | ||
|
|
||
| if !trimmed.starts_with("//") { | ||
| lines += 1; | ||
| } |
There was a problem hiding this comment.
Why did this move? I don't see lines / trailing_new_lines / leading_new_lines used until after where it was before.
There was a problem hiding this comment.
So we can continue earlier and skip attempting all the string processing stuff below on empty lines.
| let drain_handles = |handles: &mut VecDeque<ScopedJoinHandle<'_, ()>>| { | ||
| // poll all threads for completion before awaiting the oldest one | ||
| for i in (0..handles.len()).rev() { | ||
| if handles[i].is_finished() { | ||
| handles.swap_remove_back(i).unwrap().join().unwrap(); | ||
| } | ||
| } |
There was a problem hiding this comment.
Why is this useful? Does it run any faster or is it just releasing the resources earlier?
Oh, I guess removing a handle means you have more space in the queue to add more jobs? I'm a little confused why you would only run this once instead of putting it in the loop below ... maybe we should use rayon::ParallelIterator instead so we always join the first job to finish? All the custom concurrency code is a little hard to maintain in general.
There was a problem hiding this comment.
This is called every time in the check macro when a task is added to make room for the next task.
When I originally wrote this code (before is_finished existed) I think I wanted to add rayon but someone said it's better to keep the number of dependencies low so it's faster when one doesn't have tidy compiled.
There was a problem hiding this comment.
Hmm, the only place I see rayon mentioned in the original PR is #81833 (comment) - do you remember why you made that change?
There was a problem hiding this comment.
ah, looks like #81833 (comment). Ok, we don't need to revisit that here.
|
@bors r+ |
bors
added
S-waiting-on-bors
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#104854 (Symlink `build/host` -> `build/$HOST_TRIPLE`) - rust-lang#105458 (Allow blocking `Command::output`) - rust-lang#105559 (bootstrap: Allow installing `llvm-tools`) - rust-lang#105789 (rustdoc: clean up margin CSS for scraped examples) - rust-lang#105792 (docs: add long error explanation for error E0320) - rust-lang#105814 (Support call and drop terminators in custom mir) - rust-lang#105829 (Speed up tidy) - rust-lang#105836 (std::fmt: Use args directly in example code) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
This can be reviewed commit by commit since they contain separate optimizations.