[Perf] Use a SmallVec in FromBits for Field #2772
Conversation
Signed-off-by: ljedrz <ljedrz@users.noreply.github.com>
Signed-off-by: ljedrz <ljedrz@users.noreply.github.com>
a2c25cd to
cc95fca
Compare
| @@ -47,7 +49,7 @@ impl<E: Environment> FromBits for Field<E> { | |||
| Ok(Field { field: E::Field::from_bigint(field).ok_or_else(|| anyhow!("Invalid field from bits"))? }) | |||
| } else { | |||
| // Construct the sanitized list of bits padded with `false` | |||
| let mut sanitized_bits = vec![false; size_in_bits]; | |||
| let mut sanitized_bits: SmallVec<[bool; 384]> = smallvec![false; size_in_bits]; | |||
There was a problem hiding this comment.
Why use the hardcoded value here? Is it not possible to fetch/use the size_in_bits constant?
There was a problem hiding this comment.
Sadly, it is impossible, as it's not a constant value.
There was a problem hiding this comment.
Can't we make Field::size_in_bits a const function, or does the trait need to be dyn compatible?
There was a problem hiding this comment.
It's a generic object, so you can't have a Field::<N>::size_in_bits that's const.
There was a problem hiding this comment.
What about using E::Field::SIZE_IN_BITS? Seems like size_in_bits just calls into that const.
There was a problem hiding this comment.
Sadly, that's still generic:
error: generic parameters may not be used in const operations
--> console/types/field/src/from_bits.rs:52:53
|
52 | ...SmallVec<[bool; E::Field::SIZE_IN_BITS]> = smallvec![false; size_in_bits];
| ^^^^^^^^^^^^^^^^^^^^^^ cannot perform const operation using `E`
|
= note: type parameters may not be used in const expressions
There was a problem hiding this comment.
Is it possible and a good idea to define a new const and add a unit test somewhere asserting the relevant instantiated values are the same?
There was a problem hiding this comment.
hmm, as long as we knew what to test in order to find the greatest possible number of bits, we could define a MAX_BITSIZE (currently equal to 384) and check if it's of that size
Isolated from https://github.com/ProvableHQ/snarkVM-fork/pull/3.
Field::from_bits_leis an operation that's omnipresent in heap profiles; by using aSmallVecof the right capacity, we can get rid of all those allocations. The exact scale of the perf improvement depends on the workload.