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

Disable num-bigint-dig/u64_digit feature on wasm targets #252

Closed
jameysharp opened this issue Jan 18, 2023 · 3 comments · Fixed by #313
Closed

Disable num-bigint-dig/u64_digit feature on wasm targets #252

jameysharp opened this issue Jan 18, 2023 · 3 comments · Fixed by #313

Comments

@jameysharp
Copy link

WebAssembly doesn't have 128-bit integers, so on wasm targets LLVM implements 128-bit multiplies with a libcall to __multi3. That's slower than just doing arithmetic on smaller units.

I patched the rsa crate to remove the u64_digit feature from the num-bigint-dig dependency, and tested the patched version with the blind-rsa-signatures crate. The toy demo app I tested ran 15% faster under Wasmtime with this patch.

It would be nice if either this crate or num-bigint-dig would automatically determine what limb size to use based on the build target. Barring that, perhaps this crate should expose its own u64_digit feature that forwards to num-bigint-dig.

@tarcieri
Copy link
Member

tarcieri commented Jan 19, 2023

FWIW I've been working on a crate for automatic limb size selection based on the target which we can reuse among the many crates within @RustCrypto:

@tarcieri
Copy link
Member

@jameysharp curious what you use to benchmark WASM. It's something I've been wanting to figure out.

re: automatic limb size selection it seems like it will be tougher than I thought to pick good defaults for WASM. I have been leaning towards 64-bit though I wanted to check that via benchmarks.

Sidebar: crypto-bigint uses u128 in a few places where we want lowerings to adc/sbb on x86. Checking how that compiles on WASM in Godbolt, I don't see any use of __multi3:

https://godbolt.org/z/5b78hM1oz

tarcieri added a commit that referenced this issue Apr 26, 2023
Adds an on-by-default feature which enables `num-bigint-dig/u64_digit`.

Disabling this on 32-bit platforms (e.g. WASM) should improve
performance.

Closes #252
@tarcieri
Copy link
Member

Well, this isn't automatic, but it at least makes it possible to disable num-bigint-dig/u64_digit: #313

tarcieri added a commit that referenced this issue Apr 27, 2023
Adds an on-by-default feature which enables `num-bigint-dig/u64_digit`.

Disabling this on 32-bit platforms (e.g. WASM) should improve
performance.

Closes #252
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 a pull request may close this issue.

2 participants