Speedups for Uint::sqrt, Uint::sqrt_vartime

[andrewwhitehead]

These changes optimize Uint::sqrt by operating on a non-zero input, reducing the number of conditional selects. The initial division operation for both sqrt and sqrt_vartime is replaced by a bit shift. A fast path is added for sqrt_vartime when the input fits within a Limb.

Sample benchmarks:

sqrt/sqrt, U256         time:   [1.1534 µs 1.1645 µs 1.1797 µs]
                        change: [−17.817% −16.320% −15.076%] (p = 0.00 < 0.05)
                        Performance has improved.
sqrt/sqrt, U512         time:   [2.3950 µs 2.4117 µs 2.4333 µs]
                        change: [−17.119% −16.077% −15.184%] (p = 0.00 < 0.05)
                        Performance has improved.
sqrt/sqrt_vartime, U256 time:   [661.32 ns 666.22 ns 672.73 ns]
                        change: [−15.649% −14.029% −11.949%] (p = 0.00 < 0.05)
                        Performance has improved.
sqrt/sqrt_vartime, U256 one Limb
                        time:   [8.8326 ns 9.0810 ns 9.3029 ns]
                        change: [−97.465% −97.382% −97.302%] (p = 0.00 < 0.05)
                        Performance has improved.

[codecov]

Codecov Report

❌ Patch coverage is 76.92308% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.76%. Comparing base (04eadf4) to head (3d872a3).
⚠️ Report is 5 commits behind head on master.

Files with missing lines	Patch %	Lines
src/uint/sqrt.rs	76.92%	9 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1078      +/-   ##
==========================================
- Coverage   78.76%   78.76%   -0.01%     
==========================================
  Files         173      173              
  Lines       17741    17750       +9     
==========================================
+ Hits        13974    13980       +6     
- Misses       3767     3770       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[andrewwhitehead]

It might be clearer having the methods named isqrt and isqrt_vartime, as a user might not realize they need to use checked_sqrt to verify the result.

[tarcieri]

@andrewwhitehead or sqrt_unchecked, where sqrt checks the result and computes a CtOption?

[andrewwhitehead]

@tarcieri There is precedent for isqrt in the standard library now (in 1.84 and used here). I'm not sure what that does to the SquareRoot trait, though. I should probably create an issue but for completeness:

• Should OddUint implement sqrt/sqrt_vartime? It would need to change the return type, but it seems reasonable to add an Output type to the trait.
• If exact square roots (in some kind of fixed point I guess) were ever added, then the SquareRoot name would probably need to change. Maybe UintSquareRoot?
• Int could only implement a checked square root, but maybe CheckedSquareRoot would be a desirable trait too (supertrait of SquareRoot?).
• nth roots also look straightforward to implement based on the same source, but I'm not sure if that's a separate trait or a method on the same trait(s)

[tarcieri]

@andrewwhitehead those all sound like reasonably good ideas to me. Do you want to open a separate issue for it?

It would also be interesting to impl such traits for modular sqrt, at least for primefield::MontyFieldElement, though potentially *MontyForm (though the generic Tonelli-Shanks implementation in primefield relies on constants computed from the multiplicative generator, so upstreaming it wouldn't be trivial)