Skip to content

Draft: tor-chanmgr: Soften FIXME about Channel, Arc and Clone

Ian Jackson requested to merge Diziet/arti:chanel-arc-clone-fixme into main

The comment says Arc is not needed because Channel is Clone. That is true.

But the code seems to clone the Arc quite a lot, and Channel is nontrivial to clone. Indeed, it even directly contains an Arc! (As well as various cloneable interthread comms objects.) So cloning the Channel is definitely more expensive than cloning an Arc that would wrap it. In the absence of benchmarking, I would guess that this outweighs the extra indirection.

The fact that Arc is open-coded everywhere at the call sites is unfortunate. It might be worth considering moving the Arc into channel.rs. Ie, deleting all the ad-hoc Arc<> and instead, have in channel.rs's type Channel = Arc<ChannelContents>.

So err, I guess this MR is a way to prompt a conversation about this. I took a quick look at actually doing as my new FIXME suggests, and it involves a lot of quite formulaic edits. I'm not sure this is particularly urgent, so perhaps for now we should just leave this modified FIXME in place.

Edited by Ian Jackson

Merge request reports

Loading