Skip to content

Allow consumers to implement POSIX AIO sources. #3841

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

Closed
wants to merge 1 commit into from

Conversation

asomers
Copy link
Contributor

@asomers asomers commented Jun 6, 2021

Unlike every other kqueue event source, POSIX AIO registers events not
via kevent(2) but by a different mechanism that needs the kqueue's file
descriptor. This commit adds a new PollAio type, designed to be used
by an external crate that implements a POSIX AIO Future for use with
Tokio's reactor.

Fixes: #3197

Motivation

With Tokio 0.1 and 0.2, PollEvented was sufficiently powerful to allow external crates to implement a POSIX AIO event source. Beginning with Tokio 0.3, PollEvented became private, and it also lost some needed functionality. Thus, there was no possible way for a consumer to use POSIX AIO with Tokio.

Solution

This PR restores that ability. While leaving the actual implementation for an external crate, it adds just enough functionality for the external crate to hook in. It deliberately does not expose all of PollEvented, so as not to encourage consumers to use that type over AsyncFd. Instead, it creates a new, dedicated type.

Unlike every other kqueue event source, POSIX AIO registers events not
via kevent(2) but by a different mechanism that needs the kqueue's file
descriptor.  This commit adds a new `PollAio` type, designed to be used
by an external crate that implements a POSIX AIO Future for use with
Tokio's reactor.

Fixes: tokio-rs#3197
@Darksonn Darksonn requested a review from carllerche June 6, 2021 08:29
@Darksonn Darksonn added A-tokio Area: The main tokio crate M-net Module: tokio/net labels Jun 6, 2021
@carllerche
Copy link
Member

We cannot expose mio types from Tokio at this time (due to the stability differences). Even once Mio hits 1.0, it is not clear that we will.

The best strategy to expose aio will be to expose the operations proper (e.g. aio_read). The challenge then becomes figuring out the API to expose. Should the "raw" syscalls be exposed but with the kqueue FD hidden? What does portability look like? etc...

@carllerche
Copy link
Member

Another option would be to provide an alternate AIO back-end implementation for existing Tokio types (e.g. File). Though, this would require an additional copy.

@carllerche
Copy link
Member

This seems like a similar problem to exposing io-uring, which is being being worked on externally: tokio-rs/tokio-uring#1

@asomers
Copy link
Contributor Author

asomers commented Jun 14, 2021

Well yes, the ultimate goal is to merge the AIO stuff into tokio::fs, as an OS-specific implementation of write_at, read_at, writev_at, readv_at, sync_all, and sync_all_data (see #1529). I'm not sure why that would require an additional copy?

But my concern is that adding all of these methods right away would create more API-instability than adding PollAio, since there would likely be minor problems with the first attempt, and the API would require revision. That's why I want to expose PollAio - as a temporary way to refine the file API outside of the main Tokio crate.

Since it's only temporary, what if I rename the "aio" feature into "aio-unstable" or something like that?

@asomers
Copy link
Contributor Author

asomers commented Jun 14, 2021

Oh, and one other problem. readv_at and writev_at can't be properly implemented right now (rust-lang/libc#570). They must be faked by using lio_listio. But the semantics are subtly different than what a proper implementation would offer. So if we try to fully integrate AIO into tokio::fs now, its behavior (if not API) will subtly change when that issue is fixed.

@Darksonn
Copy link
Contributor

Since the strategy in this PR is not feasible, I am closing the PR. You should feel free to reopen or open a new PR if you find a way to address our concerns.

@Darksonn Darksonn closed this Jun 28, 2021
@asomers
Copy link
Contributor Author

asomers commented Jun 28, 2021

@Darksonn I can't figure out what you would be willing to accept. This was the smallest and most targeted change I could come up with that would get the job done. Rather than expose mio::Source, I could create a new type tokio::AioSource. It would basically just wrap mio::Source, but perhaps remove the reregister and deregister events. It would be pure syntactic salt, however, and would necessitate much more code that this this PR includes.
Or, I could merge all of the AIO stuff into Tokio proper, rather than leaving it in a separate crate. You might be more comfortable with that API, but it would be a much larger change. I have a WIP of this concept, which you can preview if you like. It adds a FileExt trait, with methods like FileExt::write_at<'a>(&'a self, buf: &'a [u8], ofs: u64) -> super::os::WriteAt<'a>; These methods use POSIX AIO on FreeBSD (and potentially on NetBSD and OSX), but blocking threads on other OSes.
https://github.com/tokio-rs/tokio/compare/master...asomers:aio3?expand=1#diff-7d6d5b49c03d5300afa50531aadc3d0ab5b037c2af18a6b65609c58c41592b59

@Darksonn
Copy link
Contributor

While small changes are good, using traits or types from mio in our public API is not going to be acceptable as they make it impossible to upgrade past mio version 0.7.x. You will find a similar story if you look at how named pipes got included in Tokio — the resulting change there is also rather large.

@asomers
Copy link
Contributor Author

asomers commented Jun 28, 2021

So which alternative do you prefer? Adding a tokio::AioSource, or merging all of tokio_file into tokio in one fell swoop?

@Darksonn
Copy link
Contributor

I don't have a clear enough image of how those options would look in detail to answer that. Do you have a sketch of how an tokio::io::AioSource would look?

@asomers
Copy link
Contributor Author

asomers commented Jun 28, 2021

tokio::AioSource would probably look exactly like mio::event::Source, except that it wouldn't require the deregsiter and reregister methods. Just register would suffice. It also could potentially eliminate the interests argument. And it could even replace the Registry argument with RawFd.

@Darksonn
Copy link
Contributor

Darksonn commented Jun 28, 2021

Could it be done using only the RawFd trait like we do for AsyncFd?

Another approach is to use the same idea as we are using for io_uring where you can register the uring with Tokio using AsyncFd on the uring file descriptor. Then you listen for events on the ring using the AsyncFd, and manually emit syscalls to read information about those events from the uring. Could you do the same here?

@asomers
Copy link
Contributor Author

asomers commented Jun 28, 2021

Could it be done using only the RawFd trait like we do for AsyncFd?

It's not enough for Tokio to expose the kqueue's file descriptor. It also needs to provide a Token to register the AIO operation. What makes AIO unusual is that you don't register events with kevent(2). Instead, you register them by putting the kqueue's file descriptor and a Token in the struct that you pass to the AIO syscalls.

Another approach is to use the same idea as we are using for io_uring where you can register the uring with Tokio using AsyncFd on the uring file descriptor. Then you listen for events on the ring using the AsyncFd, and manually emit syscalls to read information about those events from the uring. Could you do the same here?

If I understand correctly, you're suggesting creating a separate kqueue just for AIO, and registering its file descriptor with Tokio's main kqueue, then polling it in a separate crate whenever Tokio's main kqueue says it's readable? I don't know if you can use kqueue that way. But even if you can, it would require duplicating an awful lot of Tokio's core reactor code. And it would be less efficient than using a single kqueue.

@carllerche
Copy link
Member

But even if you can, it would require duplicating an awful lot of Tokio's core reactor code. And it would be less efficient than using a single kqueue.

The long-term target is Tokio provides a nice API to cover the aio cases. We are mostly discussing the steps to get there. As I see it, there are two options:

a) implement as a separate lib using the strategy of registering a kqueue instance w/ AsyncFd and duping a bunch of reactor code.
b) Provide a (possibly) temporary API that exposes the kqueue FD and the token.

I think I'm OK with either if @Darksonn is. From my understanding, something like AsyncFd would work if it exposes the kqueue fd + the token, so it would look very roughly something like:

struct AioSource { ... }

impl AioSource {
    fn new() -> io::Result<AioSource> { ... }

    fn kqueue_fd(&self) -> RawFd { ... }

    // `Token` might end up just being `usize`.
    fn token(&self) -> Token { ... }

    async fn ready(&self, interest: Interest) -> AioReadyGuard { ... }
}

impl AioReadyGuard {
    fn readiness(&self) -> Ready { ... }
}

I would suggest opening a design issue w/ an API proposal before doing work either way.

@asomers
Copy link
Contributor Author

asomers commented Jul 11, 2021

@carllerche I'm not sure how you intend that AioSource would work. What I proposed in the comments above was to replace mio::Source with a new trait, named tokio::AioSource. It would basically just wrap mio::Source but slim down the interface to the bare minimum that AIO requires. But now you're proposing something different: a struct named AioSource. What would you have it do? Are you suggesting that Tokio should own all of the mio code? That would basically mean merging the mio-aio crate into Tokio. If not, then it seems like AioSource needs to be generic, and wrap the some struct from an external crate. But that's just what PollAio does. So I'm not sure what you want.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate M-net Module: tokio/net
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Need an interface to poll arbitrary kqueue filters in Tokio 0.3
3 participants