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.

Sign up for GitHub

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

Fix the missing parallel feature flag for ark-serialize #833

Closed
wants to merge 3 commits into from
Closed

Fix the missing parallel feature flag for ark-serialize #833

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c9077a8
fix the missing feature flag
weikengchen May 29, 2024
3a265f9
add the rayon dependency
weikengchen May 29, 2024
716b728
fix the parallel
weikengchen May 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion bench-templates/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,4 @@ paste.workspace = true

[features]
asm = [ "ark-ff/asm" ]
parallel = [ "ark-std/parallel", "ark-ff/parallel", "ark-ec/parallel", ]
parallel = [ "ark-std/parallel", "ark-ff/parallel", "ark-ec/parallel", "ark-serialize/parallel" ]
2 changes: 1 addition & 1 deletion ec/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,4 +41,4 @@ hex.workspace = true
[features]
default = []
std = [ "ark-std/std", "ark-ff/std", "ark-serialize/std" ]
parallel = [ "std", "rayon", "ark-std/parallel" ]
parallel = [ "std", "rayon", "ark-std/parallel", "ark-serialize/parallel" ]
10 changes: 9 additions & 1 deletion ec/src/models/short_weierstrass/affine.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use ark_serialize::{
CanonicalDeserialize, CanonicalSerialize, Compress, SerializationError, Valid, Validate,
batch_check_helper, CanonicalDeserialize, CanonicalSerialize, Compress, SerializationError,
Valid, Validate,
};
use ark_std::{
borrow::Borrow,
Expand Down Expand Up @@ -382,6 +383,13 @@ impl<P: SWCurveConfig> Valid for Affine<P> {
Err(SerializationError::InvalidData)
}
}

fn batch_check<'a>(batch: impl Iterator<Item = &'a Self>) -> Result<(), SerializationError>
where
Self: 'a,
{
batch_check_helper(batch)
}
}

impl<P: SWCurveConfig> CanonicalDeserialize for Affine<P> {
Expand Down
10 changes: 9 additions & 1 deletion ec/src/models/twisted_edwards/affine.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use ark_serialize::{
CanonicalDeserialize, CanonicalSerialize, Compress, SerializationError, Valid, Validate,
batch_check_helper, CanonicalDeserialize, CanonicalSerialize, Compress, SerializationError,
Valid, Validate,
};
use ark_std::{
borrow::Borrow,
Expand Down Expand Up @@ -339,6 +340,13 @@ impl<P: TECurveConfig> Valid for Affine<P> {
Err(SerializationError::InvalidData)
}
}

fn batch_check<'a>(batch: impl Iterator<Item = &'a Self>) -> Result<(), SerializationError>
where
Self: 'a,
{
batch_check_helper(batch)
}
}

impl<P: TECurveConfig> CanonicalDeserialize for Affine<P> {
Expand Down
10 changes: 9 additions & 1 deletion ec/src/pairing.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use ark_ff::{AdditiveGroup, CyclotomicMultSubgroup, Field, One, PrimeField};
use ark_serialize::{
CanonicalDeserialize, CanonicalSerialize, Compress, SerializationError, Valid, Validate,
batch_check_helper, CanonicalDeserialize, CanonicalSerialize, Compress, SerializationError,
Valid, Validate,
};
use ark_std::{
borrow::Borrow,
Expand Down Expand Up @@ -165,6 +166,13 @@ impl<P: Pairing> Valid for PairingOutput<P> {
Err(SerializationError::InvalidData)
}
}

fn batch_check<'a>(batch: impl Iterator<Item = &'a Self>) -> Result<(), SerializationError>
where
Self: 'a,
{
batch_check_helper(batch)
}
}

impl<P: Pairing> CanonicalDeserialize for PairingOutput<P> {
Expand Down
2 changes: 1 addition & 1 deletion ff/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,5 +44,5 @@ hex.workspace = true
[features]
default = []
std = [ "ark-std/std", "ark-serialize/std", "itertools/use_std" ]
parallel = [ "std", "rayon", "ark-std/parallel" ]
parallel = [ "std", "rayon", "ark-std/parallel", "ark-serialize/parallel" ]
asm = []
4 changes: 2 additions & 2 deletions poly/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ criterion = "0.5.1"

[features]
default = []
std = [ "ark-std/std", "ark-ff/std" ]
parallel = [ "std", "ark-ff/parallel", "rayon", "ark-std/parallel" ]
std = [ "ark-std/std", "ark-ff/std", "ark-serialize/std" ]
parallel = [ "std", "ark-ff/parallel", "rayon", "ark-std/parallel", "ark-serialize/parallel" ]


[[bench]]
Expand Down
4 changes: 3 additions & 1 deletion serialize/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ ark-serialize-derive = { workspace = true, optional = true }
ark-std.workspace = true
digest.workspace = true
num-bigint.workspace = true
rayon = { workspace = true, optional = true }

[dev-dependencies]
sha2.workspace = true
Expand All @@ -30,5 +31,6 @@ ark-test-curves = { workspace = true, default-features = false, features = [ "bl

[features]
default = []
std = [ "ark-std/std", ]
parallel = [ "rayon" ]
std = [ "ark-std/std" ]
derive = [ "ark-serialize-derive" ]
37 changes: 27 additions & 10 deletions serialize/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,16 +69,8 @@ pub trait Valid: Sized {
where
Self: 'a,
{
#[cfg(feature = "parallel")]
{
use rayon::{iter::ParallelBridge, prelude::ParallelIterator};
batch.par_bridge().try_for_each(|e| e.check())?;
}
#[cfg(not(feature = "parallel"))]
{
for item in batch {
item.check()?;
}
for item in batch {
item.check()?;
}
Ok(())
}
Expand Down Expand Up @@ -241,3 +233,28 @@ pub fn buffer_bit_byte_size(modulus_bits: usize) -> (usize, usize) {
pub const fn buffer_byte_size(modulus_bits: usize) -> usize {
(modulus_bits + 7) / 8
}

pub fn batch_check_helper<'a, T>(
batch: impl Iterator<Item = &'a T>,
) -> Result<(), SerializationError>
where
T: 'a + Valid + Send + Clone,
{
#[cfg(feature = "parallel")]
{
use ark_std::vec::Vec;
use rayon::prelude::{IntoParallelIterator, ParallelIterator};
batch
.map(|x| (*x).clone())
.collect::<Vec<T>>()
.into_par_iter()
.try_for_each(|e| e.check())?;
}
Comment on lines +247 to +252
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does the par_bridge solution not work here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ParallelBridge is implemented for Iterator + Send. We no longer have Send bound on batch here. So we'd need to put an explicit bound: batch: impl Iterator<Item = &'a T> + Send, but this propagates all the way up to Valid, and would in turn lead to two definitions of batch_check:

    #[cfg(feature = "parallel")]
    fn batch_check<'a>(batch: impl Iterator<Item = &'a Self> + ParallelBridge)...

    #[cfg(not(feature = "parallel"))]
    fn batch_check<'a>(batch: impl Iterator<Item = &'a Self>) ...

#[cfg(not(feature = "parallel"))]
{
for item in batch {
item.check()?;
}
}
Ok(())
}
Loading