Skip to content

Optimizations derived from armfazh/flor-sidh-x64 project.#2

Merged
grittygrease merged 4 commits intocloudflarearchive:masterfrom
armfazh:optime01
Nov 27, 2017
Merged

Optimizations derived from armfazh/flor-sidh-x64 project.#2
grittygrease merged 4 commits intocloudflarearchive:masterfrom
armfazh:optime01

Conversation

@armfazh
Copy link
Contributor

@armfazh armfazh commented Nov 4, 2017

Hi cloudflare,

This is PR contains some optimizations derived from flor-sidh-x64 project. Basically, this PR shows the impact of the right-to-left algorithm in the calculation of shared secret function. Also, it contains a faster formula for computing point triplings, i.e. given a point P computes [3]P.

Please consider this PR as an improvement on the performance of your Go implementation.
Best,


General optimizations

The following are general and well-known optimizations.

  1. Replace ADD+DOUBLE by a xDBLADD function.
    This reduces 1M per iteration in 3-point ladder and 2M per iteration in ScalarMult function.
  2. Print for types (useful for debugging purposes)

Optimizations derived from FLOR-SIDH-x64

The following are specific optimizations based on the FLOR-SIDH-x64 work.

  1. New formulas for point tripling.
  2. New R2L method for shared secret, replaces 3-point ladder.

Benchmark Comparison

For Shared Secret computation, the execution time was reduced by 9.5%.

benchmark old ns/op new ns/op delta
BenchmarkAliceKeyGen-4 33945573 32551477 -4.11%
BenchmarkAliceKeyGenSlow-4 289292459 286781846 -0.87%
BenchmarkBobKeyGen-4 37633036 37420870 -0.56%
BenchmarkBobKeyGenSlow-4 472687918 465635415 -1.49%
BenchmarkSharedSecretAlice-4 32414528 29297806 -9.62%
BenchmarkSharedSecretAliceSlow-4 287817189 287222653 -0.21%
BenchmarkSharedSecretBob-4 37216521 33594430 -9.73%
BenchmarkSharedSecretBobSlow-4 473442424 464861983 -1.81%

Copy link
Collaborator

@vkrasnov vkrasnov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very good work, I only have a few nits.

optimizations.md Outdated
@@ -0,0 +1,37 @@

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file should not be part of the PR

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree.

}

// Given xP = x(P), xQ = x(Q), and xPmQ = x(P-Q), compute xPaddQ = x(P+Q) and x2P = x(2P).
func xDblAdd(curve *CachedCurveParameters, xP, xQ, xPmQ *ProjectivePoint) (x2P, xPaddQ ProjectivePoint) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that should probably be a method of *CachedCurveParameters

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this function operates over points, thus now it is a method of ProjectivePoint and ProjectivePrimeFieldPoint.
Good suggestion.


// Given xP = x(P), xQ = x(Q), and xPmQ = x(P-Q), compute xPaddQ = x(P+Q) and x2P = x(2P).
// Assumes that the Z-xoordinate of PmQ is equal to 1.
func xDblAdd_primefield(aPlus2Over4 *PrimeFieldElement, xP, xQ, xPmQ *ProjectivePrimeFieldPoint) (x2P, xPaddQ ProjectivePrimeFieldPoint) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be a method too

// Given xP = x(P), xQ = x(Q), and xPmQ = x(P-Q), compute xPaddQ = x(P+Q) and x2P = x(2P).
// Assumes that the Z-xoordinate of PmQ is equal to 1.
func xDblAdd_primefield(aPlus2Over4 *PrimeFieldElement, xP, xQ, xPmQ *ProjectivePrimeFieldPoint) (x2P, xPaddQ ProjectivePrimeFieldPoint) {
var A, AA, B, BB, C, D, E, DA, CB, t0, t1 PrimeFieldElement
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could reuse temporary variables more efficiently, instead having so many of them

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also applies to xDblAdd

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now DblAdd uses more efficiently the auxiliary variables.

CB.Mul(&C, &B) // CB = C*B

t1.Add(&DA, &CB) // t1 = DA+CB
t0.Sub(&DA, &CB) // t0 = DA-CB
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For example reuse DA, instead t0, or even right into z5 (xPaddQ.X?) to avoid copying.

t1.Square(&t1) // t1 = t1^2
t0.Square(&t0) // t0 = t0^2

xPaddQ.X = t1 // z5 = z1*t1
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure if z and x in comments are mixed, or is it on purpose?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed!

}

/**
Update: This is the right-to-left method for computing the x-coordinate of P+[k]Q.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update is not a valid comment IMO. I would modify the existing description.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree with that.

package p751toolbox

import (
"reflect"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need reflect here. Please avoid it at all costs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reflect package dependency removed.

func (element Fp751Element) String() string {
var out [94]byte
element.toBytesFromMontgomeryForm(out[:])
return fmt.Sprintf("%s",PrintHex(out[:]))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent commas. Did you apply gofmt?

 - optimizations.md file removed.
 - DblAdd function now is a method of ProjectivePoint and ProjectivePrimeFieldPoint.
 - A better (re)utilization of variables inside of DblAdd method.
 - Reflect package is not required anymore in p751toolbox/print.go.
@armfazh
Copy link
Contributor Author

armfazh commented Nov 17, 2017

In 4ec6a08 you can find the modifications derived from @vkrasnov review.

Copy link
Collaborator

@vkrasnov vkrasnov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@grittygrease grittygrease merged commit 6a5b901 into cloudflarearchive:master Nov 27, 2017
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