Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # 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
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants