Improve performance of `BinaryNumeralString` #34

str4d · 2023-01-18T22:40:54Z

No description provided.

This allows NumeralStrings to implement the additions and subtractions more efficiently. The Numeral trait is moved into the alloc module as an extension trait on BigUint.

We don't ever want to split a numeral string arbitrarily, so this enables trait implementors to optimise for the precise kind of split that we do need.

Benchmark throughput increases on a Ryzen 9 5950X: - 10 bytes: around 215% - 100 and 1000 bytes: around 375%

We already consume `self` in `BinaryOps::{add_mod_exp, sub_mod_exp}`, and it is guaranteed to have the same byte length as the output, so we can reuse its `Vec<u8>` for the latter.

src/ff1.rs

daira · 2023-04-03T14:08:10Z

fixes #11

daira · 2023-04-03T14:30:30Z

src/ff1.rs

@@ -127,50 +127,46 @@ impl Radix {
    }
 }


Suggestion for simpler Radix::from_u32 above this:

fn from_u32(radix: u32) -> Result<Self, InvalidRadix> { // radix must be in range [2..=2^16] if !(2..=(1 << 16)).contains(&radix) { return Err(InvalidRadix(radix)); } Ok(if radix.count_ones() == 1 { let log_radix: u32 = 31 - radix.leading_zeros(); Radix::PowerTwo { radix, min_len: cmp::max((MIN_RADIX_2_NS_LEN + log_radix - 1) / log_radix, MIN_NS_LEN), log_radix: u8::try_from(log_radix).unwrap(), } } else { use libm::{ceil, log10}; let min_len = ceil(LOG_10_MIN_NS_DOMAIN_SIZE / log10(f64::from(radix))) as u32; Radix::Any { radix, min_len } }) }

with either use std::convert::TryFrom;, or edition = "2021" in Cargo.toml.

Ah yes, we can indeed use u32::count_ones and u32::leading_zeros now (they were added in 1.32, seven months after I wrote fpe 0.1 for Sapling). I also bumped MSRV to 1.56 for this upcoming release, so we can use edition = "2021" as well. I'll make both of these changes in a separate PR (as it's unrelated to the changes in this one).

src/ff1.rs

src/ff1/alloc.rs

src/ff1.rs

src/ff1/alloc.rs

ebfull

ACK, no changes from the last time I reviewed this.

daira

utACK with suggestions (I believe the unresolved ones are now just in comments).

Co-authored-by: Daira Hopwood <[email protected]>

criterion 0.4 now has an MSRV of 1.59 due to its dependencies, and pprof has an MSRV of 1.64 due to its dependencies. This is a follow-on to 0c53a91.

str4d added 3 commits January 18, 2023 22:35

Replace the Numeral trait with an Operations trait

d1e0f5a

This allows NumeralStrings to implement the additions and subtractions more efficiently. The Numeral trait is moved into the alloc module as an extension trait on BigUint.

Introduce BinaryOps struct for BinaryNumeralString computations

4a2e057

Remove u argument from NumeralString::split

acf2d7c

We don't ever want to split a numeral string arbitrarily, so this enables trait implementors to optimise for the precise kind of split that we do need.

str4d force-pushed the perf branch from 229fa16 to d68f831 Compare January 18, 2023 22:59

Rewrite BinaryNumeralString to operate over bytes instead of bits

4c3a5ab

Benchmark throughput increases on a Ryzen 9 5950X: - 10 bytes: around 215% - 100 and 1000 bytes: around 375%

str4d force-pushed the perf branch from d68f831 to 4c3a5ab Compare January 18, 2023 23:05

str4d mentioned this pull request Jan 22, 2023

Bump aes dependency to 0.8 zcash/orchard#369

Closed

str4d added 2 commits January 23, 2023 01:32

Reuse existing allocation in BinaryOps::str_radix

2cc194a

We already consume `self` in `BinaryOps::{add_mod_exp, sub_mod_exp}`, and it is guaranteed to have the same byte length as the output, so we can reuse its `Vec<u8>` for the latter.

Avoid some more allocations

2339f63