-
-
Notifications
You must be signed in to change notification settings - Fork 14.1k
std: Use more unix.rs code on WASI targets
#147572
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
Conversation
|
rustbot has assigned @Mark-Simulacrum. Use |
This comment has been minimized.
This comment has been minimized.
c87de05 to
cb4a430
Compare
|
At a high level this seems OK (cc @joboet as you've been working on a bunch of sys cleanups). If I'm understanding the new code right, it looks like the future we'd expect is to remove the wasi modules entirely in favor of wasipN being cfg(unix) for the most part? Does that understanding seem right? I see that a bunch of the code "added" here seems like it's copy/paste from cfg(unix) modules counterparts. For example, maybe library/std/src/sys/io/io_slice/wasi.rs should already be removed and cfgs rerouted to the iovec module, with the extra methods gated on cfgs in that? I think that would also apply to other similar modules at this point. Diffing locally ( |
|
That's a good point! You have also made me realize that most of the fs implementation is entirely redundant as well. Almost all of the functions the unix variant uses are implemented by wasi-libc which means we could use wasi-libc directly. The original reason for wasi having its own implementation was to implement everything natively in Rust which requires handling preopens to translate "open this path" to "open this sub-path rooted from this fd", or changing The main difference between wasi and unix comes down to extensions where on Unix there's all sorts of extra symbols/extensions/etc. That could be handled relatively easily internally though with Lemme test out those ideas and see if I can't delete even more code here. |
|
Ok I think that actually worked out really well -- alexcrichton@6760496 -- so I'm going to switch this PR to taking that approach. Effectively WASIp{1,2,3} will look like "unix" for operations related to the filesystem, threads, timers, etc. There's still platform differences for things like getting arguments and other bits here and there but I think those, in the limit of time, can go through libc as well. Effectively wasi-libc has matured/changed enough to the point that I think it's more reasonable to go through that for various calls and that should reduce the size of the WASI-specific burden in the standard library. Additionally this reduces the distinction between WASIp1 and WASIp2 even further to by unifying their "pal" module into one since there's virtually no difference now. Overall this feels like a much better state of affairs to me. This all depends on various |
This commit fills out definitions in libc for rust-lang/rust#147572 notably filling out some fs-related functions as well as many pthread-related functions. The pthread-related functions were not available originally with wasi-libc but nowadays are stubs for single-threaded behavior. The goal is to make wasi targets more "unix like" in libstd and have less WASI-specific code.
This commit fills out definitions in libc for rust-lang/rust#147572 notably filling out some fs-related functions as well as many pthread-related functions. The pthread-related functions were not available originally with wasi-libc but nowadays are stubs for single-threaded behavior. The goal is to make wasi targets more "unix like" in libstd and have less WASI-specific code.
|
☔ The latest upstream changes (presumably #147838) made this pull request unmergeable. Please resolve the merge conflicts. |
|
I've updated this and code-wise it's where I'm pretty comfortable. This is still blocked on a |
This commit fills out definitions in libc for rust-lang/rust#147572 notably filling out some fs-related functions as well as many pthread-related functions. The pthread-related functions were not available originally with wasi-libc but nowadays are stubs for single-threaded behavior. The goal is to make wasi targets more "unix like" in libstd and have less WASI-specific code. (backport <rust-lang#4747>) (cherry picked from commit 702efb9)
This commit fills out definitions in libc for rust-lang/rust#147572 notably filling out some fs-related functions as well as many pthread-related functions. The pthread-related functions were not available originally with wasi-libc but nowadays are stubs for single-threaded behavior. The goal is to make wasi targets more "unix like" in libstd and have less WASI-specific code. (backport <rust-lang#4747>) (cherry picked from commit 702efb9)
This commit fills out definitions in libc for rust-lang/rust#147572 notably filling out some fs-related functions as well as many pthread-related functions. The pthread-related functions were not available originally with wasi-libc but nowadays are stubs for single-threaded behavior. The goal is to make wasi targets more "unix like" in libstd and have less WASI-specific code. (backport <#4747>) (cherry picked from commit 702efb9)
cb4a430 to
ffb3ff1
Compare
|
Looks like I forgot to actually push last time... I've rebased and additionally deleted the wasip2-specific network-related module so now it also shares |
libc for filesystem ops on WASI targetsunix.rs code on WASI targets
This comment has been minimized.
This comment has been minimized.
|
☔ The latest upstream changes (presumably #144465) made this pull request unmergeable. Please resolve the merge conflicts. |
This commit is a large change to the implementation of filesystem and
other system-related operations on WASI targets. Previously the standard
library explicitly used the `wasi` crate at the 0.11.x version track
which means that it used WASIp1 APIs directly. This meant that `std` was
hard-coded to use WASIp1 syscalls and there was no separate
implementation for the WASIp{2,3} targets, for example. The high-level
goal of this commit is to decouple this interaction and avoid the use of
the `wasi` crate on the WASIp2 target.
Historically when WASIp1 was originally added to Rust the wasi-libc
library was in a much different position than it is today. Nowadays Rust
already depends on wasi-libc on WASI targets for things like memory
allocation and environment variable management. As a libc library it
also has all the functions necessary to implement all filesystem
operations Rust wants. Recently wasi-libc additionally was updated to
use WASIp2 APIs directly on the `wasm32-wasip2` target instead of using
`wasm32-wasip1` APIs. This commit is leveraging this work by enabling
Rust to completely sever the dependence on WASIp1 APIs when compiling
for `wasm32-wasip2`. This is also intended to make it easier to migrate
to `wasm32-wasip3` internally in the future where now only libc need be
updated and Rust doesn't need to explicitly change as well.
The overall premise of this commit is that there's no need for
WASI-specific implementation modules throughout the standard library.
Instead the libc-style bindings already implemented for Unix-like
targets are sufficient. This means that Rust will now be using
libc-style interfaces to interact with the filesystem, for example, and
wasi-libc is the one responsible for translating these POSIX-ish
functions into WASIp{1,2} calls.
Concrete changes here are:
* `std` for `wasm32-wasip2` no longer depends on `wasi 0.11.x`
* The implementation of `std::os::wasi::fs`, which was previously
unstable and still is, now has portions gated to only work on the
WASIp1 target which use the `wasi` crate directly. Traits have been
trimmed down in some cases, updated in others, or now present a
different API on WASIp1 and WASIp2. It's expected this'll get further
cleanup in the future.
* The `std::sys::fd::wasi` module is deleted and `unix` is used instead.
* The `std::sys::fs::wasi` module is deleted and `unix` is used instead.
* The `std::sys::io::io_slice::wasi` module is deleted and `unix` is used
instead.
* The `std::sys::pal::{wasip1,wasip2}` modules are now merged together
as their difference is much smaller than before.
* The `std::sys::pal::wasi::time` is deleted and the `unix` variant is
used directly instead.
* The `std::sys::stdio::wasip{1,2}` modules are deleted and the `unix`
variant is used instead.
* The `std::sys::thread::wasip{1,2}` modules are deleted and the `unix`
variant is used instead.
Overall Rust's libstd is effectively more tightly bound to libc when
compiled to WASI targets. This is intended to mirror how it's expected
all other languages will also bind to WASI. This additionally has the
nice goal of drastically reducing the WASI-specific maintenance burden
in libstd (in theory) and the only real changes required here are extra
definitions being added to `libc` (done in separate PRs). This might be
required for more symbols in the future but for now everything should be
mostly complete.
805a23c to
ba46286
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
@bors: r=Mark-Simulacrum |
…-Simulacrum
std: Use more `unix.rs` code on WASI targets
This commit is a large change to the implementation of filesystem and
other system-related operations on WASI targets. Previously the standard
library explicitly used the `wasi` crate at the 0.11.x version track
which means that it used WASIp1 APIs directly. This meant that `std` was
hard-coded to use WASIp1 syscalls and there was no separate
implementation for the WASIp{2,3} targets, for example. The high-level
goal of this commit is to decouple this interaction and avoid the use of
the `wasi` crate on the WASIp2 target.
Historically when WASIp1 was originally added to Rust the wasi-libc
library was in a much different position than it is today. Nowadays Rust
already depends on wasi-libc on WASI targets for things like memory
allocation and environment variable management. As a libc library it
also has all the functions necessary to implement all filesystem
operations Rust wants. Recently wasi-libc additionally was updated to
use WASIp2 APIs directly on the `wasm32-wasip2` target instead of using
`wasm32-wasip1` APIs. This commit is leveraging this work by enabling
Rust to completely sever the dependence on WASIp1 APIs when compiling
for `wasm32-wasip2`. This is also intended to make it easier to migrate
to `wasm32-wasip3` internally in the future where now only libc need be
updated and Rust doesn't need to explicitly change as well.
The overall premise of this commit is that there's no need for
WASI-specific implementation modules throughout the standard library.
Instead the libc-style bindings already implemented for Unix-like
targets are sufficient. This means that Rust will now be using
libc-style interfaces to interact with the filesystem, for example, and
wasi-libc is the one responsible for translating these POSIX-ish
functions into WASIp{1,2} calls.
Concrete changes here are:
* `std` for `wasm32-wasip2` no longer depends on `wasi 0.11.x`
* The implementation of `std::os::wasi::fs`, which was previously
unstable and still is, now has portions gated to only work on the
WASIp1 target which use the `wasi` crate directly. Traits have been
trimmed down in some cases, updated in others, or now present a
different API on WASIp1 and WASIp2. It's expected this'll get further
cleanup in the future.
* The `std::sys::fd::wasi` module is deleted and `unix` is used instead.
* The `std::sys::fs::wasi` module is deleted and `unix` is used instead.
* The `std::sys::io::io_slice::wasi` module is deleted and `unix` is used
instead.
* The `std::sys::pal::{wasip1,wasip2}` modules are now merged together
as their difference is much smaller than before.
* The `std::sys::pal::wasi::time` is deleted and the `unix` variant is
used directly instead.
* The `std::sys::stdio::wasip{1,2}` modules are deleted and the `unix`
variant is used instead.
* The `std::sys::thread::wasip{1,2}` modules are deleted and the `unix`
variant is used instead.
Overall Rust's libstd is effectively more tightly bound to libc when
compiled to WASI targets. This is intended to mirror how it's expected
all other languages will also bind to WASI. This additionally has the
nice goal of drastically reducing the WASI-specific maintenance burden
in libstd (in theory) and the only real changes required here are extra
definitions being added to `libc` (done in separate PRs). This might be
required for more symbols in the future but for now everything should be
mostly complete.
Rollup of 12 pull requests Successful merges: - #147572 (std: Use more `unix.rs` code on WASI targets) - #148491 ( Correctly provide suggestions when encountering `async fn` with a `dyn Trait` return type) - #149215 (Emit `check-cfg` lints during attribute parsing rather than evaluation) - #149652 (Add release notes for 1.92.0) - #149720 (rustdoc book: mention inner doc attribute) - #149730 (lint: emit proper diagnostic for unsafe binders in improper_ctypes instead of ICE) - #149754 (Retire `opt_str2` from compiletest cli parsing) - #149755 (bootstrap: Use a `CompiletestMode` enum instead of bare strings) - #149763 (Add inline attribute to generated delegation function if needed) - #149772 (test: Add a test for 146133) - #149779 (Fix typo "an" → "and") - #149782 (Remove `[no-mentions]` handler in the triagebot config) r? `@ghost` `@rustbot` modify labels: rollup
|
☀️ Test successful - checks-actions |
What is this?This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.Comparing a371038 (parent) -> 018d269 (this PR) Test differencesShow 2 test diffs2 doctest diffs were found. These are ignored, as they are noisy. Test dashboardRun cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard 018d26972e523b8d28f9579056185a1713949dfd --output-dir test-dashboardAnd then open Job duration changes
How to interpret the job duration changes?Job durations can vary a lot, based on the actual runner instance |
|
Finished benchmarking commit (018d269): comparison URL. Overall result: ❌✅ regressions and improvements - please read the text belowOur benchmarks found a performance regression caused by this PR. Next Steps:
@rustbot label: +perf-regression Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary 2.3%, secondary 1.8%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary -2.1%, secondary -5.6%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 470.343s -> 472.223s (0.40%) |
|
Not fully sure about the change, but it seems the recent fn main() {
std::fs::hard_link("/tmp/1.txt", "/tmp/2.txt").unwrap();
}Plus, the very same problem affects also Can you please take a look? |
|
Looks like that's a bug in wasi-libc, which I've proposed #149864 as a workaround for now. |
|
Thank you for the quick fix of the issue. |
This commit is a large change to the implementation of filesystem and
other system-related operations on WASI targets. Previously the standard
library explicitly used the
wasicrate at the 0.11.x version trackwhich means that it used WASIp1 APIs directly. This meant that
stdwashard-coded to use WASIp1 syscalls and there was no separate
implementation for the WASIp{2,3} targets, for example. The high-level
goal of this commit is to decouple this interaction and avoid the use of
the
wasicrate on the WASIp2 target.Historically when WASIp1 was originally added to Rust the wasi-libc
library was in a much different position than it is today. Nowadays Rust
already depends on wasi-libc on WASI targets for things like memory
allocation and environment variable management. As a libc library it
also has all the functions necessary to implement all filesystem
operations Rust wants. Recently wasi-libc additionally was updated to
use WASIp2 APIs directly on the
wasm32-wasip2target instead of usingwasm32-wasip1APIs. This commit is leveraging this work by enablingRust to completely sever the dependence on WASIp1 APIs when compiling
for
wasm32-wasip2. This is also intended to make it easier to migrateto
wasm32-wasip3internally in the future where now only libc need beupdated and Rust doesn't need to explicitly change as well.
The overall premise of this commit is that there's no need for
WASI-specific implementation modules throughout the standard library.
Instead the libc-style bindings already implemented for Unix-like
targets are sufficient. This means that Rust will now be using
libc-style interfaces to interact with the filesystem, for example, and
wasi-libc is the one responsible for translating these POSIX-ish
functions into WASIp{1,2} calls.
Concrete changes here are:
stdforwasm32-wasip2no longer depends onwasi 0.11.xstd::os::wasi::fs, which was previouslyunstable and still is, now has portions gated to only work on the
WASIp1 target which use the
wasicrate directly. Traits have beentrimmed down in some cases, updated in others, or now present a
different API on WASIp1 and WASIp2. It's expected this'll get further
cleanup in the future.
std::sys::fd::wasimodule is deleted andunixis used instead.std::sys::fs::wasimodule is deleted andunixis used instead.std::sys::io::io_slice::wasimodule is deleted andunixis usedinstead.
std::sys::pal::{wasip1,wasip2}modules are now merged togetheras their difference is much smaller than before.
std::sys::pal::wasi::timeis deleted and theunixvariant isused directly instead.
std::sys::stdio::wasip{1,2}modules are deleted and theunixvariant is used instead.
std::sys::thread::wasip{1,2}modules are deleted and theunixvariant is used instead.
Overall Rust's libstd is effectively more tightly bound to libc when
compiled to WASI targets. This is intended to mirror how it's expected
all other languages will also bind to WASI. This additionally has the
nice goal of drastically reducing the WASI-specific maintenance burden
in libstd (in theory) and the only real changes required here are extra
definitions being added to
libc(done in separate PRs). This might berequired for more symbols in the future but for now everything should be
mostly complete.