search

ocaml-multicore / effects-examples

Examples to illustrate the use of algebraic effects in Multicore OCaml
ISC License
423 stars 35 forks source link

Testing of `promises.ml` seems incomplete #41

Open ejgallego opened 6 months ago

ejgallego commented 6 months ago

Dear effects examples maintainers,

thanks for the very nice examples on OCaml effects, they are very helpful!

@bhaktishh and I were looking into the promises.ml example, however we have noticed that the way the test are written, it seems that we never call the Wait effect, as all the promises are resolved immediately.

It seems to us that having a more complete test could be useful, what do you think?

We have a few ideas on how to solve this, let us know what do you think?

I guess the first to try would be to decouple promise creation from resolution, but we'd like to hear your opinion before trying to do a pull request.

dhil commented 6 months ago

We will happily welcome pull requests to improve the state of affairs. It sounds reasonable to me to include a test that'd trigger the Wait effect. One thing to keep in mind is that we should keep the test cases succinct as the contents of this repository are intended to be educational rather than being of "production-grade quality".

ejgallego commented 6 months ago

Indeed, it is very important the examples do remain simple and succinct!

We are not sure tho how to address this issue in the most direct way; with the current API, I see at least two possible options, hard to say which one is easiest.

How would you proceed to trigger Wait without having to change a lot of code?

dhil commented 6 months ago

Looking at the code again, it may be enough to change the scheduling policy https://github.com/ocaml-multicore/effects-examples/blob/master/promises.ml#L129-L130 it seems fork eagerly schedules its argument, if we were to schedule to the continuation k to run before the argument, then I think the end result should be that the applicative operator <*> (https://github.com/ocaml-multicore/effects-examples/blob/master/promises.ml#L173) will trigger the Waiting-case in its implementation.

dhil commented 6 months ago

Looking at the code again, it may be enough to change the scheduling policy

Coincidentally, this seems to be what I did in my toy implementation of async/await with multi-shot continuations, c.f. https://github.com/dhil/ocaml-multicont/blob/main/examples/async.ml#L103-L104

ejgallego commented 6 months ago

Thanks a lot for the details @dhil ; we will try that approach.

[The other approach we were thinking is indeed to add a resolve method]