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

Use System.nanoTime() instead of System.currentTimeMillis() in StopWatch #23235

Closed
shenjianeng opened this issue Jul 4, 2019 · 6 comments
Closed
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement
Milestone

Comments

@shenjianeng
Copy link

shenjianeng commented Jul 4, 2019

why does not StopWatch use System.nanoTime()

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jul 4, 2019
@shenjianeng shenjianeng reopened this Jul 4, 2019
@shenjianeng shenjianeng changed the title why why StopWatch not use System.nanoTime(); Jul 4, 2019
@bclozel
Copy link
Member

bclozel commented Jul 5, 2019

We don't use the Spring Framework issue tracker for questions, but judging from your question it looks like you're considering that to be a bug.

Why should StopWatch be using nanotime? Could you explain the use case and why the current implementation is wrong?

@bclozel bclozel added the status: waiting-for-feedback We need additional information before we can continue label Jul 5, 2019
@spring-projects-issues
Copy link
Collaborator

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

@spring-projects-issues spring-projects-issues added the status: feedback-reminder We've sent a reminder that we need additional information before we can continue label Jul 12, 2019
@spring-projects-issues
Copy link
Collaborator

Closing due to lack of requested feedback. If you would like us to look at this issue, please provide the requested information and we will re-open the issue.

@spring-projects-issues spring-projects-issues removed status: waiting-for-feedback We need additional information before we can continue status: feedback-reminder We've sent a reminder that we need additional information before we can continue status: waiting-for-triage An issue we've not yet triaged or decided on labels Jul 19, 2019
@pcdavid
Copy link

pcdavid commented Aug 6, 2019

I'm not the original reporter, but I just had the same remark/question when stumbling on the org.springframework.util.StopWatch implementation.

The Javadoc for currentTimeMillis() indicates:

Note that while the unit of time of the return value is a millisecond, the granularity of the value depends on the underlying operating system and may be larger. For example, many operating systems measure time in units of tens of milliseconds.

nanoTime() on the other hand is explicitly designed to measure precise time intervals between invocations (its absolute value however is meaningless). It is guaranteed to use a resolution at least as fine as currentTimeMillis(), and possibly much better.

So currentTimeMillis() answers "what time is it?" accurately, but with possibly a limited precision. nanoTime is more precise (though not necessarily to the nanosecond), but should only be used to measure time intervals. For the use case of a stop-watch, it seems nanoTime is better suited.

As additional data points:

@sbrannen sbrannen added the in: core Issues in core modules (aop, beans, core, context, expression) label Aug 6, 2019
@sbrannen
Copy link
Member

sbrannen commented Aug 6, 2019

@pcdavid, thanks for raising those valid points.

We'll switch to using nano time in Spring Framework 5.2.

@sbrannen sbrannen reopened this Aug 6, 2019
@sbrannen sbrannen added the type: enhancement A general enhancement label Aug 6, 2019
@sbrannen sbrannen self-assigned this Aug 6, 2019
@sbrannen sbrannen changed the title why StopWatch not use System.nanoTime(); Use System.nanoTime() instead of System.currentTimeMillis() in StopWatch Aug 6, 2019
@sbrannen sbrannen added this to the 5.2 RC2 milestone Aug 7, 2019
@sbrannen
Copy link
Member

sbrannen commented Aug 9, 2019

@shenjianeng and @pcdavid, the changes have been made in a532afb.

Feel free to try them out in the latest build snapshots for Spring Framework 5.2, and if you notice anything that's missing... just let us know.

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

5 participants