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

Minimal indexmap support for rustc-rayon #14

Merged
merged 2 commits into from
Jan 18, 2025

Conversation

cuviper
Copy link
Member

@cuviper cuviper commented Jan 13, 2025

Currently, indexmap has an optional "internal only" dependency on rustc-rayon to implement parallel iterator support for use in the compiler. This PR flips that around to have rustc-rayon implements those parallel iterators, so indexmap doesn't need to bother with this relatively niche use-case. If this is accepted and published, I intend to remove that "internal" dependency.

These implementations are all straightforward with the Slice API:

  • impl<'a, K, V, S> IntoParallelIterator for &'a IndexMap<K, V, S>
  • impl<'a, K, V, S> IntoParallelIterator for &'a mut IndexMap<K, V, S>
  • impl<'a, T, S> IntoParallelIterator for &'a IndexSet<T, S>

However, indexmap/rustc-rayon also had by-value IntoParallelIterator implementations that are not feasible without direct access to their storage. Nothing in the compiler is currently using that though, so I think it's ok to drop it.

@cuviper
Copy link
Member Author

cuviper commented Jan 13, 2025

For reference, here's how the impls currently look on the indexmap side:
https://github.com/indexmap-rs/indexmap/blob/31c9862f71e00411271acb7c9bea3a21b3d08880/src/rustc.rs

... and they use the forwarding macros found here:
https://github.com/indexmap-rs/indexmap/blob/31c9862f71e00411271acb7c9bea3a21b3d08880/src/macros.rs#L126

So those are using rayon's Vec/slice implementations and then mapping the private Bucket to the final Item type.

@cuviper cuviper merged commit 5bdcebb into rust-lang:rustc Jan 18, 2025
3 checks passed
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.

1 participant