From 58156ece4c0f13d01c71a3d84684656674953dda Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20P=C3=A9rez?= <37264926+CPerezz@users.noreply.github.com> Date: Fri, 5 Jan 2024 17:36:01 +0100 Subject: [PATCH] change: Move from `maybe_rayon` to `rayon-1.8` (#122) * 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: https://github.com/rayon-rs/rayon/pull/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 --- .github/workflows/ci.yml | 23 ++++++++++++++++++++++- Cargo.toml | 10 +++++++--- README.md | 13 +++++++++++++ src/fft.rs | 5 ++--- src/lib.rs | 1 - src/msm.rs | 15 +++++---------- src/multicore.rs | 16 ---------------- 7 files changed, 49 insertions(+), 34 deletions(-) delete mode 100644 src/multicore.rs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b309b9aa..d8ceb0f5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -34,6 +34,25 @@ concurrency: cancel-in-progress: true jobs: + compat: + if: github.event.pull_request.draft == false + name: Wasm-compatibility + runs-on: ubuntu-latest + strategy: + matrix: + target: + - wasm32-unknown-unknown + - wasm32-wasi + steps: + - uses: actions/checkout@v2 + - uses: actions-rs/toolchain@v1 + + - name: Download WASM targets + run: rustup target add "${{ matrix.target }}" + # We run WASM build (for tests) which compiles the lib allowig us to have + # `getrandom` as a dev-dependency. + - name: Build + run: cargo build --tests --release --features "bn256-table derive_serde prefetch" --target "${{ matrix.target }}" test: if: github.event.pull_request.draft == false name: Test @@ -41,8 +60,10 @@ jobs: strategy: matrix: include: - - feature: - feature: default + - feature: bn256-table + - feature: derive_serde + - feature: asm steps: - uses: actions/checkout@v2 - uses: actions-rs/toolchain@v1 diff --git a/Cargo.toml b/Cargo.toml index a5c1730e..69997dab 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,6 +19,11 @@ hex = "0.4" rand_chacha = "0.3.1" sha3 = "0.10.8" +# Added to make sure we are able to build the lib in the CI. +# Notice this will never be loaded for someone using this lib as dep. +[target.'cfg(all(target_arch = "wasm32", target_os = "unknown"))'.dev-dependencies] +getrandom = { version = "0.2", features = ["js"] } + [dependencies] subtle = "2.4" ff = { version = "0.13.0", default-features = false, features = ["std"] } @@ -36,13 +41,12 @@ serde = { version = "1.0", default-features = false, optional = true } serde_arrays = { version = "0.1.0", optional = true } hex = { version = "0.4", optional = true, default-features = false, features = ["alloc", "serde"] } blake2b_simd = "1" -maybe-rayon = { version = "0.1.0", default-features = false } +rayon = "1.8" digest = "0.10.7" sha2 = "0.10.8" [features] -default = ["bits", "multicore", "bn256-table", "derive_serde"] -multicore = ["maybe-rayon/threads"] +default = ["bits", "bn256-table", "derive_serde"] asm = [] bits = ["ff/bits"] bn256-table = [] diff --git a/README.md b/README.md index 3ede071c..a7057af1 100644 --- a/README.md +++ b/README.md @@ -13,6 +13,19 @@ The implementations were originally ported from [matterlabs/pairing](https://git * Various features related to serialization and deserialization of curve points and field elements. * Curve-specific optimizations and benchmarking capabilities. +## Controlling parallelism + +`halo2curves` currently uses [rayon](https://github.com/rayon-rs/rayon) for parallel +computation. + +The `RAYON_NUM_THREADS` environment variable can be used to set the number of +threads. + +When compiling to WASM-targets, notice that since version `1.7`, `rayon` will fallback automatically (with no need to handle features) to require `getrandom` in order to be able to work. +For more info related to WASM-compilation. + +See: [Rayon: Usage with WebAssembly](https://github.com/rayon-rs/rayon#usage-with-webassembly) for more info. + ## Benchmarks Benchmarking is supported through the use of Rust's built-in test framework. Benchmarks can be run without assembly optimizations: diff --git a/src/fft.rs b/src/fft.rs index 6eb3487e..00eca39a 100644 --- a/src/fft.rs +++ b/src/fft.rs @@ -1,4 +1,3 @@ -use crate::multicore; pub use crate::{CurveAffine, CurveExt}; use ff::Field; use group::{GroupOpsOwned, ScalarMulOwned}; @@ -38,7 +37,7 @@ pub fn best_fft>(a: &mut [G], omega: Scalar, r } - let threads = multicore::current_num_threads(); + let threads = rayon::current_num_threads(); let log_threads = threads.ilog2(); let n = a.len(); assert_eq!(n, 1 << log_n); @@ -107,7 +106,7 @@ pub fn recursive_butterfly_arithmetic>( a[1] -= &t; } else { let (left, right) = a.split_at_mut(n / 2); - multicore::join( + rayon::join( || recursive_butterfly_arithmetic(left, n / 2, twiddle_chunk * 2, twiddles), || recursive_butterfly_arithmetic(right, n / 2, twiddle_chunk * 2, twiddles), ); diff --git a/src/lib.rs b/src/lib.rs index 36f1fcda..3397043d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -3,7 +3,6 @@ pub mod ff_ext; pub mod fft; pub mod hash_to_curve; pub mod msm; -pub mod multicore; pub mod serde; pub mod bls12_381; diff --git a/src/msm.rs b/src/msm.rs index 1a3709c1..7ac24655 100644 --- a/src/msm.rs +++ b/src/msm.rs @@ -4,8 +4,6 @@ use ff::PrimeField; use group::Group; use pasta_curves::arithmetic::CurveAffine; -use crate::multicore; - fn get_booth_index(window_index: usize, window_size: usize, el: &[u8]) -> i32 { // Booth encoding: // * step by `window` size @@ -155,12 +153,12 @@ pub fn small_multiexp(coeffs: &[C::Scalar], bases: &[C]) -> C::C pub fn best_multiexp(coeffs: &[C::Scalar], bases: &[C]) -> C::Curve { assert_eq!(coeffs.len(), bases.len()); - let num_threads = multicore::current_num_threads(); + let num_threads = rayon::current_num_threads(); if coeffs.len() > num_threads { let chunk = coeffs.len() / num_threads; let num_chunks = coeffs.chunks(chunk).len(); let mut results = vec![C::Curve::identity(); num_chunks]; - multicore::scope(|scope| { + rayon::scope(|scope| { let chunk = coeffs.len() / num_threads; for ((coeffs, bases), acc) in coeffs @@ -186,10 +184,7 @@ mod test { use std::ops::Neg; - use crate::{ - bn256::{Fr, G1Affine, G1}, - multicore, - }; + use crate::bn256::{Fr, G1Affine, G1}; use ark_std::{end_timer, start_timer}; use ff::{Field, PrimeField}; use group::{Curve, Group}; @@ -200,12 +195,12 @@ mod test { fn best_multiexp(coeffs: &[C::Scalar], bases: &[C]) -> C::Curve { assert_eq!(coeffs.len(), bases.len()); - let num_threads = multicore::current_num_threads(); + let num_threads = rayon::current_num_threads(); if coeffs.len() > num_threads { let chunk = coeffs.len() / num_threads; let num_chunks = coeffs.chunks(chunk).len(); let mut results = vec![C::Curve::identity(); num_chunks]; - multicore::scope(|scope| { + rayon::scope(|scope| { let chunk = coeffs.len() / num_threads; for ((coeffs, bases), acc) in coeffs diff --git a/src/multicore.rs b/src/multicore.rs deleted file mode 100644 index d8323553..00000000 --- a/src/multicore.rs +++ /dev/null @@ -1,16 +0,0 @@ -pub use maybe_rayon::{ - iter::{IntoParallelIterator, IntoParallelRefMutIterator, ParallelIterator}, - join, scope, Scope, -}; - -#[cfg(feature = "multicore")] -pub use maybe_rayon::{ - current_num_threads, - iter::{IndexedParallelIterator, IntoParallelRefIterator}, - slice::ParallelSliceMut, -}; - -#[cfg(not(feature = "multicore"))] -pub fn current_num_threads() -> usize { - 1 -}