tokio icon indicating copy to clipboard operation
tokio copied to clipboard

Implement Clone for watch::Sender to make it multi-producer

Open madadam opened this issue 2 years ago • 9 comments

Currently watch is single-producer but looking at the source of Sender, it seems it would be trivial to change it to multi-producer - just slap a #[derive(Clone)] on it. I think this would make it more useful as it would be possible to watch multiple sources. It's already possible to put the Sender into Arc, but it's inefficient because it's essentially an Arc in an Arc.

Would the tokio maintainers be interested in a PR to add this?

madadam avatar Jul 06 '22 09:07 madadam

Sounds good to me, but the impl Drop for Sender will need to be updated.

Darksonn avatar Jul 06 '22 10:07 Darksonn

Good point, I haven't considered it. Guess that makes it not quite as trivial as I though...

madadam avatar Jul 06 '22 10:07 madadam

I think to properly implement this, we would have to track the number of senders instead of just a single bit indicating closed channel. Thus we won't be able to pack the information about whether the channel is closed into the same atomic as the version and we would need a separate atomic. That would make the state loading operation no longer atomic (because it would require two separate atomic loads). I'm not sure what implication that would have. I think it should be fine because the two values are independent, but I might be missing something.

madadam avatar Jul 06 '22 12:07 madadam

FWIW, the number of Receivers is already tracked in a separate atomic...https://github.com/tokio-rs/tokio/blob/4daeea8cad1ce8e67946bc0e17d499ab304b5ca2/tokio/src/sync/watch.rs#L179-L180 but, that isn't read in operations that also care about the version.

Another potential option, which would maintain the atomicity of operations on the version/sender closedness state, would be to use half of a a word to store the sender refcount, and the other half to store the version. I think 32 bits of ref-count would be plenty (nobody's gonna clone a sender 4,294,967,295 times, right...?), but we might want to be able to handle the version wrapping around. And, we would probably need to change the state to be an explicitly-64-bit atomic --- 65,535 clones/versions on 32-bit platforms seems kinda cramped...

hawkw avatar Jul 06 '22 18:07 hawkw

Yeah, that might work although I'm not sure 32 bits is enough for the version (but maybe some wrap-around tricks can be used). But I wonder, do they actually need to be packed into a single atomic? Is there some other reason for it apart from space saving?

madadam avatar Jul 12 '22 08:07 madadam

If the counter is only used to know when all senders are gone, then it does not need to be packed together.

Darksonn avatar Jul 12 '22 14:07 Darksonn

But currently the flag indicating whether the channel is closed is packed together with the version. So I wonder whether that's just to save some space or whether there is a deeper reason.

madadam avatar Jul 12 '22 14:07 madadam

Packing the closed flag together with the version is indeed done for space reasons.

Darksonn avatar Jul 12 '22 16:07 Darksonn

I've started work on this feature here: https://github.com/tokio-rs/tokio/pull/5936

Comments welcome, especially if there's other features/methods that need to be added while I'm at it.

bouk avatar Aug 16 '23 09:08 bouk