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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Integrates cats.effect.Timer and IO.cancelable #598

Merged
merged 6 commits into from Mar 8, 2018

Conversation

Projects
None yet
3 participants
@alexandru
Member

alexandru commented Feb 24, 2018

In cats-effect I managed to push a new Timer data type, as a pure alternative to Scheduler: typelevel/cats-effect#132

This is my last annoyance with Monix's API, hopefully the last breaking change before a final 3.0 release (in our own back yard so to speak, cats-effect being another story).

Summary of this PR:

  1. change Iterant.intervalAtFixedRate and Iterant.intervalWithFixedDelay to be polymorphic via Timer (instead of specialized for Task)
  2. change the Iterant builders, getting rid of Coeval or Task specializations
  3. change Scheduler to also provide clockRealTime and clockMonotonic operations, instead of currentTimeMillis, which should address an older complaint; an extension method with @deprecated flag is still provided for source compatibility
  4. add a Task.timer: Timer[Task] implicit and a Task.timer(sc: Scheduler) builder
  5. add a Scheduler#timer extension method
  6. add a Cancelable#cancelIO extension method and Cancelable.fromIO builder
  7. Task#toIO to yield cancelable IO values

1. Iterant.intervalAtFixedRate and intervalWithFixedDelay

Iterant.intervalAtFixedRate and Interant.intervalWithFixedDelay have been defined specifically for Task, however this dependency on Task annoys me because the point of Iterant was to abstract over F[_] data types.

This changes the interval builders like so:

def intervalAtFixedRate[F[_]](period: FiniteDuration)
  (implicit F: Async[F], timer: Timer[F]): Iterant[F, Long]

def intervalAtFixedRate[F[_]](initialDelay: FiniteDuration, period: FiniteDuration)
  (implicit F: Async[F], timer: Timer[F]): Iterant[F, Long]

def intervalWithFixedDelay[F[_]](delay: FiniteDuration)
  (implicit F: Async[F], timer: Timer[F]): Iterant[F, Long]

def intervalWithFixedDelay[F[_]](initialDelay: FiniteDuration, delay: FiniteDuration)
  (implicit F: Async[F], timer: Timer[F]): Iterant[F, Long]

This means that we can now work with cats.effect.IO as well:

Iterant[IO].intervalAtFixedRate(3.seconds)

2. Iterant builders

As part of this ticket the IterantBuilders is expanded into a hierarchy that allows for an 脿 la carte approach, as we now have to differentiate between Sync[F] and Async[F] data types, plus we allow Applicative[F] and simple builders to be curried as well. No more hard-coding of Task or Coeval builders, although we still have From[Task], From[Coeval] and From[IO] for performance reasons.

Plus the Iterant.CatsInstances got simplified. We no longer allow the possibility of providing alternative Task or Coeval instances for Cats type-classes because type-classes are meant to be coherent.

3. Scheduler to provide clockRealTime and clockMonotonic

So first the Scheduler interface gets rid of currentTimeMillis and changes to this:

trait Scheduler {
  // ...

  def clockRealTime(unit: TimeUnit): Long

  def clockMonotonic(unit: TimeUnit): Long

}

These are the equivalents of System.currentTimeMillis and of System.nanoTime respectively. They also allow picking the precision of the output (even if the actual resolution will be implementation dependent).

These reflect the signature of their pure counterparts in the Timer interface that I pushed in cats-effect.

4. Task.timer

We can provide a Task[Timer] that's globally available out of the box.

import cats.effect.Timer
import monix.eval.Task

val timer = Timer[Task]
// Equivalent to:
val timer = Task.timer

Given a Scheduler, we can also compile a Task[Timer] that uses it, instead of the Task's default Scheduler:

val scheduler = Scheduler.io()

val timer = Task.timer(scheduler)

This is an overload, because the default Task[Timer] instance does not take an implicit Scheduler 馃槈

5. Scheduler#timer extension method

Scheduler has a new extension method for your convenience for building Timer instances out of IO:

import cats.effect._
import monix.execution.Scheduler

val scheduler = Scheduler.io()

val timer: Timer[IO] = scheduler.timer[IO]

This is not a method of Scheduler, but an extension method defined in Scheduler.Extensions.

6. Cancelable to and from IO[Unit] conversion

The new IO.cancelable needs an IO[Unit] in the passed function describing the cancellation logic.
Therefore it becomes useful to turn Cancelable values in IO[Unit] values and vice versa.

// Conversion to IO
val c = Cancelable(() => println("Cancelling!"))
val io: IO[Unit] = c.cancelIO

// Conversion from IO
val io = IO(println("Cancelling!"))
val c: Cancelable = Cancelable.fromIO(io)

Note that the execution of an IO value is not idempotent, but the Cancelable.fromIO conversion guarantees idempotency.

7. Task#toIO to yield cancelable IO values

Such IO values are now cancelable:

val io: IO[Unit] = Task.sleep(10.seconds).toIO
@alexandru

This comment has been minimized.

Member

alexandru commented Feb 24, 2018

/cc @oleg-py I know you worked on the those interval builders, would like to know what you think.

@codecov

This comment has been minimized.

codecov bot commented Feb 24, 2018

Codecov Report

鉂楋笍 No coverage uploaded for pull request base (master@9cfa694). Click here to learn what that means.
The diff coverage is 92.41%.

@@            Coverage Diff            @@
##             master     #598   +/-   ##
=========================================
  Coverage          ?   90.74%           
=========================================
  Files             ?      376           
  Lines             ?     9927           
  Branches          ?     1871           
=========================================
  Hits              ?     9008           
  Misses            ?      919           
  Partials          ?        0

@alexandru alexandru changed the title from WIP: Add Timer data type, refactor Interval.interval* builders to Add Timer data type, refactor Interval.interval* builders Feb 24, 2018

@alexandru

This comment has been minimized.

Member

alexandru commented Feb 24, 2018

Decided that this PR will get merged after typelevel/cats-effect#121 gets merged in cats-effect.

This is because I'm hoping we get a Timer in cats-effect, as painful as this wait may be, because I'm striving for stability here.

@oleg-py

This comment has been minimized.

Collaborator

oleg-py commented Feb 24, 2018

@alexandru I'm +1 on waiting for cats-effect. If somebody needs to use these builders with IO as of now, they can do .liftMap(_.to[IO], _.to[IO]).

@alexandru

This comment has been minimized.

Member

alexandru commented Feb 26, 2018

@oleg-py and now the PR is official: typelevel/cats-effect#132

馃槒

@alexandru alexandru changed the title from Add Timer data type, refactor Interval.interval* builders to Add Timer data type, change Interant.interval* and Scheduler Feb 28, 2018

@alexandru alexandru changed the title from Add Timer data type, change Interant.interval* and Scheduler to Integrates cats.effect.Timer, change Interant.interval* and Scheduler Feb 28, 2018

@alexandru alexandru changed the title from Integrates cats.effect.Timer, change Interant.interval* and Scheduler to Integrates cats.effect.Timer Feb 28, 2018

@alexandru alexandru changed the title from Integrates cats.effect.Timer to Integrates cats.effect.Timer and IO.cancelable Feb 28, 2018

@alexandru

This comment has been minimized.

Member

alexandru commented Feb 28, 2018

This PR is unfortunately a little big now due to the refactoring of Scheduler.

But I'm personally happy with it because it fixes a long time complaint of Scheduler using System.currentTimeMillis.

@oleg-py @Avasil would like to know your thoughts.

N.B. this PR makes Monix depend on a hash version. Will have to wait on a final 0.10 release of cats-effect for this to be merged.


import scala.annotation.tailrec
import scala.collection.immutable.SortedSet
import scala.concurrent.duration.{Duration, FiniteDuration, TimeUnit}
import scala.util.Random

/** A scheduler meant for testing purposes. */
/** [[Scheduler]] implementation meant and a provider of

This comment has been minimized.

@Avasil

Avasil Mar 1, 2018

Collaborator

I think it could be worded a bit better, maybe without "implementation meant" phrase?

* {{{
* implicit val ec = TestScheduler()
*
* val f = Future(1 + 1).flatMap(_ + 1)

This comment has been minimized.

@Avasil

Avasil Mar 1, 2018

Collaborator
val f = Future(1 + 1).flatMap(x => Future(x + 1))

try {
c.cancel()
fail("c.cance() should throw")

This comment has been minimized.

@Avasil

Avasil Mar 1, 2018

Collaborator

cancel

@alexandru

This comment has been minimized.

Member

alexandru commented Mar 8, 2018

This is getting merged, because the next release of Monix is now synced with cats-effect 0.10.

@alexandru

This comment has been minimized.

Member

alexandru commented Mar 8, 2018

N.B. not everything that needs to happen for cats-effect 0.10 is in this PR.

We need to implement the new Concurrent type class for example. Will create another PR.
W00t, exciting!

@alexandru

This comment has been minimized.

Member

alexandru commented Mar 8, 2018

Oh, the good news is that I fixed the Sync / Async tests for Iterant by making that param for testing stack safety configurable in cats-effect.

@alexandru alexandru merged commit 8964d3e into monix:master Mar 8, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment