Skip to content

Commit

Permalink
change: Move from maybe_rayon to rayon-1.8 (#122)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
CPerezz authored Jan 5, 2024
1 parent 0afb812 commit 94dd63e
Show file tree
Hide file tree
Showing 7 changed files with 49 additions and 35 deletions.
24 changes: 22 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,18 +32,38 @@ env:
concurrency:
group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }}
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
runs-on: ubuntu-latest
strategy:
matrix:
include:
- feature:
- feature: default
- feature: bn256-table
- feature: derive_serde
- feature: asm
steps:
- uses: actions/checkout@v2
- uses: actions-rs/toolchain@v1
Expand Down
10 changes: 7 additions & 3 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ serde_json = "1.0.105"
hex = "0.4"
rand_chacha = "0.3.1"

# 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"] }
Expand All @@ -35,11 +40,10 @@ 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"

[features]
default = ["bits", "multicore"]
multicore = ["maybe-rayon/threads"]
default = ["bits"]
asm = []
bits = ["ff/bits"]
bn256-table = []
Expand Down
13 changes: 13 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,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:
Expand Down
5 changes: 2 additions & 3 deletions src/fft.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use crate::multicore;
pub use crate::{CurveAffine, CurveExt};
use ff::Field;
use group::{GroupOpsOwned, ScalarMulOwned};
Expand Down Expand Up @@ -38,7 +37,7 @@ pub fn best_fft<Scalar: Field, G: FftGroup<Scalar>>(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);
Expand Down Expand Up @@ -107,7 +106,7 @@ pub fn recursive_butterfly_arithmetic<Scalar: Field, G: FftGroup<Scalar>>(
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),
);
Expand Down
1 change: 0 additions & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 bn256;
Expand Down
15 changes: 5 additions & 10 deletions src/msm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -155,12 +153,12 @@ pub fn small_multiexp<C: CurveAffine>(coeffs: &[C::Scalar], bases: &[C]) -> C::C
pub fn best_multiexp<C: CurveAffine>(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
Expand All @@ -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};
Expand All @@ -200,12 +195,12 @@ mod test {
fn best_multiexp<C: CurveAffine>(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
Expand Down
16 changes: 0 additions & 16 deletions src/multicore.rs

This file was deleted.

0 comments on commit 94dd63e

Please sign in to comment.