change: Move from maybe_rayon to rayon-1.8#122
Merged
Conversation
After seeing the issues with WASM that the last release (`0.5.0`) of this crate made, the idea is to now control the compatibility with WASM while at the same time, making it easy to handle. In this line of work, the idea was to simply do the following: - Use `rayon 1.8` as a dependency for paralellism which fixes the WASM compat issues with `multicore`-related things. See: rayon-rs/rayon#1019 thanks @han0110 for the suggestion. - Use conditional dev-dependency loading for `getrandom` such that we can compile the lib for WASM-targets in the CI without needing to have the dependency being pulled downstream. - The `multicore` module is gone, and the rest of the code has been refactored since the "fallback" is now handled by rayon itself.
6b6795f to
a96d02e
Compare
han0110
reviewed
Jan 5, 2024
cb55f40 to
d560c95
Compare
d560c95 to
c5acdaf
Compare
han0110
approved these changes
Jan 5, 2024
Contributor
han0110
left a comment
There was a problem hiding this comment.
LGTM! Just some final nits that can be ignored.
This was referenced Jan 9, 2024
Merged
kilic
reviewed
Jan 10, 2024
|
|
||
| [features] | ||
| default = ["bits", "multicore"] | ||
| multicore = ["maybe-rayon/threads"] |
Contributor
There was a problem hiding this comment.
[[bench]]
name = "msm"
harness = false
required-features = ["multicore"]
Seems like we had a leftover feature for msm benchs
Contributor
Author
There was a problem hiding this comment.
Ohh dammit!!
Will update. Hopefully this will not affect the released crate.
jonathanpwang
pushed a commit
to axiom-crypto/halo2curves
that referenced
this pull request
Apr 19, 2024
* change: Move from `maybe_rayon` to `rayon-1.8` After seeing the issues with WASM that the last release (`0.5.0`) of this crate made, the idea is to now control the compatibility with WASM while at the same time, making it easy to handle. In this line of work, the idea was to simply do the following: - Use `rayon 1.8` as a dependency for paralellism which fixes the WASM compat issues with `multicore`-related things. See: rayon-rs/rayon#1019 thanks @han0110 for the suggestion. - Use conditional dev-dependency loading for `getrandom` such that we can compile the lib for WASM-targets in the CI without needing to have the dependency being pulled downstream. - The `multicore` module is gone, and the rest of the code has been refactored since the "fallback" is now handled by rayon itself. * change: Update CI to account for WASM compat * chore: Add paralellism section to README * chore: Fix CI missing " * chore: Split WASM build & add targets * chore: Test all features for regular-target builds * chore: Address review nits
jonathanpwang
pushed a commit
to axiom-crypto/halo2curves
that referenced
this pull request
Apr 24, 2024
* change: Move from `maybe_rayon` to `rayon-1.8` After seeing the issues with WASM that the last release (`0.5.0`) of this crate made, the idea is to now control the compatibility with WASM while at the same time, making it easy to handle. In this line of work, the idea was to simply do the following: - Use `rayon 1.8` as a dependency for paralellism which fixes the WASM compat issues with `multicore`-related things. See: rayon-rs/rayon#1019 thanks @han0110 for the suggestion. - Use conditional dev-dependency loading for `getrandom` such that we can compile the lib for WASM-targets in the CI without needing to have the dependency being pulled downstream. - The `multicore` module is gone, and the rest of the code has been refactored since the "fallback" is now handled by rayon itself. * change: Update CI to account for WASM compat * chore: Add paralellism section to README * chore: Fix CI missing " * chore: Split WASM build & add targets * chore: Test all features for regular-target builds * chore: Address review nits
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
After seeing the issues with WASM that the last release (
0.5.0) of this crate made, the idea is to now control the compatibility with WASM while at the same time, making it easy to handle.In this line of work, the idea was to simply do the following:
rayon 1.8as a dependency for paralellism which fixes the WASM compat issues withmulticore-related things. See: Add a fallback when threading is unsupported rayon-rs/rayon#1019 thanks @han0110 for the suggestion.getrandomsuch that we can compile the lib for WASM-targets in the CI without needing to have the dependency being pulled downstream.multicoremodule is gone, and the rest of the code has been refactored since the "fallback" is now handled by rayon itself.wasmiandwasm32targets on each CI run. (It does so by targetingtestbuild such that we can getgetrandomas a dev-dependency which will not get pulled on consumer's machines.