TristanCacqueray / ki-effectful

MIT License
7 stars 1 forks source link

Consider including Scope in the effect #1

Closed mmhat closed 2 years ago

mmhat commented 2 years ago

Not sure if that is feasible, but we could try to include the Scope in the StucturedConcurrency effect, for example like:

data StructuredConcurrency :: Effect

type instance DispatchOf StructuredConcurrency = 'Static 'WithSideEffects
data instance StaticRep StructuredConcurrency = StructuredConcurrency Scope

-- | Run the 'StructuredConcurrency' effect.
runStructuredConcurrency :: IOE :> es => Eff (StructuredConcurrency : es) a -> Eff es a
runStructuredConcurrency k = withEffToIO $ \runInIO ->
    Ki.scoped $ \scope ->
        runInIO $ evalStaticRep (StructuredConcurrency scope) k

fork ::
    StructuredConcurrency :> es =>
    Eff es a ->
    Eff es (Thread a)
fork action = do
    StructuredConcurrency scope <- getStaticRep
    unsafeEff $ \es -> do
        es' <- cloneEnv es
        Ki.fork scope (unEff action es')

This would make the scoped function superfluous.

mmhat commented 2 years ago

Hm, I just noticed that the following would be difficult using these definitions:

foo = scoped $ \s1 -> do
    t <- scoped $ \s2 -> do
        fork s1 action
    await t
TristanCacqueray commented 2 years ago

Perhaps using a Reader Scope would also work, here is demo: https://github.com/TristanCacqueray/ki-effectful/compare/reader-scope?expand=1

Though I think I prefer the explicit version where the scope is passed explicitly.

mmhat commented 2 years ago

What I meant was something inline with the changes on this branch: https://github.com/mmhat/ki-effectful/tree/scope-in-effect

The idea was that the lifetime of the effect in the row determines correlates with the lifetime of the current scope. This comes with the assumption that a user most likely wants to fork within the current Scope. The withCurrentScope is an escape hatch here allowing one to run one or more threads in some scope further up the stack.

mmhat commented 2 years ago

BTW: You definitely want to use cloneEnv as there might be segfault otherwise!

TristanCacqueray commented 2 years ago

@mmhat thanks, that looks great. I'd be happy to merge your change and the cloneEnv additions.

In this test: https://github.com/mmhat/ki-effectful/commit/e732d8e8e0cf08d46c53215b0d1f49d546bd5ea9#diff-9e8bdd24d557cfe94781d844e68e97f4b626c833420d18df4e04619a54273b34R49-R53, would it be possible to avoid the IOE constraint, so that creating a scope within an existing scope only requires the StructuredConcurrency effect.

mmhat commented 2 years ago

Great! I opened a PR.

would it be possible to avoid the IOE constraint, so that creating a scope within an existing scope only requires the StructuredConcurrency effect.

Unfortunately this won't be possible: We call a runStructuredConconcurrency in there which requires IO due to the unlifting (withEffToIO) happening in there. And we cannot do a (safe) unlifting without IO as that needs the underlying unlifting strategy (See the definition of unliftStategy).