Skip to content
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

Feature/interrupt with algebra #1059

Merged
merged 3 commits into from
Jan 7, 2018
Merged

Conversation

pchlupacek
Copy link
Contributor

@mpilquist @SystemFw there is the interrupt implemented with Algebra.

The idea is pretty simple, I had it on mind couple of weeks, but always failed to implement up to until now :-). Essentially the idea is that scope that is first to be interrupted is caching it`s next step in case of interruption and using that step when the interruption occurs.

Also as part of this -

  • Interrupted exception is gone
  • Scope now takes type O
  • ++, repeat, flatMap reverted to M10 implementations

The implementation is bit noisy (if you can find a way how to make it cleaner I will really appreciate). Going forward I am thinking of introducing new type Compile[F, O] that will Contain current Scope[F, O] to cleanup algebra and also allow different compilation implementations to be plugged in (i.e. debugging compiler, javascript-optimized compiler etc..)

@pchlupacek
Copy link
Contributor Author

FWIW I think the travis failure is unrelated

@SystemFw
Copy link
Collaborator

SystemFw commented Jan 6, 2018

I think some instability in the retry spec is unavoidable (is testing repetitions in time), let's see if the failure is indeed a random one

@pchlupacek
Copy link
Contributor Author

@SystemFw I think the issue is that delays is mutable unguarded ListBuffer. I think that spec needs to be made stronger, as the delays are accesses potentially from multiple threads.

@@ -2793,7 +2771,7 @@ object Stream {
val runStep =
Algebra
.compileScope(
scope,
scope.asInstanceOf[fs2.internal.CompileScope[F, UO]], // todo: resolve cast
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can the TODO be resolved by specifying UO as type param to getScope (instead of Nothing)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunatelly the result of unconsAsync is Pull[F, Nothing, AsyncPull[F, Option[(Segment[O, Unit], Stream[F, O])]]] so I would need then typecase UO back to Nothing, at least that's what compiler is asking me to do.
I have plan to address in #1056.

@pchlupacek
Copy link
Contributor Author

@mpilquist @SystemFw pls let me know when you will be done reviewing this. I have ready merge pull request that uses new interruption, and can just make PR once this is merged.

@SystemFw
Copy link
Collaborator

SystemFw commented Jan 7, 2018 via email

@mpilquist mpilquist merged commit 9d99377 into series/0.10 Jan 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants