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

Replace currentTimeMillis() with nanoTime() for duration measuring #1199

Merged
merged 1 commit into from
May 12, 2023

Conversation

Vampire
Copy link
Contributor

@Vampire Vampire commented May 9, 2023

No description provided.

@hcoles
Copy link
Owner

hcoles commented May 11, 2023

What is the issue this is addressing?

@Vampire
Copy link
Contributor Author

Vampire commented May 11, 2023

The issues this is adressing is, that you should not use System.currentTimeMillis() for measuring durations within one JVM, but System.nanoTime() if the duration is less than 292 years. It is more precise and not influenced by things like wall clock, system time, and similar that influence System.currentTimeMillis().

@hcoles
Copy link
Owner

hcoles commented May 12, 2023

Nano time costs more cpu cycles, although that is unlikely to have a measurable impact. Only concret benefit seems to be that there are (or were) issues with currentTimeMillis on windows, where the accuracy was ~50ms by default, which would indeed be an issue if it's still the case. We need time within about 1ms.

@Vampire
Copy link
Contributor Author

Vampire commented May 12, 2023

nanoTime is faster than currentTimeMillis on some systems (afair mainly Linux), but slower on others.
But the main point is, that currentTimeMillis is not monotonic. It depends on wall-clock. If you change your system time while PIT is running, you get wrong results, this could also be done by some time synchronization utility for example.
nanoTime on the other hand is monotonic and especially there to measure durations within one JVM.
It should practically always be used when measuring durations within one JVM instance.

@hcoles hcoles merged commit 8b5af8e into hcoles:master May 12, 2023
@Vampire Vampire deleted the nanoTime branch May 12, 2023 16:35
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.

2 participants