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

Rust sorting benchmark #24302

Merged
merged 8 commits into from Feb 5, 2024
Merged

Rust sorting benchmark #24302

merged 8 commits into from Feb 5, 2024

Conversation

mo8it
Copy link
Contributor

@mo8it mo8it commented Jan 31, 2024

On my machine, Rust has almost the same performance now. Please update your blog post for a better comparison.

Why would you use the rand crate for random number generation and not rayon for parallelism?

rayon is written in 100% Rust. About the "Standard library" thing: Rust by design keeps its standard library as small as possible. Major crates like rayon are often maintained by the Rust maintainers themself. For rayon, the two main maintainers Josh Stone and Niko Matsakis are core Rust contributors. Niko is even a team leader of the language team: https://www.rust-lang.org/governance

@mppf
Copy link
Member

mppf commented Jan 31, 2024

@mo8it, thanks for making a Rust+Rayon version of the benchmark.

Why would you use the rand crate for random number generation and not rayon for parallelism?

Because the benchmark is about sorting, not about generating random numbers. So, the test programs can generate random numbers in any old way.

However, your comments here have pointed out to me that the article shouldn’t claim that the Chapel random number generation is 17x faster than one in Rust since the Rust code I measured was generating the random numbers with a crate (vs a standard library) and using different crates can make it faster (as you show here). So I'll update that piece.

Please update your blog post for a better comparison.

I agree that Rust+Rayon is an interesting comparison. At the same time, I think that we can all agree that Rayon isn’t in Rust’s standard library. There are all kinds of interesting sort implementations in the world that aren’t in standard libraries & these are not what the article is about.

In particular with Rust, as you say, there is a design goal to have a small standard library. But, in this case, I think that makes it harder for newcomers to use parallelism. For one thing, they have to start by choosing between Rayon and Crossbeam. I think that something being in a standard library makes it more valuable and accessible and that’s why the article focuses on standard libraries. Of course, you are free to disagree with my position on the importance of standard libraries.

That said, I’d like to include your Rust+Rayon benchmark among the other benchmarks as an alternative rather than a replacement for the original. If you want to help me do that, there will be some updates that are needed to match our contributing guidelines. Please see https://chapel-lang.org/docs/latest/developer/bestPractices/DCO.html#best-practices-dco for the most immediate step, and in addition, it’ll be necessary to change the PR to add a separate test/library/packages/Sort/performance/comparison/sort-random-rust-rayon/ directory to include your implementation.

@mppf
Copy link
Member

mppf commented Jan 31, 2024

I've created a Discourse topic for the blog post being discussed here: https://chapel.discourse.group/t/comparing-standard-library-sorts-the-impact-of-parallelism/30411 . That discourse group is a better place for discussion about the blog post itself.

Signed-off-by: mo8it <mo8it@proton.me>
Signed-off-by: mo8it <mo8it@proton.me>
Signed-off-by: mo8it <mo8it@proton.me>
Signed-off-by: mo8it <mo8it@proton.me>
Signed-off-by: mo8it <mo8it@proton.me>
Signed-off-by: mo8it <mo8it@proton.me>
Signed-off-by: mo8it <mo8it@proton.me>
@mo8it
Copy link
Contributor Author

mo8it commented Feb 5, 2024

For one thing, they have to start by choosing between Rayon and Crossbeam.

Rayon and Crossbeam do completely different things and aren't interchangeable.

I signed my commits and separated the rayon sorting.

@mppf mppf merged commit 6980ca9 into chapel-lang:main Feb 5, 2024
7 checks passed
mppf added a commit that referenced this pull request Feb 5, 2024
Follow-up to PR #24302 to adjust
`test/library/packages/Sort/performance/comparison`
* use a subdirectory .gitignore since that is preferred to adjusting the
top-level one
* include the parallel array generation as a commented-out alternative
to keep the blog performance reproducible
* comment out some cargo configuration that makes it slower on the
benchmark system


Test change only, not reviewed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants