> Hi,
> 
> May I have this patch reviewed?
> 
> This is one of a few steps to improve the EC performance. The multiplicative 
> inverse implementation could be improved for better performance. 
> 
> For secp256r1 prime p, the current  multiplicative inverse impl needs 256 
> square and 128 multiplication.  With the path, the operation needs 256 square 
> and 13 multiplication.
> 
> For secp256r1 order n, the current  multiplicative inverse impl needs 256 
> square and 169 multiplication. With the patch, the operation needs  256 
> square and 43 multiplication.
> 
> In EC operations, square operation is much faster than multiplication.  
> Decreasing multiplication numbers could speed up the multiplicative inverse 
> significantly.
> 
> The benchmark for ECDSA Signature is checked in order to see if the 
> performance improvement is visible.  Here are the benchmark numbers before 
> the patch applied:
> 
> Benchmark        (messageLength)   Mode  Cnt     Score    Error  Units
> Signatures.sign               64  thrpt   15  1412.644 ±  5.529  ops/s
> Signatures.sign              512  thrpt   15  1407.711 ± 14.118  ops/s
> Signatures.sign             2048  thrpt   15  1415.674 ±  6.965  ops/s
> Signatures.sign            16384  thrpt   15  1395.582 ± 12.689  ops/s
> 
> 
> And the following are the benchmarking after the patch applied.
> 
> Signatures.sign               64  thrpt   15  1484.404 ± 10.705  ops/s
> Signatures.sign              512  thrpt   15  1486.563 ±  7.514  ops/s
> Signatures.sign             2048  thrpt   15  1479.866 ± 15.028  ops/s
> Signatures.sign            16384  thrpt   15  1469.789 ±  3.844  ops/s
> 
> 
> The performance improvement of the patch is about 5% for ECDSA signature.  It 
> looks like the improvement is no significant enough for now.  But it may be 
> 2+ times more in numbers when the scalar multiplication implementation is 
> improved in a follow-up enhancement in another pull request.
> 
> For comparing, here is the benchmarking numbers by using 
> BigInteger.modInverse();
> 
> Benchmark        (messageLength)   Mode  Cnt     Score     Error  Units
> Signatures.sign               64  thrpt   15  1395.628 ± 180.649  ops/s
> Signatures.sign              512  thrpt   15  1510.590 ±   9.826  ops/s
> Signatures.sign             2048  thrpt   15  1514.282 ±   3.382  ops/s
> Signatures.sign            16384  thrpt   15  1497.325 ±   6.854  ops/s
> 
> and numbers for using BigInteger.modPow():
> 
> Benchmark        (messageLength)   Mode  Cnt     Score    Error  Units
> Signatures.sign               64  thrpt   15  1486.764 ± 17.908  ops/s
> Signatures.sign              512  thrpt   15  1494.801 ± 14.072  ops/s
> Signatures.sign             2048  thrpt   15  1500.170 ±  6.998  ops/s
> Signatures.sign            16384  thrpt   15  1434.192 ± 49.269  ops/s
> 
> 
> Enhancement for other curves will be considered in separate pull requests.
> 
> Thanks,
> Xuelei

Xue-Lei Andrew Fan has updated the pull request incrementally with one 
additional commit since the last revision:

  more improvement

-------------

Changes:
  - all: https://git.openjdk.org/jdk/pull/10544/files
  - new: https://git.openjdk.org/jdk/pull/10544/files/b9bf0269..ae1df949

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=10544&range=02
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=10544&range=01-02

  Stats: 93 lines in 1 file changed: 73 ins; 1 del; 19 mod
  Patch: https://git.openjdk.org/jdk/pull/10544.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/10544/head:pull/10544

PR: https://git.openjdk.org/jdk/pull/10544

Reply via email to