Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve SecureRandom benchmarks (especially in multi-threaded settings) #375

Merged
merged 3 commits into from
Jun 20, 2024

Conversation

fabrice102
Copy link
Contributor

Issue #, if available: N/A

Description of changes:

  1. Rename Drbg.java benchmark to Random.java and add more benchmarks (SUN/NativePrng, java.util.random, and some baseline using copy instead of randomness generation). SUN/NativePrng is the default Java implementation of SecureRandom when ACCP is not installed.
  2. Remove configuration line in build.gradle.kts forcing single thread in all benchmarks. This is because command line arguments override annotations, and we need to use multiple threads in some benchmarks.
  3. Use average measurement (ns/op) in build.gradle.kts instead of throughput (op/ns). This is because for multi-threaded benchmark of randomness generation, we want to know the duration of a single randomness generation on a single thread (and we want to check if performance drops because of multi-threading).
  4. Make the data variable (the target of randomness generation) thread-local to avoid L1 cache contention.
  5. Add a few more benchmarks:
    a. multiThreadedLocal where the SecureRandom instance is thread local, instead of global: this is useful for users of ACCP to know whether they should have SecureRandom be thread local or not.
    b. singleThreadedNew creating a new instance of SecureRandom: this is useful for users of ACCP to know whether it is ok to instantiate a SecureRandom every time they need it, or to instead create a global instance.
    c. A variant of the benchmarks generating 4 bytes of random data, instead of 1,024 bytes. For example, generating a random int requires 4 bytes of random data.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@fabrice102 fabrice102 requested a review from a team as a code owner April 3, 2024 18:57
@fabrice102 fabrice102 force-pushed the multithreaded-rand-bench branch from 09e68b8 to b3c1eab Compare April 3, 2024 19:15
@WillChilds-Klein WillChilds-Klein force-pushed the multithreaded-rand-bench branch from b3c1eab to 9eb49d1 Compare April 4, 2024 19:13
geedo0
geedo0 previously approved these changes Apr 5, 2024
Copy link
Contributor

@geedo0 geedo0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are great and well thought out improvements, thanks!

Fabrice Benhamouda added 2 commits April 10, 2024 18:28
This is to later add more non-Drbg Random algorithms.
1. Add more benchmarked randomness generation algorithms/providers (SUN/NativePrng, java.util.random, and some baseline using copy instead of randomness generation). SUN/NativePrng is the default Java implementation of SecureRandom when ACCP is not installed.
2. Remove configuration line in build.gradle.kts forcing single thread in all benchmarks. This is because command line arguments override annotations, and we need to use multiple threads in some benchmarks.
3. Use average measurement (ns/op) in benchmarks of randomness generation instead of throughput (op/ns). This is because for multi-threaded benchmarks of randomness generation, we want to know the duration of a single randomness generation on a single thread (and we want to check if performance drops because of multi-threading).
4. Make the data variable (the target of randomness generation) thread-local to avoid L1 cache contention.
5. Add a few more benchmarks:
    a. multiThreadedLocal where the SecureRandom instance is thread local, instead of global: this is useful for users of ACCP to know whether they should have SecureRandom be thread local or not.
    b. singleThreadedNew creating a new instance of SecureRandom: this is useful for users of ACCP to know whether it is ok to instantiate a SecureRandom every time they need it, or to instead create a global instance.
    c. A variant of the benchmarks generating 4 bytes of random data, instead of 1,024 bytes. For example, generating a random int requires 4 bytes of random data.
@fabrice102 fabrice102 force-pushed the multithreaded-rand-bench branch from b7f9771 to 0fdb6e4 Compare April 10, 2024 18:34
@fabrice102
Copy link
Contributor Author

I've rebased to split the commits in two:

  1. rename Drbg.java into Random.java
  2. apply other changes

This is to facilitate the comparison with diff in Github (using the diff between the second commit and the first commit).

@fabrice102 fabrice102 requested a review from amirhosv April 17, 2024 22:00
@amirhosv amirhosv merged commit 0917177 into corretto:main Jun 20, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants