Rewrite the aio module #1713
Rewrite the aio module #1713
Conversation
|
Still in the draft state because it's waiting on a libc PR to merge. |
|
I actually was just looking at the existing aio implementation, so I'm glad I saw this. Some of the existing aio tests fail when run with |
|
So the tests are failing on FreeBSD because they use two symbols that are only defined on 13.0+, but we run CI on 12.3. A good solution would be weak symbol resolution. Unfortunately, that's unstable, and I can't get it to work anyway. But it's only the tests that are problems. The crate itself builds fine on FreeBSD 12.3, as long as no executables try to use aio_writev or aio_readv. So should we switch CI to 13.0? That would pose a very low risk that we'd accidentally break something on 12. A bigger risk would be that we inconvenience outside contributors who might be using 12. Or do we disable those tests by default, but perhaps enable them with a --cfg option that we set during CI? I'm not sure which is best. |
|
Should we just bite the bullet and build against multiple versions of FreeBSD as separate jobs? It doesn't mirror any of the other platforms, but neither does the ABI situation. We could just run tests against the latest, or invent some macro to indicate which versions of FreeBSD support a test. |
|
Given the volume of changes already made to |
What if we skip tests incompatible with FreeBSD 12.3 by default, add a manually-set cfg flag to enable them, similarly to how we do it for qemu, and run a second CI job on FreeBSD 14 ? |
Seems reasonable. |
|
Ok, this PR should be ready now. I left it un-squashed because one commit changes formatting, and the Redox failure is addressed in a separate PR. |
|
What's wrong with the current libc dep? I did update it from the asomers fork to the rust-lang one, if that's what you mean. |
|
Oh, I missed that. I don't see a new libc release, so yeah that's fine. |
The existing AIO implementation has some problems: 1) The in_progress field is checked at runtime, not compile time. 2) The mutable field is checked at runtime, not compile time. 3) A downstream lio_listio user must store extra state to track whether the whole operation is partially, completely, or not at all submitted. 4) Nix does heap allocation itself, rather than allowing the caller to choose it. This can result in double (or triple, or quadruple) boxing. 5) There's no easy way to use lio_listio to submit multiple operations with a single syscall, but poll each individually. 6) The lio_listio usage is far from transparent and zero-cost. 7) No aio_readv or aio_writev support. 8) priority has type c_int; should be i32 9) aio_return should return a usize instead of an isize, since it only uses negative values to indicate errors, which Rust represents via the Result type. This rewrite solves several problems: 1) Unsolved. I don't think it can be solved without something like C++'s guaranteed type elision. It might require changing the signature of Future::poll too. 2) Solved. 3) Solved, by the new in_progress method and by removing the complicated lio_listio resubmit code. 4) Solved. 5) Solved. 6) Solved, by removing the lio_listo resubmit code. It can be reimplemented downstream if necessary. Or even in Nix, but it doesn't fit Nix's theme of zero-cost abstractions. 7) Solved. 8) Solved. 9) Solved. The rewrite includes functions that don't work on FreeBSD, so add CI testing for FreeBSD 14 too. By default only enable tests that will pass on FreeBSD 12.3. But run a CI job on FreeBSD 14 and set a flag that will enable such tests.
|
bors r=rtzoeller |
The existing AIO implementation has some problems:
the whole operation is partially, completely, or not at all
submitted.
choose it. This can result in double (or triple, or quadruple)
boxing.
a single syscall, but poll each individually.
uses negative values to indicate errors, which Rust represents via
the Result type.
This rewrite solves several problems:
C++'s guaranteed type elision. It might require changing the
signature of Future::poll too.
lio_listio resubmit code.
reimplemented downstream if necessary. Or even in Nix, but it
doesn't fit Nix's theme of zero-cost abstractions.