On Tue, 29 Nov 2022 18:38:34 GMT, Sean Mullan <mul...@openjdk.org> wrote:
>>> I may run it again after the integration of multiplicative inversion and >>> point multiplication improvement. >> >> After the integration of the improvement above, here is the benchmark >> numbers with this patch: >> >> Benchmark (algorithm) (messageLength) Mode Cnt >> Score Error Units >> Signatures.EdDSA.sign Ed25519 64 thrpt 15 >> 1084.556 ± 135.637 ops/s >> Signatures.EdDSA.sign Ed25519 512 thrpt 15 >> 1168.663 ± 25.072 ops/s >> Signatures.EdDSA.sign Ed25519 2048 thrpt 15 >> 1186.863 ± 16.224 ops/s >> Signatures.EdDSA.sign Ed25519 16384 thrpt 15 >> 1095.034 ± 6.462 ops/s >> Signatures.EdDSA.sign Ed448 64 thrpt 15 >> 323.771 ± 2.156 ops/s >> Signatures.EdDSA.sign Ed448 512 thrpt 15 >> 326.995 ± 2.101 ops/s >> Signatures.EdDSA.sign Ed448 2048 thrpt 15 >> 320.799 ± 5.452 ops/s >> Signatures.EdDSA.sign Ed448 16384 thrpt 15 >> 317.715 ± 2.554 ops/s >> Signatures.sign secp256r1 64 thrpt 15 >> 4072.636 ± 22.441 ops/s >> Signatures.sign secp256r1 512 thrpt 15 >> 4048.822 ± 40.769 ops/s >> Signatures.sign secp256r1 2048 thrpt 15 >> 4042.884 ± 20.147 ops/s >> Signatures.sign secp256r1 16384 thrpt 15 >> 3911.856 ± 12.039 ops/s >> Signatures.sign secp384r1 64 thrpt 15 >> 634.203 ± 4.532 ops/s >> Signatures.sign secp384r1 512 thrpt 15 >> 637.623 ± 1.592 ops/s >> Signatures.sign secp384r1 2048 thrpt 15 >> 620.283 ± 10.014 ops/s >> Signatures.sign secp384r1 16384 thrpt 15 >> 622.617 ± 5.695 ops/s >> Signatures.sign secp521r1 64 thrpt 15 >> 311.957 ± 5.420 ops/s >> Signatures.sign secp521r1 512 thrpt 15 >> 316.605 ± 2.204 ops/s >> Signatures.sign secp521r1 2048 thrpt 15 >> 316.700 ± 1.654 ops/s >> Signatures.sign secp521r1 16384 thrpt 15 >> 309.599 ± 7.167 ops/s >> >> >> and the numbers without this patch: >> >> Benchmark (algorithm) (messageLength) Mode Cnt >> Score Error Units >> Signatures.EdDSA.sign Ed25519 64 thrpt 15 >> 1138.578 ± 57.908 ops/s >> Signatures.EdDSA.sign Ed25519 512 thrpt 15 >> 1172.242 ± 17.180 ops/s >> Signatures.EdDSA.sign Ed25519 2048 thrpt 15 >> 1163.793 ± 21.095 ops/s >> Signatures.EdDSA.sign Ed25519 16384 thrpt 15 >> 1093.856 ± 5.964 ops/s >> Signatures.EdDSA.sign Ed448 64 thrpt 15 >> 324.089 ± 2.894 ops/s >> Signatures.EdDSA.sign Ed448 512 thrpt 15 >> 323.580 ± 1.437 ops/s >> Signatures.EdDSA.sign Ed448 2048 thrpt 15 >> 323.680 ± 2.555 ops/s >> Signatures.EdDSA.sign Ed448 16384 thrpt 15 >> 310.641 ± 2.256 ops/s >> Signatures.sign secp256r1 64 thrpt 15 >> 4070.733 ± 27.059 ops/s >> Signatures.sign secp256r1 512 thrpt 15 >> 4061.835 ± 18.776 ops/s >> Signatures.sign secp256r1 2048 thrpt 15 >> 4041.226 ± 19.082 ops/s >> Signatures.sign secp256r1 16384 thrpt 15 >> 3893.323 ± 11.869 ops/s >> Signatures.sign secp384r1 64 thrpt 15 >> 632.924 ± 8.273 ops/s >> Signatures.sign secp384r1 512 thrpt 15 >> 628.807 ± 7.604 ops/s >> Signatures.sign secp384r1 2048 thrpt 15 >> 631.052 ± 1.782 ops/s >> Signatures.sign secp384r1 16384 thrpt 15 >> 530.402 ± 122.967 ops/s >> Signatures.sign secp521r1 64 thrpt 15 >> 316.634 ± 1.724 ops/s >> Signatures.sign secp521r1 512 thrpt 15 >> 315.830 ± 2.333 ops/s >> Signatures.sign secp521r1 2048 thrpt 15 >> 315.855 ± 1.093 ops/s >> Signatures.sign secp521r1 16384 thrpt 15 >> 315.019 ± 1.124 ops/s > > @XueleiFan, I have some questions about this integration. The performance > numbers you last posted showed some very small improvements but also some > regressions in throughput numbers, so its not clear to me if this is worth > integrating yet. Earlier, you said that the performance benefits of this fix > would not be realized until the changes for > https://github.com/openjdk/jdk/pull/10398 were to be integrated, but that > change is still in draft and has comments that have not been resolved. So, is > it possible this was integrated too early? > @seanjmullan if you check the benchmark numbers, the throughput impact is > limited. For some curves, it is improved; and some others is impacted. In > theory of this update, it is not expected to have performance impact if no > improvement. The throughput impact in the benchmark is more from the noice in > the benchmark platform, I think. > > The fix for [10398](https://github.com/openjdk/jdk/pull/10398) depends on > this update. Only after integrating this one, I can open > [10398](https://github.com/openjdk/jdk/pull/10398) for review. The code is > ready for [10398](https://github.com/openjdk/jdk/pull/10398) , I'm running > the benchmarking. Once the benchmark numbers are ready, I will open it for > review. It should be ready to review today. Ok, thanks for the update. Will keep an eye out for the updated benchmark numbers. ------------- PR: https://git.openjdk.org/jdk/pull/10624
