Remove unneeded clone #94
Conversation
| @@ -267,9 +266,10 @@ extern "C" fn init(callbacks: *mut PluginCallbacks, config_path: *const c_char) | |||
| message_threads | |||
| }; | |||
|
|
|||
| let mut senders = Vec::with_capacity(num_threads); | |||
There was a problem hiding this comment.
Creating Vec with Vec::with_capacity is a best practice when you know the size in advance.
From memory allocation point of view, I read there is no allocation when using Vec::new, there is an allocation for 4 items on the first push, then a new allocation with capacity x2 when the capacity is reached and so on.
Here nothing changes fundamentally, this code is executed when the plugin initialize, we may save an extra allocation if you have more than 4 cpus. :)
| for i in 0..num_threads { | ||
| let (messages_tx, messages_rx) = mpsc::sync_channel(0); | ||
| senders.push(messages_tx.clone()); | ||
| senders.push(messages_tx); |
There was a problem hiding this comment.
messages_tx is added to senders Vec, then the Vec is set on the static variable.
There is no need to clone messages_tx, this is not moved to the created thread. Only messages_rx is moved to the created thread.
| @@ -479,7 +479,7 @@ fn process_join(from: &Arc<Session>, room_id: RoomId, user_id: UserId, subscribe | |||
| switchboard.join_publisher(Arc::clone(from), user_id.clone(), room_id.clone()); | |||
| notify_except(¬ification, &user_id, switchboard.publishers_occupying(&room_id)); | |||
| } else { | |||
| switchboard.join_subscriber(Arc::clone(from), user_id.clone(), room_id.clone()); | |||
| switchboard.join_subscriber(Arc::clone(from), user_id, room_id); | |||
There was a problem hiding this comment.
If found this one with cargo clippy.
user_id and room_id are owned String from the MessageKind::Join message (serde DeserializeOwned) that are already moved to process_join. Those two String are not used after this, so no need to clone, join_subscriber can own them.
Note that in process_block, the whom String is not cloned, the String is moved to switchboard.establish_block without issue.
|
@mqp did you see this PR and the other one? |
|
I merged this in master in networked-aframe fork https://github.com/networked-aframe/janus-plugin-sfu |
Small changes after understanding how Rust works :)
I'm using these changes in production.