Skip to content
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

eio_posix: use directory FDs instead of realpath #694

Merged
merged 1 commit into from
Feb 19, 2024

Conversation

talex5
Copy link
Collaborator

@talex5 talex5 commented Feb 15, 2024

realpath was an old hack from the libuv days and isn't safe against races with symlinks being updated.

This should be faster too in some cases.

Copy link
Collaborator

@SGrondin SGrondin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've read through it. I don't have enough knowledge of this area of the code to formally approve it, but this area is well tested and I have a codebase that performs a lot of file operations so I'll test this PR on it.

@talex5 talex5 force-pushed the posix-dirfd branch 5 times, most recently from 71e3bf9 to 4261f87 Compare February 19, 2024 12:44
realpath was an old hack from the libuv days.
@talex5
Copy link
Collaborator Author

talex5 commented Feb 19, 2024

The only noticeable change in the benchmarks is for the Path.stat rmtree time:

Before: Removed in 20.94 s
After: Removed in 6.65 s

@talex5 talex5 merged commit b9a0c20 into ocaml-multicore:main Feb 19, 2024
5 checks passed
@talex5 talex5 deleted the posix-dirfd branch February 19, 2024 15:36
@talex5
Copy link
Collaborator Author

talex5 commented Feb 19, 2024

I've merged this so we can get some more testing. Please file issues with any problems you notice!

talex5 added a commit to talex5/opam-repository that referenced this pull request Feb 22, 2024
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).
talex5 added a commit to talex5/opam-repository that referenced this pull request Feb 22, 2024
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).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants