Fixed Sleeping Action #236
Conversation
|
RxJava-pull-requests #95 SUCCESS |
| @@ -29,14 +29,18 @@ | |||
| public SleepingAction(Func0<Subscription> underlying, Scheduler scheduler, long timespan, TimeUnit timeUnit) { | |||
| this.underlying = underlying; | |||
| this.scheduler = scheduler; | |||
| this.execTime = scheduler.now() + timeUnit.toMillis(timespan); | |||
| this.execTime = scheduler.now() + timeUnit.toNanos(timespan); | |||
There was a problem hiding this comment.
Is scheduler.now() supposed to be in millis or nanos? If nanos, why exactly? That means it's not actually "time" it's the relative time since the JVM started.
I question if this is correct in AbstractScheduler:
@Override
public long now() {
return System.nanoTime();
}There was a problem hiding this comment.
We need to make it very clear on the Scheduler interface if it is millis or nanos ... and that should be defined by whether "now" is intended to be absolute or relative.
There was a problem hiding this comment.
This implementation returns absolute time in nanoseconds. I'll update the javadoc.
There was a problem hiding this comment.
It's not absolute time ... the JVM returns nanoseconds since JVM started thus we can't use it to determine absolute time and therefore anything involving absolute time would not work.
http://docs.oracle.com/javase/6/docs/api/java/lang/System.html#nanoTime()
This method can only be used to measure elapsed time and is not related to any other notion of system or wall-clock time. The value returned represents nanoseconds since some fixed but arbitrary time (perhaps in the future, so values may be negative).
There was a problem hiding this comment.
I see, thanks for pointing out and educating me!
The implementation is still incorrect I'll update this pull request.
There was a problem hiding this comment.
Thanks ... the schedulers changes have been merged now as well so this pull request can now correctly work against that.
I'm also in the process of reviewing the ObserveOn changes right now.
Conflicts: rxjava-core/src/main/java/rx/concurrency/SleepingAction.java
|
RxJava-pull-requests #98 SUCCESS |
Fixed Sleeping Action
Sleeping action wasn't working correctly.