alcotest
alcotest copied to clipboard
mirage
Proof of concept - Fix the race with Formatters (issue #375)
I have no idea where these should be, but this is enough to fix the race, if someone could step in I'd appreciate.
The fix looks ok to me, just two questions:
- what's the cost of looking for something in
DLSfor every print operation? (I suspect this is not really an issue as people are not expected to write to stdout/stderr in a hot loop, but I'm still interested to know the answer :p) - is there a better way to use both the OCaml 5 API and OCaml 4? For instance, could we have a mock version for DLS somewhere?
what's the cost of looking for something in DLS for every print operation?
Lookup on DLS is constant time.
is there a better way to use both the OCaml 5 API and OCaml 4? For instance, could we have a mock version for DLS somewhere?
We have an OCaml 4 compatible Domain API in domain-shims, it also supports DLS -- https://gitlab.com/gasche/domain-shims/-/blob/trunk/lib/domain.ocaml4.mli#L88. The point to note here is domain-shims uses systhreads internally to simulate domains. If that's an issue, a mock DLS implementation for OCaml 4 should be fairly straightforward.
The fix looks ok to me, just two questions:
- what's the cost of looking for something in
DLSfor every print operation? (I suspect this is not really an issue as people are not expected to write to stdout/stderr in a hot loop, but I'm still interested to know the answer :p)
Not that I profiled, but I'd say that it's many magnitudes less than the actual IO operation.
- is there a better way to use both the OCaml 5 API and OCaml 4? For instance, could we have a mock version for DLS somewhere?
Even if we mock the DLS we would still need to mock the Format.synchronized_formatter_of_out_channel.
A simpler alternative could be to convert all prints to some function that grabs a mutex, and then we just use the normal output channel, this way the code could be the same for 4 and 5.
@haesbaert do you mind adding a change entry? I'm happy to merge/release this quickly to make a release compatible with multi-domains.
@haesbaert do you mind adding a change entry? I'm happy to merge/release this quickly to make a release compatible with multi-domains.
I've reworded the commit and added the Changes entry.
