Skip to content

Commit b343c5f

Browse files
w-utterw-utter
authored
avoid unnecessary locator list cloning (#387)
1 parent fae8b56 commit b343c5f

File tree

4 files changed

+53
-45
lines changed

4 files changed

+53
-45
lines changed

‎.DS_Store‎

8 KB
Binary file not shown.

‎src/dds/with_key/datareader.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1227,8 +1227,8 @@ mod tests {
12271227
reader.matched_writer_add(
12281228
writer_guid,
12291229
EntityId::UNKNOWN,
1230-
mr_state.unicast_reply_locator_list.clone(),
1231-
mr_state.multicast_reply_locator_list.clone(),
1230+
mr_state.unicast_reply_locator_list.to_vec(),
1231+
mr_state.multicast_reply_locator_list.to_vec(),
12321232
&QosPolicies::qos_none(),
12331233
);
12341234

@@ -1392,8 +1392,8 @@ mod tests {
13921392
reader.matched_writer_add(
13931393
writer_guid,
13941394
EntityId::UNKNOWN,
1395-
mr_state.unicast_reply_locator_list.clone(),
1396-
mr_state.multicast_reply_locator_list.clone(),
1395+
mr_state.unicast_reply_locator_list.to_vec(),
1396+
mr_state.multicast_reply_locator_list.to_vec(),
13971397
&QosPolicies::qos_none(),
13981398
);
13991399

‎src/rtps/message_receiver.rs‎

Lines changed: 30 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -67,14 +67,14 @@ pub struct SecureWrapping {
6767

6868
// This is partial receiver state to be sent to a Reader or a Writer with a
6969
// Submessage
70-
#[derive(Debug, Clone)]
71-
pub struct MessageReceiverState {
70+
#[derive(Debug)]
71+
pub struct MessageReceiverState<'a> {
7272
pub source_guid_prefix: GuidPrefix,
73-
pub unicast_reply_locator_list: Vec<Locator>,
73+
pub unicast_reply_locator_list: &'a [Locator],
7474

7575
#[allow(dead_code)]
7676
// TODO: We do not use the multicast Locators from InfoReply for anything for now.
77-
pub multicast_reply_locator_list: Vec<Locator>,
77+
pub multicast_reply_locator_list: &'a [Locator],
7878

7979
pub source_timestamp: Option<Timestamp>,
8080

@@ -83,12 +83,12 @@ pub struct MessageReceiverState {
8383
pub secure_rtps_wrapped: Option<SecureWrapping>,
8484
}
8585

86-
impl Default for MessageReceiverState {
86+
impl Default for MessageReceiverState<'_> {
8787
fn default() -> Self {
8888
Self {
8989
source_guid_prefix: GuidPrefix::default(),
90-
unicast_reply_locator_list: Vec::default(),
91-
multicast_reply_locator_list: Vec::default(),
90+
unicast_reply_locator_list: &[],
91+
multicast_reply_locator_list: &[],
9292
source_timestamp: Some(Timestamp::INVALID),
9393
#[cfg(feature = "security")]
9494
secure_rtps_wrapped: None,
@@ -183,15 +183,24 @@ impl MessageReceiver {
183183
}
184184
}
185185

186-
fn clone_partial_message_receiver_state(&self) -> MessageReceiverState {
187-
MessageReceiverState {
186+
fn partial_message_receiver_state(
187+
&mut self,
188+
target_reader_entity_id: &EntityId,
189+
) -> (
190+
MessageReceiverState<'_>,
191+
Option<&mut Reader>,
192+
Option<&SecurityPluginsHandle>,
193+
) {
194+
let reader = self.available_readers.get_mut(target_reader_entity_id);
195+
let state = MessageReceiverState {
188196
source_guid_prefix: self.source_guid_prefix,
189-
unicast_reply_locator_list: self.unicast_reply_locator_list.clone(),
190-
multicast_reply_locator_list: self.multicast_reply_locator_list.clone(),
197+
unicast_reply_locator_list: &self.unicast_reply_locator_list,
198+
multicast_reply_locator_list: &self.multicast_reply_locator_list,
191199
source_timestamp: self.source_timestamp,
192200
#[cfg(feature = "security")]
193201
secure_rtps_wrapped: self.secure_rtps_wrapped.clone(),
194-
}
202+
};
203+
(state, reader, self.security_plugins.as_ref())
195204
}
196205

197206
pub fn add_reader(&mut self, new_reader: Reader) {
@@ -569,17 +578,16 @@ impl MessageReceiver {
569578
}
570579
}
571580

572-
let mr_state = self.clone_partial_message_receiver_state();
581+
let (mr_state, target_reader, security_plugins) =
582+
self.partial_message_receiver_state(&target_reader_entity_id);
573583
let writer_entity_id = submessage.sender_entity_id();
574584
let source_guid_prefix = mr_state.source_guid_prefix;
575585
let source_guid = &GUID {
576586
prefix: source_guid_prefix,
577587
entity_id: writer_entity_id,
578588
};
579589

580-
let security_plugins = self.security_plugins.clone();
581-
582-
let target_reader = if let Some(target_reader) = self.reader_mut(target_reader_entity_id) {
590+
let target_reader = if let Some(target_reader) = target_reader {
583591
target_reader
584592
} else {
585593
return error!("No reader matching the CryptoHandle found");
@@ -588,7 +596,7 @@ impl MessageReceiver {
588596
match submessage {
589597
WriterSubmessage::Data(data, data_flags) => {
590598
Self::decode_and_handle_data(
591-
security_plugins.as_ref(),
599+
security_plugins,
592600
source_guid,
593601
data,
594602
data_flags,
@@ -623,7 +631,7 @@ impl MessageReceiver {
623631

624632
WriterSubmessage::DataFrag(datafrag, flags) => {
625633
Self::decode_and_handle_datafrag(
626-
security_plugins.as_ref(),
634+
security_plugins,
627635
source_guid,
628636
datafrag.clone(),
629637
flags,
@@ -646,7 +654,7 @@ impl MessageReceiver {
646654
data: Data,
647655
data_flags: BitFlags<DATA_Flags, u8>,
648656
reader: &mut Reader,
649-
mr_state: &MessageReceiverState,
657+
mr_state: &MessageReceiverState<'_>,
650658
) {
651659
reader.handle_data_msg(data, data_flags, mr_state);
652660
}
@@ -658,7 +666,7 @@ impl MessageReceiver {
658666
data: Data,
659667
data_flags: BitFlags<DATA_Flags, u8>,
660668
reader: &mut Reader,
661-
mr_state: &MessageReceiverState,
669+
mr_state: &MessageReceiverState<'_>,
662670
) {
663671
let Data {
664672
inline_qos,
@@ -705,7 +713,7 @@ impl MessageReceiver {
705713
datafrag: DataFrag,
706714
datafrag_flags: BitFlags<DATAFRAG_Flags, u8>,
707715
reader: &mut Reader,
708-
mr_state: &MessageReceiverState,
716+
mr_state: &MessageReceiverState<'_>,
709717
) {
710718
let payload_buffer_length = datafrag.serialized_payload.len();
711719
if payload_buffer_length
@@ -737,7 +745,7 @@ impl MessageReceiver {
737745
datafrag: DataFrag,
738746
datafrag_flags: BitFlags<DATAFRAG_Flags, u8>,
739747
reader: &mut Reader,
740-
mr_state: &MessageReceiverState,
748+
mr_state: &MessageReceiverState<'_>,
741749
) {
742750
let DataFrag {
743751
inline_qos,

‎src/rtps/reader.rs‎

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -894,13 +894,6 @@ impl Reader {
894894
.with_mutable_writer_proxy(writer_guid, |this, writer_proxy| {
895895
// Note: This is worker closure. Use `this` instead of `self`.
896896

897-
// Decide where should we send a reply, i.e. ACKNACK
898-
let reply_locators = match mr_state.unicast_reply_locator_list.as_slice() {
899-
[] | [Locator::Invalid] => writer_proxy.unicast_locator_list.clone(),
900-
//TODO: What is writer_proxy has an empty list?
901-
others => others.to_vec(),
902-
};
903-
904897
if heartbeat.count <= writer_proxy.received_heartbeat_count {
905898
// This heartbeat was already seen an processed.
906899
return false;
@@ -1021,14 +1014,21 @@ impl Reader {
10211014
}
10221015
}
10231016

1017+
// Decide where should we send a reply, i.e. ACKNACK
1018+
let reply_locators = match mr_state.unicast_reply_locator_list {
1019+
[] | [Locator::Invalid] => &writer_proxy.unicast_locator_list,
1020+
//TODO: What is writer_proxy has an empty list?
1021+
others => others,
1022+
};
1023+
10241024
if !nackfrags.is_empty() {
10251025
this.send_nackfrags_to(
10261026
nackfrag_flags,
10271027
nackfrags,
10281028
InfoDestination {
10291029
guid_prefix: mr_state.source_guid_prefix,
10301030
},
1031-
&reply_locators,
1031+
reply_locators,
10321032
writer_guid,
10331033
);
10341034
}
@@ -1039,7 +1039,7 @@ impl Reader {
10391039
InfoDestination {
10401040
guid_prefix: mr_state.source_guid_prefix,
10411041
},
1042-
&reply_locators,
1042+
reply_locators,
10431043
writer_guid,
10441044
);
10451045

@@ -1518,8 +1518,8 @@ mod tests {
15181518
reader.matched_writer_add(
15191519
writer_guid,
15201520
EntityId::UNKNOWN,
1521-
mr_state.unicast_reply_locator_list.clone(),
1522-
mr_state.multicast_reply_locator_list.clone(),
1521+
mr_state.unicast_reply_locator_list.to_vec(),
1522+
mr_state.multicast_reply_locator_list.to_vec(),
15231523
&QosPolicies::qos_none(),
15241524
);
15251525

@@ -1605,8 +1605,8 @@ mod tests {
16051605
reader.matched_writer_add(
16061606
writer_guid,
16071607
EntityId::UNKNOWN,
1608-
mr_state.unicast_reply_locator_list.clone(),
1609-
mr_state.multicast_reply_locator_list.clone(),
1608+
mr_state.unicast_reply_locator_list.to_vec(),
1609+
mr_state.multicast_reply_locator_list.to_vec(),
16101610
&QosPolicies::qos_none(),
16111611
);
16121612

@@ -1709,8 +1709,8 @@ mod tests {
17091709
reader.matched_writer_add(
17101710
writer_guid,
17111711
EntityId::UNKNOWN,
1712-
mr_state.unicast_reply_locator_list.clone(),
1713-
mr_state.multicast_reply_locator_list.clone(),
1712+
mr_state.unicast_reply_locator_list.to_vec(),
1713+
mr_state.multicast_reply_locator_list.to_vec(),
17141714
&reliable_qos,
17151715
);
17161716

@@ -1818,8 +1818,8 @@ mod tests {
18181818
reader.matched_writer_add(
18191819
writer_guid,
18201820
EntityId::UNKNOWN,
1821-
mr_state.unicast_reply_locator_list.clone(),
1822-
mr_state.multicast_reply_locator_list.clone(),
1821+
mr_state.unicast_reply_locator_list.to_vec(),
1822+
mr_state.multicast_reply_locator_list.to_vec(),
18231823
&QosPolicies::qos_none(),
18241824
);
18251825

@@ -1956,8 +1956,8 @@ mod tests {
19561956
reader.matched_writer_add(
19571957
writer_guid,
19581958
EntityId::UNKNOWN,
1959-
mr_state.unicast_reply_locator_list.clone(),
1960-
mr_state.multicast_reply_locator_list.clone(),
1959+
mr_state.unicast_reply_locator_list.to_vec(),
1960+
mr_state.multicast_reply_locator_list.to_vec(),
19611961
&QosPolicies::qos_none(),
19621962
);
19631963

0 commit comments

Comments
 (0)