RFR: 8295010: Reduce if required in EC limbs operations [v5]
Xue-Lei Andrew Fan
xuelei at openjdk.org
Tue Nov 29 18:57:30 UTC 2022
On Tue, 29 Nov 2022 18:38:34 GMT, Sean Mullan <mullan at 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.
-------------
PR: https://git.openjdk.org/jdk/pull/10624
More information about the security-dev
mailing list