The implementation of the class StopWatch in graphhopper/core/…/util (see StopWatch on Github) relies on System.nanoTime() returning a positive long value. However, the Java documentation explicitly states that System.nanoTime() may return a negative value:
The value returned represents nanoseconds since some fixed but arbitrary origin time (perhaps in the future, so values may be negative).
(see JavaDoc System.nanoTime)
Is there a specific reason for checking against negative values or is this a bug?
Further, I suggest implementing a method reset() that resets a StopWatch to 0, so that it can be re-used in later measurements. This would be more friendly to the GC.
Why do you think it relies on positive value? We do a check in stop once against lastTime and shouldn’t this be sufficient?
This would be more friendly to the GC.
This will be only a problem if you create StopWatches in a loop, but not necessarily even then. I would change it only if this is a problem in real world and shown e.g. from a profiler.
Why do you think it relies on positive value? We do a check in stop once
against lastTime and shouldn’t this be sufficient?
Because method stop() checks whether lastTime < 0. If System.nanoTime()
returns a negative value when lastTime is set in method start(), then a
call to stop() has no effect, i.e., the stop-watch runs forever. At the
same time getCurrentSeconds() would always report 0.
Using the value -1 to determine whether the stop-watch is running
doesn’t seem correct.
This would be more friendly to the GC.
This will be only a problem if you create StopWatches in a loop, but not
necessarily even then. I would change it only if this is a problem in
real world and shown e.g. from a profiler.
This is a valid point. Thanks for your opinion.