On Wed, 17 Jul 2024 15:35:26 GMT, Fernando Guallini <fguall...@openjdk.org> wrote:
>> The manual test Cipher/DES/PerformanceTest.java fails with >> ArithmeticException due to potential division by zero. The issue arises when >> calculating the elapsed time using end - start, which could result in zero >> milliseconds if start and end are identical due to the high speed of >> execution. This leads to a division error in the following code snippet: >> >> >> start = System.currentTimeMillis(); >> end = System.currentTimeMillis(); >> int speed = (int)((data.length * count)/(end - start)); >> >> This issue is easily reproducible on platforms where >> System.currentTimeMillis() has low granularity, such as many versions of >> Windows, end and start can be equal when obtaining >> System.currentTimeMillis() if the test runs very quickly. >> >> The fix is to use System.nanoTime() instead, which is designed to measure >> elapsed time with very high precision. This value is then converted to >> microseconds so the tests can properly calculate the throughput (bytes >> processed per microsecond) for the report. Example output: >> >> >> Algorithm DataSize Rounds Bytes/microsec >> DES/ECB/NoPadding 1024 100 66 >> DES/ECB/NoPadding 1024 1000 50 >> DES/ECB/NoPadding 8192 100 70 >> DES/ECB/NoPadding 8192 1000 70 >> Average: 64 > > Fernando Guallini has updated the pull request incrementally with one > additional commit since the last revision: > > byter per microsec While this should work for the systems we use, be aware that it's not guaranteed by the Java API. From the `System` javadoc: > public static long nanoTime() > > This method provides nanosecond precision, but not necessarily nanosecond > resolution (that is, how frequently the value changes) - no guarantees are > made except that the resolution is at least as good as that of > [currentTimeMillis()](https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/lang/System.html#currentTimeMillis()). It's probably fine, or you could also use the !=0?end-start:1 hack as a backup. Otherwise, LGTM. ------------- Marked as reviewed by wetmore (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/20135#pullrequestreview-2187051660
