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

Faster signatures #134

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Faster signatures #134

wants to merge 2 commits into from

Conversation

dchest
Copy link
Owner

@dchest dchest commented Nov 11, 2017

Use plain arrays instead of Float64Array.

This makes signing/verifying ~3 times faster.

On 2.6 GHz Intel Core i5 (MBPr Mid 2014), Node.js v8.8.1:

Before:

sign                                    92 ops           5.46 ms/op       183.17 ops/sec
sign.open                               48 ops          10.50 ms/op        95.26 ops/sec

After:

sign                                   269 ops           1.86 ms/op       536.52 ops/sec
sign.open                              136 ops           3.70 ms/op       270.55 ops/sec
@dchest
Copy link
Owner Author

dchest commented Mar 5, 2018

This potentially breaks constant timing. Arrays created by gf must be able to contain at least 44 bits, which is why Float64Array is used (JavaScript numbers, which are double/float64 can represent 2^53-bit integers). If we change it to Array, V8 will initially assume that these are 32-bit integers, which is why it performs faster. However, if multiplication pushes the number out of 32-bit range, V8 will have to convert the array to another representation, capable of representing more than 32-bit numbers (I assume, to doubles), thus producing a timing variation which depends on the number, which is secret data (in case of dh and signing, but not verifying).

This is just a theory, but it explains why I'm hesitant to merge this PR. I'd really like to do it, since it brings a great improvement in performance. Perhaps, someone more familiar with internals of JavaScript VMs can take a look?

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.

1 participant