Clock time

[jdantonio]

Addresses #256

• Use monotonic clock for all timers via Concurrent.monotonic_time
  • Use Process.clock_gettime(Process::CLOCK_MONOTONIC) when available
  • Fallback to java.lang.System.nanoTime() on unsupported JRuby versions
  • Pure Ruby implementation for everything else
  • Effects Concurrent.timer, Concurrent.timeout, TimerSet, TimerTask, and ScheduledTask
• Deprecated all clock-time based timer scheduling
  • Only support scheduling by delay
  • Effects Concurrent.timer, TimerSet, and ScheduledTask

[pitr-ch]

monotonic_time may be describing the method better.

[pitr-ch]

• minor: there should be do end instead of {} for multiline blocks
• Why is the class hidden and anonymous? That seems very unusual.
• Why usual pattern for different implementations is not followed? (JavaMonotonicClock etc.)

[jdantonio]

I experimented with a couple different implementations of this and didn't like any of them. The only reason this code exists is because not all runtimes support Process.clock_gettime(Process::CLOCK_MONOTONIC). At ope point I had a set of classes that follow our normal convention but I couldn't think of a single use case for them. I can't think of a single reason a user would need to create an object of this type. All anyone should ever need is Concurrent.monotonic_time. The class/object underneath that is just an implementation detail. In an early version I only created GLOBAL_MONOTONIC_CLOCK on systems which needed it. I didn't like having a global variable that wasn't available on all platforms so I implemented it universally. But I also wanted to discourage users from creating new instances of the class. A single clock is all that's necessary. So I made it anonymous and hidden from the yardoc.

I agree that it's an odd implementation, but it's also a very odd feature--one that should go away over time as people move to newer versions Ruby.

If the internal inconsistency (with respect to class name and hierarchy) bothers people I don't have a problem with us changing it. In that case I will still want to hide the classes from yardoc. And also add a notice that the class shouldn't be used directly, instead recommending only the use of Concurrent.monotonic_time.

[jdantonio]

The reason I used {} for that block is that I really dislike calling methods on an end statement. The natural replacement for that line would be end.new and that syntax always seems difficult to read. Since the convention of using begin/end for multiline blocks is just that--a convention--and not universally accepted by all Rubyists, I went with the syntax that made the most sense to be in this case.

If the team wants to develop a style guide for this gem (or adopt one created by another project) I'm happy to support that. I've never rejected a PR based on style, nor have I ever rejected a PR because someone changed the style of existing code. It's just something that's never been a high priority for me.

[pitr-ch]

I see your point. Nevertheless It already happen to me more than once that I though there is no use-case for something, but there was. For that reason I think it's better to assume that the use-case is just not obvious. In this case testing might be the reason to provide the class, to be able to create new instance injecting mocked Time, then messing with the clock testing that the monotonic condition holds, without interfering with the global instance in the gem.