Is seq-unlift the right default?

[ocharles]

A major pain-point to me in effectful is the use of unlifting strategies and runtime errors. Worse is that MonadUnliftIO uses withEffToIO, which uses the currently set unlifting strategy, which defaults to seq-unlift. This makes porting an existing codebase that uses MonadUnliftIO pervasively very difficult, as you know way of statically knowing your program will actually work at runtime, or if it will crash - a proper port requires domain knowledge as to where threading happens. If we were to default to a conc-unlifting strategy, I believe we avoid this problem.

Or is it not that simple? Also, should it maybe be suggested that porting efforts use withUnliftStrategy to upgrade the default?

[arybczak]

Hmm, that's a tricky one. If you just want to use a specific function with MonadUnliftIO constraint, then you probably know if it runs other threads (or you'll know soon enough during testing).

But if you're porting existing code, then that no longer works 🤔 Then you have two options:

1. Testing and using withUnliftStrategy in appropriate places.
2. Setting the unlift strategy to concurrent at the top level and be done with it.

With the second point you trade a performance hit for every function that would run fine with sequential unlift for your testing time.

Both approaches are valid in different circumstances, so it's hard to pick a default that'll satisfy everyone.

[ocharles]

Hmmm, I think having something that out-of-the-box only works with single threaded MonadUnliftIO code is a pretty huge footgun, and I'll likely change all my uses of runEff to a (locally defined) runConcEff that wraps runEff . withUnliftStrategy.

With the second point you trade a performance hit for every function that would run fine with sequential unlift for your testing time.

Is this really a meaningful hit? Looking at the code, barely anything really happens in concurrent unlifting.

Both approaches are valid in different circumstances, so it's hard to pick a default that'll satisfy everyone.

This to me hints at removing runEff entirely, and exporting runEffSeq and runEffConc.

[arybczak]

Is this really a meaningful hit? Looking at the code, barely anything really happens in concurrent unlifting.

It has to make a deep copy of the environment, which is relatively expensive.

Hmmm, I think having something that out-of-the-box only works with single threaded MonadUnliftIO code is a pretty huge footgun

This to me hints at removing runEff entirely, and exporting runEffSeq and runEffConc.

I'm not sure. Presumably you'll get a runtime error quite fast when using the wrong unlifting strategy, so it shouldn't be a big deal. Am I wrong here?

The only sad thing is the lack of HasCallStack constraint on withRunInIO, thus you aren't getting any help with pinpointing where the error originated from (in contrast, effectful specific variants are arranged in a way that they'll tell you at least the location of the unlift that was misused).

Perhaps documentation could be augmented to mention potential issues under a new section "Porting existing applications that use MonadUnliftIO extensively".

[ocharles]

I'm not sure. Presumably you'll get a runtime error quite fast when using the wrong unlifting strategy, so it shouldn't be a big deal. Am I wrong here?

You'll get a runtime error when that codepath is executed. Unfortunately, we are very test-sparse at work, for better or worse. We also have a large surface area where some things aren't hit frequently, so I'm mostly worried it'll be a long run of a whack-a-mole as we find all the sources of cross-thread unlifting.

Perhaps documentation could be augmented to mention potential issues under a new section "Porting existing applications that use MonadUnliftIO extensively".

Yea, I think this would be a great idea! I'm happy to switch out runEff with runEff . withUnliftStrategy (ConcUnlift ...) for example, and at least know that things won't be crashing. I also think it's a relatively sensible choice for an application that relied on MonadUnliftIO, because by definition that can't have state that isn't tracked by things like IORefs