[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-graal-dev
Subject:    Re: RFR: 8329538: Accelerate P256 on x86_64 using Montgomery intrinsic [v12]
From:       Tobias Hartmann <thartmann () openjdk ! org>
Date:       2024-05-22 5:03:15
Message-ID: lY7vMCPrX-Mtk1_NGcc2sDh05tuKtafmgLpGwq6iRs0=.e98249d2-c313-4a6b-9ad2-f5546f59743a () github ! com
[Download RAW message or body]

On Tue, 21 May 2024 17:41:46 GMT, Volodymyr Paprotski <duke@openjdk.org> wrote:

> > Performance. Before:
> > 
> > Benchmark                        (algorithm)  (dataSize)  (keyLength)  (provider) \
> > Mode  Cnt     Score    Error  Units SignatureBench.ECDSA.sign    SHA256withECDSA  \
> > 1024          256              thrpt    3  6443.934  ±  6.491  ops/s \
> > SignatureBench.ECDSA.sign    SHA256withECDSA       16384          256             \
> > thrpt    3  6152.979  ±  4.954  ops/s SignatureBench.ECDSA.verify  \
> > SHA256withECDSA        1024          256              thrpt    3  1895.410  ± \
> > 36.979  ops/s SignatureBench.ECDSA.verify  SHA256withECDSA       16384          \
> > 256              thrpt    3  1878.955  ± 45.487  ops/s Benchmark                  \
> > (algorithm)  (keyLength)  (kpgAlgorithm)  (provider)   Mode  Cnt     Score    \
> > Error  Units o.o.b.j.c.full.KeyAgreementBench.EC.generateSecret          ECDH     \
> > 256              EC              thrpt    3  1357.810  ± 26.584  ops/s \
> > o.o.b.j.c.small.KeyAgreementBench.EC.generateSecret         ECDH          256     \
> > EC              thrpt    3  1352.119  ± 23.547  ops/s Benchmark                   \
> > (isMontBench)   Mode  Cnt     Score    Error  Units \
> > PolynomialP256Bench.benchMultiply          false  thrpt    3  1746.126  ± 10.970  \
> > ops/s 
> > Performance, no intrinsic:
> > 
> > Benchmark                        (algorithm)  (dataSize)  (keyLength)  (provider) \
> > Mode  Cnt     Score     Error  Units SignatureBench.ECDSA.sign    SHA256withECDSA \
> > 1024          256              thrpt    3  6529.839  ±  42.420  ops/s \
> > SignatureBench.ECDSA.sign    SHA256withECDSA       16384          256             \
> > thrpt    3  6199.747  ± 133.566  ops/s SignatureBench.ECDSA.verify  \
> > SHA256withECDSA        1024          256              thrpt    3  1973.676  ±  \
> > 54.071  ops/s SignatureBench.ECDSA.verify  SHA256withECDSA       16384          \
> > 256              thrpt    3  1932.127  ±  35.920  ops/s Benchmark                 \
> > (algorithm)  (keyLength)  (kpgAlgorithm)  (provider)   Mode  Cnt     Score    \
> > Error  Units o.o.b.j.c.full.KeyAgreementBench.EC.generateSecret          ECDH     \
> > 256              EC              thrpt    3  1355.788  ± 29.858  ops/s \
> > o.o.b.j.c.small.KeyAgreementBench.EC.generateSecret         ECDH          256     \
> > EC              thrpt    3  1346.523  ± 28.722  ops/s Benchmark                   \
> > (isMontBench)   Mode  Cnt     Score    Error  Units \
> > PolynomialP256Bench.benchMultiply           true  thrpt    3  1919.57...
> 
> Volodymyr Paprotski has updated the pull request with a new target base due to a \
> merge or a rebase. The incremental webrev excludes the unrelated changes brought in \
> by the merge/rebase. The pull request contains 17 additional commits since the last \
> revision: 
> - Merge remote-tracking branch 'origin/master' into ecc-montgomery
> - shenandoah verifier
> - comments from Sandhya
> - whitespace
> - add message back
> - whitespace
> - Use AffinePoint to exit Montgomery domain
> 
> Style notes:
> Affine.equals()
> - Mismatched fields only appear to be used from testing, perhaps should be moved \
> there instead Affine.getX(boolean)|getY(boolean)
> - "Passing flag is bad design" - cleanest/performant alternative to several \
>                 instanceof checks
> - needed to convert Affine to Projective (need to stay in montgomery domain)
> ECOperations.PointMultiplier
> - changes could probably be restored to original (since ProjectivePoint handling no \
>                 longer required)
> - consider these changes an improvement? (fewer nested classes)
> - was an inner-class but not using inner-class features (i.e. ecOps variable should \
>                 be converted)
> - whitespace
> - Comments from Tony and Jatin
> - Comments from Jatin and Tony
> - ... and 7 more: https://git.openjdk.org/jdk/compare/9ee91a9f...b1a33004

Thanks! I submitted testing and will report back once it passed.

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

PR Comment: https://git.openjdk.org/jdk/pull/18583#issuecomment-2123869579


[prev in list] [next in list] [prev in thread] [next in thread]