Add `Async#asyncCheckAttempt` for #3087 #3091

seigert · 2022-07-11T18:18:15Z

Implementation of proposed utility method in Async for double-check registration. Also implemented in IO itself and some tests.

I've chosen asyncPoll as a name as method itself reminds me about Rust's Future poll method with also returns computed result or provides waker to schedule another poll sometime later.

armanbilge · 2022-07-11T18:38:02Z

core/shared/src/main/scala/cats/effect/IO.scala

+  def asyncPoll[A](
+      k: (Either[Throwable, A] => Unit) => IO[Either[Option[IO[Unit]], A]]): IO[A] = {
+    val body = new Cont[IO, A, A] {
+      def apply[G[_]](implicit G: MonadCancel[G, Throwable]) = { (resume, get, lift) =>
+        G.uncancelable { poll =>
+          lift(k(resume)) flatMap {
+            case Right(a) => G.pure(a)
+            case Left(Some(fin)) => G.onCancel(poll(get), lift(fin))
+            case Left(None) => poll(get)
+          }
+        }
+      }
+    }
+
+    IOCont(body, Tracing.calculateTracingEvent(k))
+  }


Sorry if I missed it—do we get an optimization be re-implementing this for IO? Otherwise probably we can delegate to the Async implementation?

I've added it because a) only tests for async I've found were in IOSpec.scala and b) IO has async & async_ reimplemented too (and only visible bonus is Tracking + IOCont). If that not a concern, I could delete it.

Maybe some law should be added to AsyncLaws instead? Like (F.asyncPool[A](k => F.pure(Right(a))) <* F.unit) <-> F.pure(a))?

Ah right, thanks, I missed the Tracing. Actually my main concern was that if we override the implementation in IO, then the generic implementation in Async is not being tested anywhere, since as you point out the only are were in IOSpec.

Well, it was this way before -- Async.async & Async.async_ a re-implemented in IO almost word by word, so the status quo stays the same. :)

Anyway, I've added three more laws to AsyncLaws about asyncPoll <-> (async, pure) (hope they make some sense) and added some synthetic test for some 'basic' Async variant based off IO which reuses all method implementations in Async itself.

seigert · 2022-07-11T19:49:45Z

Also, should docs for cont be updated about this method? "If you are an implementor, and you have async (or asyncPool), Async.defaultCont provides an implementation of cont in terms of async/asyncPool."

seigert · 2022-07-14T07:27:59Z

@armanbilge, do you have any insight why check may randomly flick? Yesterday it was IoAppSpec, not it is WorkerThreadNameSpec -- both should be unaffected by my changes. :(

armanbilge · 2022-07-14T12:40:21Z

Darn, I just restarted CI for you. IOAppSpec is a well known flake. WorkerThreadNameSpec was only recently added, but I guess it's a flaky one too :(

djspiewak

This is great! Thank you for taking this on, and I'm sorry it took me so long to review. Left some comments.

djspiewak · 2022-09-19T20:24:12Z

tests/shared/src/test/scala/cats/effect/kernel/AsyncSpec.scala

+import scala.concurrent.ExecutionContext
+import scala.concurrent.duration._
+
+class AsyncSpec extends BaseSpec with Discipline {


Why did this have to be pulled out?

As @armanbilge mentioned above, Async version of async_ (and new generic version of async itself) was not tested anywhere, as IOSpec.scala only tests Async[IO] instance that, in turn, uses redefined IO.async & IO.async_.

They are almost literal copies of one another, but still -- as there is two methods in Async that both should provide backward compatible generic implementations via asyncPoll I've decided to pull out test spec for Async itself.

The problem is that Async does not have any instances except for IO, and IO overrides most methods, so spec provides another Async instance that only defines necessary abstract methods.

Oh gotcha. Nice catch!

djspiewak · 2022-09-19T20:24:49Z

core/shared/src/main/scala/cats/effect/IO.scala

+   * @see
+   *   [[async]] for a simplified variant without an option for immediate result
+   */
+  def asyncPoll[A](


The one problem I see with the name is we already use the word Poll for something quite different! This might be worth bikeshedding a bit to avoid user confusion.

I don't think my English is good enough to provide a meaningful name, but after some thoughts there are my suggestions:

asyncAttempt/tryAsync -- as in 'we'll try to schedule async operation but if results are already available, then so be it'. Both with 'error handling' connotations, sadly.

resolve/resolveEventually -- as in 'computed value will be resolved one way or another, right now or in future';

registerAsyncCallback -- intentionally enterprise-like name to emphasis that this is a low-level method like cont.

I'll give this some thought and report back in a day or two! I actually rather like asyncAttempt, but you're right it does have that negative "error" context.

@djspiewak, did you had time to think about a name?

Ah, I haven't! Thank you for the reminder. I'll get to this, I apologize.

I think asyncCheckAttempt is probably the best I can come up with. I agree with you that we don't necessarily want it to be a really concise name.

Suggested change

def asyncPoll[A](

def asyncCheckAttempt[A](

core/shared/src/main/scala/cats/effect/IO.scala

kernel/shared/src/main/scala/cats/effect/kernel/Async.scala

djspiewak · 2022-10-29T19:36:37Z

kernel/shared/src/main/scala/cats/effect/kernel/Async.scala

@@ -62,22 +62,28 @@ import java.util.concurrent.atomic.AtomicReference
 trait Async[F[_]] extends AsyncPlatform[F] with Sync[F] with Temporal[F] {

  /**
-   * The asynchronous FFI.
+   * The asynchronous FFI with option for immediate result.


Can we unify this with the IO.asyncCheckAttempt scaladoc?

Not sure that I understood you correctly, but I've tried to sync IO and Async async* method descriptions with exception of 'real-world examples'.

djspiewak

Thank you!! This has been a very much appreciated effort.

Add Async#asyncPoll for typelevel#3087

732603e

armanbilge linked an issue Jul 11, 2022 that may be closed by this pull request

Consider an Async utility to encapsulate double-check registration #3087

Closed

armanbilge reviewed Jul 11, 2022

View reviewed changes

Format sources with scalafmt

52c08c9

seigert requested a review from armanbilge July 12, 2022 12:03

Add three more AsyncLaws tests and spec for basic Async implementation

673f7f5

seigert force-pushed the add-async-poll branch from 42a22ec to f05f288 Compare July 13, 2022 16:11

Fix scalafix

2e5a38c

seigert force-pushed the add-async-poll branch from f05f288 to 2e5a38c Compare July 15, 2022 08:49

djspiewak reviewed Sep 19, 2022

View reviewed changes

djspiewak reviewed Oct 29, 2022

View reviewed changes

core/shared/src/main/scala/cats/effect/IO.scala Outdated Show resolved Hide resolved

djspiewak reviewed Oct 29, 2022

View reviewed changes

kernel/shared/src/main/scala/cats/effect/kernel/Async.scala Outdated Show resolved Hide resolved

djspiewak reviewed Oct 29, 2022

View reviewed changes

Alexey Shuksto added 3 commits November 9, 2022 14:30

Merge remote-tracking branch 'origin/series/3.x' into add-async-poll

3988ac3

Rename 'asyncPoll => asyncCheckAttempt', sync 'Async/IO' scaladoc

e4e05aa

Run scalafmtAll

719c106

seigert changed the title ~~Add Async#asyncPoll for #3087~~ Add Async#asyncCheckAttempt for #3087 Nov 14, 2022

djspiewak approved these changes Nov 14, 2022

View reviewed changes

djspiewak merged commit fa3324a into typelevel:series/3.x Nov 14, 2022

durban mentioned this pull request Feb 7, 2024

Improve MonadCancel scaladoc #3994

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `Async#asyncCheckAttempt` for #3087 #3091

Add `Async#asyncCheckAttempt` for #3087 #3091

seigert commented Jul 11, 2022

armanbilge Jul 11, 2022

seigert Jul 11, 2022

armanbilge Jul 13, 2022

seigert Jul 13, 2022 •

edited

Loading

seigert commented Jul 11, 2022

seigert commented Jul 14, 2022

armanbilge commented Jul 14, 2022

djspiewak left a comment

djspiewak Sep 19, 2022

seigert Sep 20, 2022

djspiewak Sep 20, 2022

djspiewak Sep 19, 2022

seigert Sep 20, 2022

djspiewak Sep 20, 2022

seigert Oct 3, 2022

djspiewak Oct 9, 2022

djspiewak Oct 29, 2022

seigert Nov 9, 2022

djspiewak Oct 29, 2022

seigert Nov 9, 2022 •

edited

Loading

djspiewak left a comment

Add Async#asyncCheckAttempt for #3087 #3091

Add Async#asyncCheckAttempt for #3087 #3091

Conversation

seigert commented Jul 11, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seigert Jul 13, 2022 • edited Loading

Choose a reason for hiding this comment

seigert commented Jul 11, 2022

seigert commented Jul 14, 2022

armanbilge commented Jul 14, 2022

djspiewak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seigert Nov 9, 2022 • edited Loading

Choose a reason for hiding this comment

djspiewak left a comment

Choose a reason for hiding this comment

Add `Async#asyncCheckAttempt` for #3087 #3091

Add `Async#asyncCheckAttempt` for #3087 #3091

seigert Jul 13, 2022 •

edited

Loading

seigert Nov 9, 2022 •

edited

Loading