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

Parallel lints #625

Merged
merged 4 commits into from Jan 13, 2024
Merged

Parallel lints #625

merged 4 commits into from Jan 13, 2024

Conversation

jw013
Copy link
Contributor

@jw013 jw013 commented Jan 8, 2024

Closes #621.

The rayon implementation lacks the per query "Running {query}\r" dynamic messages as I couldn't think of any sensible way to do that with multiple threads.

@obi1kenobi
Copy link
Owner

Thanks, this is great!

Any chance we can tackle this portion of the linked issue in the process as well?

In particular, ensure that reported timing information accurately communicates how much time is spent generating rustdoc data (serial, blocking) and how much time is spent actually running each lint (after this change, parallel across lints) — this is covered by #298.

@jw013
Copy link
Contributor Author

jw013 commented Jan 8, 2024

Sure, I can have a go at #298 this next week. When I do, do you want me to make a new PR branch or just add to this one?

@obi1kenobi
Copy link
Owner

obi1kenobi commented Jan 8, 2024 via email

Copy link
Owner

@obi1kenobi obi1kenobi left a comment

Choose a reason for hiding this comment

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

One nitpick, otherwise this is good to go! 🚀

Thanks for putting it together!

src/check_release.rs Outdated Show resolved Hide resolved
@obi1kenobi
Copy link
Owner

obi1kenobi commented Jan 13, 2024

Based on an unscientific "I compared two CI runs of one of one of our beefier tests," GitHub Actions runners have 4 cores and the speedup is in the neighborhood of 2-2.5x. This would make sense if they have 2 physical cores with hyperthreading enabled.

I'll do some more significant tests before we ship the next release.

@obi1kenobi
Copy link
Owner

Awesome! 🚀

I'm planning to post about this PR on social media, and I'd like to give you a shout-out if you're okay with that (no pressure if not!). Lmk if you have a Mastodon / Twitter / bsky I should tag in the post?

@obi1kenobi obi1kenobi enabled auto-merge (squash) January 13, 2024 02:12
@obi1kenobi obi1kenobi merged commit b6fc8ef into obi1kenobi:main Jan 13, 2024
31 checks passed
@jw013
Copy link
Contributor Author

jw013 commented Jan 13, 2024

Awesome! 🚀

I'm planning to post about this PR on social media, and I'd like to give you a shout-out if you're okay with that (no pressure if not!). Lmk if you have a Mastodon / Twitter / bsky I should tag in the post?

That's fine with me. Outside of lurking occasionally on Mastodon I don't really use social media.

@obi1kenobi
Copy link
Owner

Happy to tag your Mastodon if you'd like (just let me know what it is). Again, no pressure!

@obi1kenobi
Copy link
Owner

Also, if you're interested in contributing more things, happy to help however I can! There's a mix of issues on the issue tracker, but I haven't done an amazing job of curating them thus far so feel free to ping me with any questions or ask for suggestions.

@jw013
Copy link
Contributor Author

jw013 commented Jan 13, 2024

Mastodon is how I found out about this project - I only use it to follow people who post interesting things about Rust and other topics, so probably not worth mentioning.

I'm happy just picking through open issues for now - what I have seen so far has been well documented and fairly easy to understand so I definitely appreciate that plus the additional feedback. Thanks!

@jw013 jw013 deleted the parallel_lints branch January 17, 2024 02:40
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.

Run lints in parallel using rayon
2 participants