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.

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

fp: Unifying implementations of prime fields that fit in a single word. #1099

Merged
merged 1 commit into from
Aug 27, 2024

Conversation

armfazh
Copy link
Contributor

@armfazh armfazh commented Jul 19, 2024

It adds a new generic structure that implements field operations specialized for prime moduli that fit in a single word. Implementation is generic for basic datatypes.

Field32 is now 1.8x faster. No performance changes for Field64 and Field128.

Field32

poly_mul N Diff New Old
direct 1 1.42 38.6±0.69 ns 55.0±2.16ns
direct 120 1.88 40.1±0.28 µs 75.3±0.74µs
direct 150 1.88 158.4±1.13 µs 298.3±2.48µs
direct 255 1.89 158.3±1.82 µs 298.9±3.48µs
direct 30 1.87 2.5±0.07 µs 4.7±0.07µs
direct 60 1.85 10.3±0.20 µs 19.1±0.15µs
direct 90 1.89 40.1±0.20 µs 75.8±3.02µs
fft 1 1.26 118.3±1.34 ns 149.2±2.33ns
fft 120 1.77 13.7±0.08 µs 24.3±0.09µs
fft 150 1.82 30.0±0.59 µs 54.7±0.91µs
fft 255 1.80 29.9±0.11 µs 53.8±0.71µs
fft 30 1.69 2.8±0.07 µs 4.8±0.02µs
fft 60 1.73 6.3±0.10 µs 10.9±0.05µs
fft 90 1.78 13.7±0.08 µs 24.4±0.32µs

Copy link
Collaborator

@cjpatton cjpatton left a comment

Choose a reason for hiding this comment

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

It uses a macro since the implementation with generic types introduced a non-small performance overhead.

Interesting. What benchmarks did you see regress? Did it make Field64 slower, or just Field32?

src/fp/single_word.rs Outdated Show resolved Hide resolved
src/fp/single_word.rs Outdated Show resolved Hide resolved
src/fp/single_word.rs Outdated Show resolved Hide resolved
//! where `W` is a specified word size.

/// impl_field_single_word implements field operations for prime modulus
/// that fits in one word of `W` bits.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this true if W is u128? The mul() looks to be tailored for u64 (and maybe u32).

Copy link
Contributor

Choose a reason for hiding this comment

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

The u128 case is different because it needs multiprecision formulas, as we don't have a widening multiplication primitive from u128 to 256-bit integers. Thus, we need to say something about the $W2 type in order for the conditions to be necessary and sufficient.

Suggested change
/// that fits in one word of `W` bits.
/// that fits in one word of `W` bits. It requires a multiplication operation on unsigned integers
/// with `2*W` bits.

}
}

#[cfg(test)]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of making this code match the API of the tests, I'd suggest making the tests generic in the word size.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have updated the code and tests to rely on a generic implementation.

@armfazh armfazh force-pushed the Field_SingleWord_macro branch 2 times, most recently from 74ee996 to 4cfebf2 Compare July 27, 2024 02:16
@armfazh
Copy link
Contributor Author

armfazh commented Jul 27, 2024

Hi folks, this is ready for another round of reviews.

Cargo.toml Outdated Show resolved Hide resolved
src/fp/ops.rs Show resolved Hide resolved
src/fp/ops.rs Outdated Show resolved Hide resolved
src/fp/ops.rs Outdated Show resolved Hide resolved
src/fp/ops.rs Outdated Show resolved Hide resolved
src/fp/ops.rs Outdated Show resolved Hide resolved
src/fp/ops.rs Outdated Show resolved Hide resolved
src/fp/ops.rs Outdated Show resolved Hide resolved
src/fp/ops.rs Outdated Show resolved Hide resolved
src/fp/ops.rs Outdated Show resolved Hide resolved
This is a refactor of the FieldParameters struct
to use generic datatypes providing implementations for
primes that fit in one primitive word.

Tests script and documentation parameters were updated to
support the new structure.

Performance for FieldPrio2 operation is twice faster.
@armfazh
Copy link
Contributor Author

armfazh commented Aug 26, 2024

I have included the latest review comments and squashed all commits.

@divergentdave
Copy link
Contributor

Thanks!

@divergentdave divergentdave merged commit c842f2d into divviup:main Aug 27, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants