Skip to content

Force inline mul_wide() and reduce() to get rid of performance regression#166

Merged
tarcieri merged 1 commit into
RustCrypto:masterfrom
fjarri:scalar-mul-inline
Sep 5, 2020
Merged

Force inline mul_wide() and reduce() to get rid of performance regression#166
tarcieri merged 1 commit into
RustCrypto:masterfrom
fjarri:scalar-mul-inline

Conversation

@fjarri

@fjarri fjarri commented Sep 4, 2020

Copy link
Copy Markdown
Contributor

Noticed while checking performance in PR #164 that scalar mul() became slower. Seems to be caused by the compiler ceasing to inline mul_wide() and reduce() starting from commit 0586693 on.

Adding this makes Scalar4x64::mul() 10% faster, but has no effect on Scalar8x32 performance on my machine; nevertheless, I inlined these functions there as well for the sake of uniformity (may be important for a 32-bit embedded device). These functions are only used once (in mul()), so it won't degrade compilation.

I usually prefer not to add explicit inlining, but 10% is a pretty solid speed-up...

…sion

They are only used in Scalar[8x32/4x64]::mul() internally,
so it will not cause binary bloat.
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

Merging #166 into master will decrease coverage by 0.11%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #166      +/-   ##
==========================================
- Coverage   60.25%   60.14%   -0.12%     
==========================================
  Files          25       25              
  Lines        3555     3555              
==========================================
- Hits         2142     2138       -4     
- Misses       1413     1417       +4     
Impacted Files Coverage Δ
k256/src/arithmetic/scalar/scalar_4x64.rs 0.00% <ø> (ø)
k256/src/arithmetic/scalar/scalar_8x32.rs 92.45% <ø> (-0.87%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 23def1e...be18643. Read the comment docs.

@tarcieri tarcieri merged commit e30f84e into RustCrypto:master Sep 5, 2020
@fjarri fjarri deleted the scalar-mul-inline branch July 18, 2021 20:59
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