| Age | Commit message (Collapse) | Author | Lines |
|---|
|
Avoid SeqCst or static mut in mach_timebase_info and QueryPerformanceFrequency caches
This patch went through a couple iterations but the end result is replacing a pattern where an `AtomicUsize` (updated with many SeqCst ops) guards a `static mut` with a single `AtomicU64` that is known to use 0 as a value indicating that it is not initialized.
The code in both places exists to cache values used in the conversion of Instants to Durations on macOS, iOS, and Windows.
I have no numbers to prove that this improves performance (It seems a little futile to benchmark something like this), but it's much simpler, safer, and in practice we'd expect it to be faster everywhere where Relaxed operations on AtomicU64 are cheaper than SeqCst operations on AtomicUsize, which is a lot of places.
Anyway, it also removes a bunch of unsafe code and greatly simplifies the logic, so IMO that alone would be worth it unless it was a regression.
If you want to take a look at the assembly output though, see https://godbolt.org/z/rbr6vn for x86_64, https://godbolt.org/z/cqcbqv for aarch64 (Note that this just the output of the mac side, but i'd expect the windows part to be the same and don't feel like doing another godbolt for it). There are several versions of this function in the godbolt:
- `info_new`: version in the current patch
- `info_less_new`: version in initial PR
- `info_original`: version currently in the tree
- `info_orig_but_better_orderings`: a version that just tries to change the original code's orderings from SeqCst to the (probably) minimal orderings required for soundness/correctness.
The biggest concern I have here is if we can use AtomicU64, or if there are targets that dont have it that this code supports. AFAICT: no. (If that changes in the future, it's easy enough to do something different for them)
r? `@Amanieu` because he caught a couple issues last time I tried to do a patch reducing orderings 😅
---
<details>
<summary>I rewrote this whole message so the original is inside here</summary>
I happened to notice the code we use for caching the result of mach_timebase_info uses SeqCst exclusively.
However, thinking a little more, it's actually pretty easy to avoid the static mut by packing the timebase info into an AtomicU64.
This entirely avoids needing to do the compare_exchange. The AtomicU64 can be read/written using Relaxed ops, which on current macos/ios platforms (x86_64/aarch64) have no overhead compared to direct loads/stores. This simplifies the code and makes it a lot safer too.
I have no numbers to prove that this improves performance (It seems a little futile to benchmark something like this), although it should do that on both targets it applies to.
That said, it also removes a bunch of unsafe code and simplifies the logic (arguably at least — there are only two states now, initialized or not), so I think it's a net win even without concrete numbers.
If you want to take a look at the assembly output though, see below. It has the new version, the original, and a version of the original with lower Orderings (which is still worse than the version in this PR)
- godbolt.org/z/obfqf9 x86_64-apple-darwin
- godbolt.org/z/Wz5cWc aarch64-unknown-linux-gnu (godbolt can't do aarch64-apple-ios but that doesn't matter here)
A different (and more efficient) option than this would be to just use the AtomicU64 and use the knowledge that after initialization the denominator should be nonzero... That felt like it's relying on too many things I'm not confident in, so I didn't want to do that.
</details>
|
|
|
|
|
|
Unbox mutexes and condvars on some platforms
Both mutexes and condition variables contained a Box containing the actual os-specific object. This was done because moving these objects may cause undefined behaviour on some platforms.
However, this is not needed on Windows[1], Wasm[2], cloudabi[2], and 'unsupported'[3], were the box was only needlessly making them less efficient.
This change gets rid of the box on those platforms.
On those platforms, `Condvar` can no longer verify it is only used with one `Mutex`, as mutexes no longer have a stable address. This was addressed and considered acceptable in #76932.
[1]\: https://docs.microsoft.com/en-us/windows/win32/api/synchapi/nf-synchapi-initializesrwlock
[2]\: These are just a single atomic integer together with futex wait/wake calls/instructions.
[3]\: The `unsupported` platform doesn't support multiple threads at all.
|
|
Add accessors to Command.
This adds some accessor methods to `Command` to provide a way to access the values set when building the `Command`. An example where this can be useful is to display the command to be executed. This is roughly based on the [`ProcessBuilder`](https://github.com/rust-lang/cargo/blob/13b73cdaf76b2d9182515c9cf26a8f68342d08ef/src/cargo/util/process_builder.rs#L105-L134) in Cargo.
Possible concerns about the API:
- Values with NULs on Unix will be returned as `"<string-with-nul>"`. I don't think it is practical to avoid this, since otherwise a whole separate copy of all the values would need to be kept in `Command`.
- Does not handle `arg0` on Unix. This can be awkward to support in `get_args` and is rarely used. I figure if someone really wants it, it can be added to `CommandExt` as a separate method.
- Does not offer a way to detect `env_clear`. I'm uncertain if it would be useful for anyone.
- Does not offer a way to get an environment variable by name (`get_env`). I figure this can be added later if anyone really wants it. I think the motivation for this is weak, though. Also, the API could be a little awkward (return a `Option<Option<&OsStr>>`?).
- `get_envs` could skip "cleared" entries and just return `&OsStr` values instead of `Option<&OsStr>`. I'm on the fence here. My use case is to display a shell command, and I only intend it to be roughly equivalent to the actual execution, and I probably won't display `None` entries. I erred on the side of providing extra information, but I suspect many situations will just filter out the `None`s.
- Could implement more iterator stuff (like `DoubleEndedIterator`).
I have not implemented new std items before, so I'm uncertain if the existing issue should be reused, or if a new tracking issue is needed.
cc #44434
|
|
Windows condition variables are movable (while not borrowed) according
to their documentation.
|
|
This commit keeps all condvars boxed on all platforms, but makes it
trivial to remove the box on some platforms later.
|
|
Windows SRW locks are movable (while not borrowed) according to their
documentation.
|
|
This commit keeps all mutexes boxed on all platforms, but makes it
trivial to remove the box on some platforms later.
|
|
|
|
|
|
|
|
- Module name can now be any string, not just an ident.
(Not all Windows api modules are valid Rust identifiers.)
- Adds c::FuncName::is_available() for checking if a function is really
available without having to do a duplicate lookup.
- Add comment explaining the lack of locking.
- Use `$_:block` to simplify the macro_rules.
- Apply allow(unused_variables) only to the fallback instead of
everything.
|
|
|
|
|
|
|
|
- Move `held` into the boxed part, since the SRW lock implementation
does not use this. This makes the Mutex 50% smaller.
- Use `Cell` instead of `UnsafeCell` for `held`, such that `.replace()`
can be used.
- Add some comments.
|
|
Adds missing descriptions for the modules std::os::linux::fs and std::os::windows::io.
Also adds punctuation for consistency with other descriptions.
|
|
|
|
Also doing fmt inplace as requested.
|
|
|
|
|
|
|
|
|
|
Call into fastfail on abort in libpanic_abort on Windows x86(_64)
This partially resolves #73215 though this is only for x86 targets. This code is directly lifted from [libstd](https://github.com/rust-lang/rust/blob/13290e83a6e20f3b408d177a9d64d8cf98fe4615/library/std/src/sys/windows/mod.rs#L315). `__fastfail` is the preferred way to abort a process on Windows as it will hook into debugger toolchains.
Other platforms expose a `_rust_abort` symbol which wraps `std::sys::abort_internal`. This would also work on Windows, but is a slightly largely change as we'd need to make sure that the symbol is properly exposed to the linker. I'm inlining the call to the `__fastfail`, but the indirection through `rust_abort` might be a cleaner approach.
A different instruction must be used on ARM architectures. I'd like to verify this works first before tackling ARM.
|
|
|
|
Raw standard streams constructors are infallible. Remove unnecessary
result type.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
