-
Notifications
You must be signed in to change notification settings - Fork 73
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make Switch.on_release thread safe #684
Conversation
d695632
to
ce6001f
Compare
I took a very brief look and, IIUC, then what you basically need is a lock-free set. I developed a lock-free starvation avoiding data structure to deal with a similar problem in Picos. See here. Both The elements in the set are mutable (think Because both attach and detach perform the gc at the same point (i.e. the same threshold) the gc is effectively wait-free (i.e. if you e.g. attach and that requires gc, then the only reason to require a retry is that another operation performed the gc for you). The attach and detach operations are not wait-free, because an operation could repeatedly need to retry as other parties finish their operations more quickly, but the amounts of work performed by attach and detach operations are very similar (both make an allocation to update the |
Thanks - that does sound like what we want. I'm not sure making it lock-free is necessary yet - adding the mutex didn't seem to have any noticeable effect on the benchmarks and there isn't likely to be much contention for the Irmin use (opening a file takes much longer than taking the mutex and appending to the list). But if it becomes a problem I'll use that code. |
ce6001f
to
cb66c5c
Compare
This is needed to allow resource pools to be shared between domains.
cb66c5c
to
d979cc0
Compare
CHANGES: New features: - eio_posix: use directory FDs instead of realpath (@talex5 ocaml-multicore/eio#694 ocaml-multicore/eio#696, reviewed by @SGrondin). Using realpath was an old hack from the libuv days and subject to races. It was also slow. - Keep pool of systhreads for blocking operations (@SGrondin @talex5 ocaml-multicore/eio#681). This is much faster than creating a new thread for each operation. It mainly benefits the eio_posix backend, as that uses lots of systhreads. - Make `Switch.on_release` thread-safe (@talex5 ocaml-multicore/eio#684, requested by @art-w and @clecat). This allows resource pools to be shared between domains easily. - Add `Eio.Path.read_link` (@talex5 ocaml-multicore/eio#686). - Add `Eio_unix.Fd.is_open` (@talex5 ocaml-multicore/eio#690). - Include backtrace in systhread errors (@talex5 ocaml-multicore/eio#688, reviewed by @SGrondin). Also, add `Eio.Exn.empty_backtrace` as a convenience. - eio.mock: add tracing support to mock backend (@talex5 ocaml-multicore/eio#687). - Improve tracing (@talex5 ocaml-multicore/eio#675 ocaml-multicore/eio#683 ocaml-multicore/eio#676, reviewed by @SGrondin). Update tracing section of README and trace more things (`run_in_systhread`, `close`, `submit`, `traceln`, cancellation and domain spawning). Documentation: - Link to verification work in docs (@talex5 ocaml-multicore/eio#682). - Add more trace diagrams to README (@talex5 ocaml-multicore/eio#698). - Adjust COC contacts (@polytypic ocaml-multicore/eio#685, reviewed by @Sudha247). Bug fixes: - eio_linux: retry `openat2` on `EAGAIN` (@talex5 ocaml-multicore/eio#693, reviewed by @SGrondin). - eio_posix and eio_windows: check for IO periodically (@talex5 ocaml-multicore/eio#674). - Handle EPERM when trying to initialise uring (@talex5 ocaml-multicore/eio#691). This can happen when using a Docker container. Build and tests: - Benchmark `Eio_unix.run_in_systhread` (@talex5 ocaml-multicore/eio#678, reviewed by @SGrondin). - Enable lintcstubs for `Eio_unix.Private` too (@talex5 ocaml-multicore/eio#689). - Stat benchmark: report cleanup time and optimise (@talex5 ocaml-multicore/eio#692). - Make benchmarks start faster (@talex5 ocaml-multicore/eio#673). - Update build for new eio-trace CLI (@talex5 ocaml-multicore/eio#699). - Expect opam-repo-ci tests to fail on macos (@talex5 ocaml-multicore/eio#672).
CHANGES: New features: - eio_posix: use directory FDs instead of realpath (@talex5 ocaml-multicore/eio#694 ocaml-multicore/eio#696, reviewed by @SGrondin). Using realpath was an old hack from the libuv days and subject to races. It was also slow. - Keep pool of systhreads for blocking operations (@SGrondin @talex5 ocaml-multicore/eio#681). This is much faster than creating a new thread for each operation. It mainly benefits the eio_posix backend, as that uses lots of systhreads. - Make `Switch.on_release` thread-safe (@talex5 ocaml-multicore/eio#684, requested by @art-w and @clecat). This allows resource pools to be shared between domains easily. - Add `Eio.Path.read_link` (@talex5 ocaml-multicore/eio#686). - Add `Eio_unix.Fd.is_open` (@talex5 ocaml-multicore/eio#690). - Include backtrace in systhread errors (@talex5 ocaml-multicore/eio#688, reviewed by @SGrondin). Also, add `Eio.Exn.empty_backtrace` as a convenience. - eio.mock: add tracing support to mock backend (@talex5 ocaml-multicore/eio#687). - Improve tracing (@talex5 ocaml-multicore/eio#675 ocaml-multicore/eio#683 ocaml-multicore/eio#676, reviewed by @SGrondin). Update tracing section of README and trace more things (`run_in_systhread`, `close`, `submit`, `traceln`, cancellation and domain spawning). Documentation: - Link to verification work in docs (@talex5 ocaml-multicore/eio#682). - Add more trace diagrams to README (@talex5 ocaml-multicore/eio#698). - Adjust COC contacts (@polytypic ocaml-multicore/eio#685, reviewed by @Sudha247). Bug fixes: - eio_linux: retry `openat2` on `EAGAIN` (@talex5 ocaml-multicore/eio#693, reviewed by @SGrondin). - eio_posix and eio_windows: check for IO periodically (@talex5 ocaml-multicore/eio#674). - Handle EPERM when trying to initialise uring (@talex5 ocaml-multicore/eio#691). This can happen when using a Docker container. Build and tests: - Benchmark `Eio_unix.run_in_systhread` (@talex5 ocaml-multicore/eio#678, reviewed by @SGrondin). - Enable lintcstubs for `Eio_unix.Private` too (@talex5 ocaml-multicore/eio#689). - Stat benchmark: report cleanup time and optimise (@talex5 ocaml-multicore/eio#692). - Make benchmarks start faster (@talex5 ocaml-multicore/eio#673). - Update build for new eio-trace CLI (@talex5 ocaml-multicore/eio#699). - Expect opam-repo-ci tests to fail on macos (@talex5 ocaml-multicore/eio#672).
This matches the change to the same test in ocaml-multicore/eio#684.
This is needed to allow resource pools to be shared between domains. Fixes #669.
It works by putting a mutex around the
on_release
list. I tried a lock-free version too, but it was quite messy.There is one change to the existing behaviour: it used to be possible to attach new resources to a switch in a release handler, but now that is rejected. I don't think the old behaviour was useful, but it could be brought back if needed (we'd just need to take the mutex again at the end).
I was thinking of deprecating
remove_hook
in favour oftry_remove_hook
, but all existing uses were safe so I decided not to do that for now.Note: There are some places where we do
Switch.check
before creating a resource. This isn't thread-safe, and might pass even after the switch has ended if called from a different domain. That shouldn't be a problem (the switch could always have ended after the check anyway), but might annoy TSan.