Skip to content

Abstraction for Clock with DI via RxJavaPlugins #3295

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

Closed
artem-zinnatullin opened this issue Sep 4, 2015 · 7 comments
Closed

Abstraction for Clock with DI via RxJavaPlugins #3295

artem-zinnatullin opened this issue Sep 4, 2015 · 7 comments

Comments

@artem-zinnatullin
Copy link
Contributor

Intro: in RxJava (for the Operators) time is related to a Scheduler that runs the code, and this is awesome (Schedulers.test() proves that).

But, at the moment RxJava's Schedulers use System.currentTimeMillis() as the source of the time to make some time-relative things work: delay(), window(), schedule(), schedulePeriodically(), Schedulers.test() and so on.

The problem with System.currentTimeMillis() is that it represents "wall clock" which can be changed by:

  • By the user of the OS/device.
  • By the OS itself: for example, your mobile devices sync wall clock with the carrier periodically, desktop and server OSes also do this.
  • Leap second, half of the internet was not available because of Leap second problem this year, we can prevent this problem in our systems!

Such changes of wall clock may break the business logic of the applications (think about banking, security, apps and systems) and even crash them or create dead/live locks.

Proposal: Add Clock abstraction which will use System.currentTimeMillis() by default (no breaking changes) and ability for the users to inject their own implementation via RxJavaPlugins.

In Android we can use SystemClock.elapsedRealtime() as the source of linear, monotonic time in milliseconds. I guess, we will even be able to ship :shipit: this via RxAndroid as a default option for RxJava on Android, or it's possible to do this via reflection directly in RxJava (though I am not a fan of reflection in RxJava).

See also #2943.

I can work on PR for this both for RxJava and RxAndroid if RxJava team will agree with this proposal.

CC all who interested in this, what is your opinion?

@akarnokd
Copy link
Member

akarnokd commented Sep 4, 2015

Do we have a desktop Java version of this SystemClock.elapsedRealtime?

@konmik
Copy link

konmik commented Sep 4, 2015

We have System.nanoTime at both - Android and JRE.

@akarnokd
Copy link
Member

akarnokd commented Sep 4, 2015

System.nanoTime has much weaker guarantees, see this.

@konmik
Copy link

konmik commented Sep 4, 2015

Weaker guarantees than what? There are no other nanosecond precision counters in Java. nanoTime has better guarantees than others just because it is nano. It is just slower sometimes.

@artem-zinnatullin
Copy link
Contributor Author

@akarnokd

Do we have a desktop Java version of this SystemClock.elapsedRealtime?

Nope :( Some OSses give such APIs but it'll require JNI call, though one of the ideas of Clock abstraction is that you can inject your own implementation and do such JNI calls if it's really critical for your system.

@konmik

Weaker guarantees than what? There are no other nanosecond precision counters in Java. nanoTime has better guarantees than others just because it is nano. It is just slower sometimes.

System.nanoTime() can depend on internal CPU timer (depends on JDK/JRE).

For example, in multi-CPU systems System.nanoTime() can give different results between CPUs, since RxJava actively used in backend systems like Netflix and multi-CPU servers are pretty usual — we can not use System.nanoTime() for Schedulers.

@sparty02
Copy link

sparty02 commented Sep 6, 2015

To note, I think the spirit of this has been addressed in Java 8's Clock. If there is a need to support < 8, the Java 8 Date/Time refactor has been externally back ported via the ThreeTen project, which exposes a similar clock abstraction. cc @jodastephen. If there is a strong need to limit the dependencies, maybe it's still worth modeling after these abstractions.

@artem-zinnatullin
Copy link
Contributor Author

Closing for now, #3467 looks good so far.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants