-
Notifications
You must be signed in to change notification settings - Fork 26
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
Use AtomicU64
instead of u64 for the position field in Receiver
.
#66
Comments
Thanks. Atomics always have a cost so best to leave it to the user if they're ok with paying that cost. |
@zeenix I agree about the cost of using |
I'd believe so (even though I don't have sufficient data to back it up), yes. However, each task can own a clone of the receiver and don't necessarily need to use interior mutability.
If you need interior mutability in your design, yes.
Perhaps but I don't know how they both compare in terms of performance. If you've some data on this, I'd be very curious to know. |
@zeenix Sorry for the late response here.
That's indeed true, but most of the time the task will be spawned inside a method of a struct, and usually the struct will own the receiver (that's how I use it :)). So, I think using interior mutability is important most of the time
I will open a draft pull request soon with some benchmarks. |
But why does that necessitate interior mutability? You need mutability on the struct level then. |
👍 |
Here is an example: use std::sync::Arc;
struct Task {
stop_signal: async_channel::Receiver<String>,
b_stop_signal: async_broadcast::Receiver<String>,
}
impl Task {
async fn run(self: Arc<Self>) {
smol::spawn({
async move {
loop {
let msg = self.stop_signal.recv().await;
}
}
})
.detach();
}
async fn run2(self: Arc<Self>) {
smol::spawn({
async move {
loop {
// XXX this will not work.
let msg = self.b_stop_signal.recv().await;
}
}
})
.detach();
}
} |
Thanks but I don't need examples of cases where you need interior mutability. In my own project, I need interior mutability. My point was that it all depends on the design. You don't always have to put things in an The question is how much faster are atomic locks compared to (async) mutexes/rwlocks to justify:
|
Yes indeed, I was just showing an example of the need for interior mutability.
I am aware that there might be breaking changes in the API. I need to research more about that and conduct some benchmarks.
That's amazing news! |
To be honest, I can't imagine any case where mutability is needed. |
The `recv()`, `recv_direct()`, `recv_blocking()`, and `try_recv()` methods currently require `&mut self` to modify the value of the `position` field. By using AtomicU64 for the `position` field eliminates the need for mutability. Fixes issue smol-rs#66
The `recv()`, `recv_direct()`, `recv_blocking()`, and `try_recv()` methods currently require `&mut self` to modify the value of the `position` field. By using AtomicU64 for the `position` field eliminates the need for mutability. Fixes issue smol-rs#66
I will close this issue due to the performance regression when using atomic operations. Check out the benchmarks in PR #70 for more details. |
Hello,
I noticed that the only reason
recv()
,recv_direct()
, andtry_recv()
need to take&mut self
to modify the value ofself.pos
. IfAtomicU64
is used instead, there will be no need to use&mut self
there.The text was updated successfully, but these errors were encountered: