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

Integrate io_uring in the Reactor #85

Closed
wants to merge 2 commits into from

Conversation

notgull
Copy link
Member

@notgull notgull commented Aug 18, 2022

This PR resolves #39 by adding an alternative Reactor that uses io_uring in addition to epoll as a backend on Linux.

Changes this PR makes to the architecture of async-io:

  • reactor.rs is turned into a folder, containing two implementations of Reactor that are selected at compile time through cfg-if.
  • The previous implementation of the Reactor has been moved to poll.rs. It is used on non target_os = "linux" platforms.
  • A new implementation of Reactor in uring.rs is present. Instead of just using polling, it uses io_uring, but uses that system to wait for readiness on epoll.
  • Reactor has two new methods: poll_read and poll_write, which are now called by Async's impls of AsyncRead and AsyncWrite. On the polling Reactor, it is the same as the previous implementation. On the new Reactor, it files new Read/Write entries in the submission queue.
  • If io_uring isn't available, it falls back to the previous reactor.

Reasons why I'm filing it as a draft instead of as a full PR:

Current issues:

  • In order to ensure everything is sound, I keep an internal per-Source buffer that the io_uring operations read from/write to, instead of the buffers passed in by the user. This is because the io_uring temporarily takes ownership of the buffer, and that's invalid to do with the current model. However, for large reads/writes, this may incur a performance penalty. Open to suggestions on how to fix this.
  • The system currently seems to spuriously error with "Operation Cancelled". I'm not sure if this is a problem with the io_uring crate or our implementation.
  • We currently submit per-operation. We should only submit if there is an ongoing wait.

Potential future work:

  • io_uring supports quite a few different operations that we could also integrate into it.
  • We could use the runtime to also support asynchronous file reading/writing if io_uring is available.
  • Do the same thing but with IOCP on Windows.

@notgull notgull mentioned this pull request Aug 21, 2022
@notgull
Copy link
Member Author

notgull commented Aug 24, 2022

Benchmarks are showing a pretty significant regression when using them to see how fast read and write are now. Since the "main" reason for being able to do this are quick reads and writes, this means that this PR is now obsolete and should be closed.

However, something else we should consider is: would something of this variety be worth it if we wanted to integrate regular files into async-io? That's a question that should probably have some more discussion put into it.

@notgull notgull closed this Aug 24, 2022
@sehz
Copy link

sehz commented Aug 24, 2022

This is too bad. Is performance regression due to using Reactor?

@notgull
Copy link
Member Author

notgull commented Aug 25, 2022

@sehz I'm not entirely sure, but needing to deal with both epoll and io-uring at once led to most of the problems.

My current plan now is to register io-uring to an EventFd that we then register into an Async to wait on file I/O. The good part about this is I can just turn it into another crate separate from async-io.

@sehz
Copy link

sehz commented Sep 25, 2023

any update?

@notgull
Copy link
Member Author

notgull commented Sep 25, 2023

any update?

I'm trying not to add any new features to smol, unless they're important. The value proposition of io_uring is speed; however when it comes to doing polling (which is the primary value of async-io) I've found that io_uring doesn't offer a real speed advantage over epoll, at least at smaller scales.

It would be nice to have more efficient file I/O, but there would really be two paths here:

  • Develop a wrapper around io_uring for async and make that an external crate, that we can then later integrate into async-fs. See Conditionally use more efficient strategies for polling file I/O async-fs#24
  • Add an alternative backend to polling that uses io_uring. It can't replace epoll, as epoll supports level and edge triggered modes that io_uring doesn't. In addition, it adds a layer of complexity to polling that I'm uncomfortable having.

I don't have the time or motivation for option number one, and option number two has serious drawbacks. So unless someone else spearheads the effort, I think we're fine with epoll for now.

@sehz
Copy link

sehz commented Sep 25, 2023

A little bit surprised to see no improvement in performance. I do agree however, it probably doesn't make sense to replace epoll since io_uring uses a fundamentally different I/O model. So it better to create a different library

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

io_uring support
2 participants