Closed patricoferris closed 2 years ago
The changes look good.
We can revive the multishot continuations thanks to @dhil's https://github.com/dhil/ocaml-multicont library.
I see this repository used to use travis, as a follow-up PR I can remove that and add a Github Action to test things if that's desirable?
Certainly. This would be useful.
LGTM!
One point about continuations in 5.00 is that the stack memory allocated for a continuation is not managed by the GC. Hence, the continuations must be resumed either by continue or discontinue.
I believe that the version of the queens benchmark currently in the repo leaks the stacks, and it may be better to use the one here. The pointed benchmark is also idiomatic in that it collects all of the solutions.
This is good to go when OCaml 5.00 compatible packages have been released.
Keen to have this up as it is going to be the primary reference manual for effect handlers users in OCaml 5.00.
@patricoferris Let me know when the dependent packages have been released. I am eager to merge the PR :-)
I agree @kayceesrk, I believe dune 2.9.2+
is compatible now (and released). I'm not sure how long it will be before lwt
and multicont
get a new (or first) release. In the meantime I can just add an effects-examples.opam
file so the setup is just
$ opam install .
$ dune build
Hiding the pinning information in the opam file and changing it once there are proper releases ?
I plan to release multicont
as soon as 5.00.0+trunk stabilise a bit.
I can just add an effects-examples.opam file so the setup is just
This sounds good to me.
Lots of folks are starting to look at this repo. It would be useful if the code reflected what will be in OCaml 5. With that in mind, the proposed plan works well.
Hmm, the latest trunk branch causes the multishot examples to seg fault it would seem. I happen to have two switches for 5.00.0+trunk
, one I just created and one from a while ago (I'm not aware of any way to ask opam what commit the two compilers are from ://). The one from a while ago doesn't seg fault.
GDB points to:
Program received signal SIGSEGV, Segmentation fault.
0x00005555555bddc1 in multicont_alloc_stack_noexc ()
(gdb) bt
#0 0x00005555555bddc1 in multicont_alloc_stack_noexc ()
#1 0x00005555555bdbe8 in multicont_clone_continuation ()
#2 <signal handler called>
#3 0x000055555559346c in camlDune__exe__Memo__save_cont_314 () at multicont.ml:6
#4 0x00005555555935fb in camlDune__exe__Memo__test_369 () at multishot/memo.ml:75
#5 0x00005555555937d4 in camlDune__exe__Memo__entry () at multishot/memo.ml:82
#6 0x0000555555590a7b in caml_program ()
#7 <signal handler called>
There is a separate breaking change that is in flight: https://github.com/ocaml/ocaml/pull/11044. It may be worth waiting for this to land first.
Latest commit handles the renaming along with the compiler version number changing too. Multishot still seg faulting.
I plan to release multicont as soon as 5.00.0+trunk stabilise a bit.
@dhil do you think you could cut a release? I think the code is a bit more stable. If you want more time, perhaps we can merge the PRs and mark the multicont tests as failing.
Latest commit handles the renaming along with the compiler version number changing too. Multishot still seg faulting.
Interesting. The memo.ml
example does not segfault on my system. I suppose this could be related to the recent changes to alloc_for_stack
(c.f. https://github.com/ocaml/ocaml/commit/dd2fe0a5db). What operating system are you using, and what CPU architecture is it running on top of?
I plan to release multicont as soon as 5.00.0+trunk stabilise a bit.
@dhil do you think you could cut a release? I think the code is a bit more stable. If you want more time, perhaps we can merge the PRs and mark the multicont tests as failing.
Yes, I can look into making a release soon.
Segmentation faults are now gone on latest trunk + latest commit of ocaml-multicont :)) 👍
I have released a version of multicont
on OPAM
$ opam search multicont
# Packages matching: match(*multicont*)
# Name # Installed # Synopsis
multicont -- Multi-shot continuations in OCaml
Unfortunately I think trunk
may have just broken multicont
:/ https://github.com/ocaml/ocaml/pull/11105
#=== ERROR while compiling multicont.1.0.0~rc.1 ===============================#
# context 2.1.0 | macos/x86_64 | ocaml-variants.5.0.0+trunk | https://opam.ocaml.org#257f9ce1
# path ~/.opam/5.0.0+trunk/.opam-switch/build/multicont.1.0.0~rc.1
# command ~/.opam/opam-init/hooks/sandbox.sh build make all
# exit-code 2
# env-file ~/.opam/log/multicont-21630-61b387.env
# output-file ~/.opam/log/multicont-21630-61b387.out
### output ###
# ocamlc -c -strict-formats -strict-sequence -safe-string -bin-annot -warn-error -a multicont.mli multicont.ml
# ocamlc -c fiber_primitives.c
# fiber_primitives.c:97:12: error: no member named 'size_bucket' in 'struct stack_info'
# stack->size_bucket = size_bucket;
# ~~~~~ ^
# 1 error generated.
# make: *** [fiber_primitives.o-byte] Error 2
Looks like it was changed to cache_bucket
but the semantics may have also changed, I'm not super familiar with the internals of fibers...
There was a bug in the implementation of cache buckets, which was fixed in https://github.com/ocaml/ocaml/pull/11105.
OK. Thanks for bringing this to my attention. I will look into syncing up the library with trunk.
I've sync'ed multicont with the latest ocaml/ocaml trunk now. I will prepare another release soon.
@dhil Thanks for making the release: https://github.com/dhil/ocaml-multicont/releases/tag/v1.0.0-rc.2.
It was recently merged into the main open repository.
Great, thanks @dhil, it's all working as expected on my machine.
@kayceesrk I think this is as close as we can get to a normal setup without official releases of libraries which won't happen until OCaml 5 is released. I've opted for using the alpha repository as I think that's the recommended approach instead of having to manage pins.
Ok. Let's merge this one. Thanks @patricoferris and @dhil for the work!
The next step is to use Github Actions for the repo.
This PR ports all of the examples to OCaml 5.00 (effects without the dedicated syntax). It also updates the information on how to get it running locally with pins for OCaml 5.00 compatible versions of dune (just upstream) and lwt (a personal fork, see ocsigen/lwt#923). It's probably not worth merging until that issue is properly fixed.
Most of the changes were mechanical. One to maybe check is
ref.ml
which like thestate.ml
one you shared in patricoferris/awesome-multicore-ocaml#2 need some extra type annotations.I see this repository used to use travis, as a follow-up PR I can remove that and add a Github Action to test things if that's desirable?