4.0.0 rewrite

[natefaubion]

This is a PR to rewrite the internals of Aff to be faster and have actual guarantees around cancellation and finalization. There's still a lot to do, but I wanted to go ahead and get some feedback on the implementation.

Closes #97
Closes #96
Fixes #104
Fixes #95
Fixes #94
Fixes #91
Fixes #18

[natefaubion]

I've added a bunch of tests and worked out a lot of the kinks. I still have more tests to add back pertaining to ParAff.

I'd like to detail the behavior around the threading model and exceptions, and the interplay of effect labels. I've removed the exception label from launchAff, because it is extremely misleading. This is always a lie because it implies you should use catchException to remove the label, yet no exception will actually be caught because it is thrown asynchronously. Instead, I guarantee the following: if a thread throws an exception, and there are no other threads observing the exception (via joinThread), the exceptions will always be thrown in a fresh stack so as not to interrupt another thread, and still remain observable (eg., by a global onerror handler). Alternatively, I've added an async :: ASYNC effect to launchAff and runAff. These are equivalent to starting up a new thread scheduler, so I think it's appropriate for them to carry an appropriate effect.

I've also tried something different here with regards to liftEff and makeAff:

type AffModality eff =
  ( exception ∷ EXCEPTION
  , async ∷ ASYNC
  | eff
  )

-- Needs to have alias inlined because synonym desugaring bug in instance declarations
instance monadEffAff ∷ EffectRowEquals eff1 (exception ∷ EXCEPTION, async ∷ ASYNC | eff2) ⇒ MonadEff eff1 (Aff eff2)

makeAff ∷ ∀ eff a. ((Either Error a → Eff (AffModality eff) Unit) → Eff (AffModality eff) (Canceler eff)) → Aff eff a

In other words, any underlying Eff actions used to build an Aff implicitly carry EXCEPTION and ASYNC effects. This means that calling liftEff will remove the EXCEPTION label, as exceptions are an implicit effect in Aff. This mostly just works the way you want it to, except in one case.

someAff = makeAff \k -> do
  thread <- launchAff do
    delay (Milliseconds 100.0)
    liftEff $ k (Right unit)
  pure $ Canceler (flip killThread thread)

Or in other words, take the continuation from one scheduler, launch a new one, and invoke the continuation from the new scheduler. The effect rows won't unify in this case. Even though this is how some things are implemented internally, I'd argue this is an inherently unsafe thing to do, so I've provided unsafeLaunchAff and unsafeLiftEff functions without the AffModality restriction on effect labels. I'm open to removing it, but then we are back to what we currently have.

[natefaubion]

@paf31 suggested trying ASYNC :: # Effect -> Effect to describe whatever async effects are happening. So launchAff would have the following type:

launchAff
  :: forall eff1 eff2.
   . Aff eff1
  => Eff (async :: ASYNC (exception :: EXCEPTION | eff1) | eff2) Unit

I think this is interesting, and perhaps more accurately describes what's going on.

[jdegoes]

@natefaubion Last time I tried that, the type inferencer had major problems, and adding labels of any kind selectively results in user-pain.

What is the pain that adding ASYNC solve? The use of string labels to model semantic effects is already provably a failure IMO.

[natefaubion]

@natefaubion Last time I tried that, the type inferencer had major problems, and adding labels of any kind selectively results in user-pain.

Likely before fundeps? The EffectRowEquals constraints uses functional dependencies, so it should be the same as wrapping every Eff with catchException. I'd like to use RowLacks to actually prevent exception labels from being introduced at all, but the currying of the multiple runtime dictionaries used to encode it has significant overhead.

But you are right, it just may not be worth it, and there are tons of instances in the current ecosystem where exception labels are introduced asynchronously and can't be caught.

[natefaubion]

I think I will still elide the additional EXCEPTION effect from launchAff, but will probably remove the other machinery.

[syaiful6]

We have withResource defined on purescript-transformer and I always use it when want similiar feature of Haskell's bracket function. It look like we implement that here. I wonder what's difference here. Also try on purescript-transformer have similiar behaviour with attempt. Should we really implement same functionality here?

[natefaubion]

@syaiful6 We could probably formulate Aff so catchError is the primitive operation rather than attempt. attempt is slightly easier to implement, but you are right, it is otherwise redundant. bracket however must be a primitive because it's not as simple as just error handling (which is what withResource does). It also affects cancellation behavior. Actions run during acquisition and cleanup cannot be cancelled. If you try to kill a thread during these actions it will block until they have completed.

[natefaubion]

This still needs documentation updated, but otherwise the code is ready for review.

[natefaubion]

The ParAff instances are clearly not stack safe, so I will try to reformulate it with some sort of free applicative.

[natefaubion]

I wanted to make sure everything worked whether or not we decided to schedule forks (rather than evaluating immediately), and there were some edge cases that failed. That was a red flag to me, so I've refactored the internal Fiber API to be more robust, and at least manually verified that it works and all tests pass regardless of evaluation strategy around forks in case we decide to change it at some point.

[natefaubion]

@jdegoes I think we just need to decide whether we want supervised threads to be the default, and what we should do about laws in purescript-contrib/purescript-fork#4 (comment).

[jdegoes]

@natefaubion All right. Give me a day to think this over and we can hash out the details tomorrow.

[natefaubion]

I've added attempt, apathize, cancelWith, and nonCanceler.

[natefaubion]

I've changed the default behavior of forkAff to be unsupervised, and added a supervise combinator instead.

[natefaubion]

This is ready to go. Would anyone like to review the README changes?

[garyb]

Worth mentioning Parallel here too? Or perhaps just "and others" if you'd rather reserve the mention of that for the section below.

[garyb]

We usually put library names in backticks rather than making them italic I think. Perhaps we can reword it generally to avoid mentioning the library though, since it seems a bit redundant when you're already reading the readme.

[garyb]

May as well move the link ref onto the class name (rather than linking with a here) while we're doing these updates!

[garyb]

Is it worth briefly mentioning the take/put, etc. sematics AVar blocking here? If people aren't familiar with MVar it might not be obvious what the difference between this and a Ref is, even with the code example.

[garyb]

All the new changes to the readme LGTM!

[natefaubion]

Edits made, with slightly more explanatory avar stuff.

[natefaubion]

@jdegoes @garyb Should we merge this? We can do a pre-release, I guess, since there's quite a bit to look into updating.

[jdegoes]

@natefaubion Yes, let's merge and do a pre-release as well.

[natefaubion]

🎉

[garyb]

🎆 🍾 🎈