Conversation
|
Maybe want to ask the unresolved questions at the ACP thread? To make sure it's seen by those already involved. (I'll take a closer look later) |
Because But mpmc is multiple producer multiple consumer, so it doesn't matter how many threads For this one, they can both be unconditionally |
|
Thank you for the answer! Please let me know if I'm interpreting this right: If all methods where synchronization must occur consume |
c7e668b to
1a515fd
Compare
|
I haven't gotten a chance to look into this unfortunately r? libs |
|
r? libs (based on this comment) |
This comment has been minimized.
This comment has been minimized.
|
☔ The latest upstream changes (presumably #147928) made this pull request unmergeable. Please resolve the merge conflicts. |
|
It's been a little while so Feel free to reroll if you want but you've reviewed a lot of sync module things. |
library/std/src/sync/oneshot.rs
Outdated
| #[unstable(feature = "oneshot_channel", issue = "143674")] | ||
| impl<T> fmt::Debug for TryRecvError<T> { | ||
| fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { | ||
| "oneshot::TryRecvError(..)".fmt(f) |
There was a problem hiding this comment.
Somethings gone wrong here, this issue is back...
|
argh I accidentally dropped the commit instead of squashing it... Thank you for catching that |
The `oneshot` channel is gated under the `oneshot_channel` feature. Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Tests inspired by tests in the `oneshot` third-party crate.
|
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. |
|
@rustbot ready |
|
Great, thank you! |
|
☀️ 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 9e8a0c5 (parent) -> b7bcaa5 (this PR) Test differencesShow 67 test diffsStage 1
Stage 2
Additionally, 29 doctest diffs were found. These are ignored, as they are noisy. Job group index
Test dashboardRun cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard b7bcaa5c715ed07af2d74dac7ddb8786abeb4299 --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 (b7bcaa5): comparison URL. Overall result: ❌ regressions - no action needed@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.4%, secondary 2.4%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary -2.1%, secondary -2.6%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (primary -0.0%, secondary -0.0%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 474.474s -> 474.563s (0.02%) |
… r=workingjubilee Disable flaky test `oneshot::recv_timeout_before_send` This test is inherently flaky due to a thread-scheduling race condition, and has failed several times in CI, e.g.: - rust-lang#151739 (comment) - rust-lang#151971 (comment) - rust-lang#151376 (comment) --- - cc @connortsui20, author of rust-lang#143741
… r=workingjubilee Disable flaky test `oneshot::recv_timeout_before_send` This test is inherently flaky due to a thread-scheduling race condition, and has failed several times in CI, e.g.: - rust-lang#151739 (comment) - rust-lang#151971 (comment) - rust-lang#151376 (comment) --- - cc @connortsui20, author of rust-lang#143741
Rollup merge of #152145 - Zalathar:recv-timeout-before-send, r=workingjubilee Disable flaky test `oneshot::recv_timeout_before_send` This test is inherently flaky due to a thread-scheduling race condition, and has failed several times in CI, e.g.: - #151739 (comment) - #151971 (comment) - #151376 (comment) --- - cc @connortsui20, author of #143741
…gjubilee Disable flaky test `oneshot::recv_timeout_before_send` This test is inherently flaky due to a thread-scheduling race condition, and has failed several times in CI, e.g.: - rust-lang/rust#151739 (comment) - rust-lang/rust#151971 (comment) - rust-lang/rust#151376 (comment) --- - cc @connortsui20, author of rust-lang/rust#143741
Tracking Issue: #143674
This PR adds an experimental
oneshotmodule.Before talking about the API itself, I would prefer to get some of these questions below out of the way first. And as discussed in the ACP it would be
Unresolved Questions
Why exactly is it okay forSenderto beSync? Or basically, how do we boil down the discussion in ImplementSyncformpsc::Sender#111087 into a comment for theunsafe impl<T: Send> Sync for Sender<T> {}?Why ismpsc::Receiver!Syncbutmpmc::ReceiverisSync? Shouldoneshot::ReceiverbeSyncor not?is_readymethod as proposed in the tracking issue? If so, then the surface of this PR would likely need to increase to add apub(crate) fn is_disconnectedmethod tompmc(might even be a good idea to add that to all 3 channel flavors).mpmc, or should it be a wrapper around the internal crossbeam implementation?SenderandReceiveroperations be methods or associated methods? Sosender.send(msg)orSender::send(sender, msg)? The method syntax is more consistent with the rest of the ecosystem (namelytokio)