Thanks for adding in AES/256. I think your rationale for DH is fine, it
was something that I was more curious about than concerned. The
-pkeyLength option gives the benchmarks the flexibility we need if we
want to try larger or smaller keys. I don't think we need to include
other sizes by default.
Getting some feedback from a JMH expert is definitely a good idea. I'll
be interested to see what feedback and suggestions they have to offer,
if any.
Thanks,
--Jamil
On 12/20/2018 7:53 AM, Adam Petcher wrote:
updated webrev: http://cr.openjdk.java.net/~apetcher/8215643/webrev.01/
On 12/19/2018 4:49 PM, Jamil Nimeh wrote:
Hi Adam. On the whole the benchmarks look good to me. Can I ask why
those ciphers and key agreement schemes that support multiple key
lengths aren't called out in the @Param annotations? I'm thinking
192 and 256 bit for AES and maybe 1024 and 3072 and/or 4096 for DH.
Do we not need numbers across various key lengths with these
microbenchmarks?
There probably needs to be some balance in the default parameters that
are included in the test. If we include too many, then the test will
take too long to run. My strategy here is to include default
parameters for only the most important key sizes, and for things that
we want to keep an eye on (ECDH 521 is only there because we have been
tinkering with it). Including AES 256 in the defaults is probably a
good idea, and I added it in the latest webrev. I think 192 is less
important, because it isn't widely used. For DH, benchmarking it at
all may not be really valuable, but I added it for completeness. I
don't think it is worthwhile to test multiple key sizes for DH, and
1024 is too small these days. If you have any other suggestions for
default key sizes, let me know. But keep in mind that we don't need to
be complete, because someone running the benchmark can always do
-pkeyLength=... to choose other sizes.
Thanks for the review. This is my first time writing a microbenchmark,
so I would like to also get a review from a JMH/microbenchmark expert
before I push.