feat(rust): avoiding memory fragmentation by reducing allocations

build-trust · Nov 14, 2024 · 503e98b · 503e98b
1 parent 3f01471
commit 503e98b
Show file tree

Hide file tree

Showing 73 changed files with 442 additions and 363 deletions.
diff --git a/examples/rust/get_started/examples/bob.rs b/examples/rust/get_started/examples/bob.rs
@@ -17,7 +17,7 @@ impl Worker for Echoer {
         println!("\n[✓] Address: {}, Received: {:?}", ctx.address(), msg);
 
         // Echo the message body back on its return_route.
-        ctx.send(msg.return_route(), msg.into_body()?).await
+        ctx.send(msg.return_route().clone(), msg.into_body()?).await
     }
 }
 

diff --git a/examples/rust/get_started/src/echoer.rs b/examples/rust/get_started/src/echoer.rs
@@ -11,6 +11,6 @@ impl Worker for Echoer {
         println!("Address: {}, Received: {:?}", ctx.address(), msg);
 
         // Echo the message body back on its return_route.
-        ctx.send(msg.return_route(), msg.into_body()?).await
+        ctx.send(msg.return_route().clone(), msg.into_body()?).await
     }
 }
diff --git a/implementations/rust/ockam/ockam/src/relay_service/relay.rs b/implementations/rust/ockam/ockam/src/relay_service/relay.rs
@@ -71,7 +71,7 @@ impl Worker for Relay {
         .await?;
 
         // Remove the last hop so that just route to the node itself is left
-        self.forward_route.modify().pop_back();
+        self.forward_route = self.forward_route.clone().modify().pop_back().into();
 
         Ok(())
     }

diff --git a/implementations/rust/ockam/ockam/src/relay_service/relay_service.rs b/implementations/rust/ockam/ockam/src/relay_service/relay_service.rs
@@ -72,7 +72,7 @@ impl Worker for RelayService {
         let secure_channel_local_info =
             SecureChannelLocalInfo::find_info(message.local_message()).ok();
 
-        let forward_route = message.return_route();
+        let forward_route = message.return_route().clone();
         let requested_relay_address = message.into_body()?;
 
         let requested_relay_name = if requested_relay_address == "register" {

diff --git a/implementations/rust/ockam/ockam/src/remote/worker.rs b/implementations/rust/ockam/ockam/src/remote/worker.rs
@@ -33,7 +33,6 @@ impl Worker for RemoteRelay {
         msg: Routed<Self::Message>,
     ) -> Result<()> {
         if msg.msg_addr() == self.addresses.main_remote {
-            let return_route = msg.return_route();
             let mut local_message = msg.into_local_message();
 
             // Remove my address from the onward_route
@@ -53,8 +52,12 @@ impl Worker for RemoteRelay {
                     }
 
                     if !self.completion_msg_sent {
-                        info!(registration_route = %self.registration_route, "RemoteRelay registered with route: {}", return_route);
-                        let address = match return_route.recipient()?.to_string().strip_prefix("0#")
+                        info!(registration_route = %self.registration_route, "RemoteRelay registered with route: {}", local_message.return_route);
+                        let address = match local_message
+                            .return_route
+                            .recipient()?
+                            .to_string()
+                            .strip_prefix("0#")
                         {
                             Some(addr) => addr.to_string(),
                             None => return Err(OckamError::InvalidResponseFromRelayService)?,
@@ -63,7 +66,7 @@ impl Worker for RemoteRelay {
                         ctx.send_from_address(
                             self.addresses.completion_callback.clone(),
                             RemoteRelayInfo::new(
-                                return_route,
+                                local_message.return_route,
                                 address,
                                 self.addresses.main_remote.clone(),
                                 self.flow_control_id.clone(),

diff --git a/implementations/rust/ockam/ockam_api/src/address.rs b/implementations/rust/ockam/ockam_api/src/address.rs
@@ -22,19 +22,19 @@ pub fn extract_address_value(input: &str) -> Result<String, ApiError> {
                 Node::CODE => {
                     addr = p
                         .cast::<Node>()
-                        .ok_or(ApiError::message("Failed to parse `node` protocol"))?
+                        .ok_or_else(|| ApiError::message("Failed to parse `node` protocol"))?
                         .to_string();
                 }
                 Service::CODE => {
                     addr = p
                         .cast::<Service>()
-                        .ok_or(ApiError::message("Failed to parse `service` protocol"))?
+                        .ok_or_else(|| ApiError::message("Failed to parse `service` protocol"))?
                         .to_string();
                 }
                 Project::CODE => {
                     addr = p
                         .cast::<Project>()
-                        .ok_or(ApiError::message("Failed to parse `project` protocol"))?
+                        .ok_or_else(|| ApiError::message("Failed to parse `project` protocol"))?
                         .to_string();
                 }
                 code => return Err(ApiError::message(format!("Protocol {code} not supported"))),

diff --git a/...ions/rust/ockam/ockam_api/src/authenticator/credential_issuer/credential_issuer_worker.rs b/...ions/rust/ockam/ockam_api/src/authenticator/credential_issuer/credential_issuer_worker.rs
@@ -56,13 +56,13 @@ impl Worker for CredentialIssuerWorker {
             Ok(secure_channel_info) => secure_channel_info,
             Err(_e) => {
                 let resp = Response::bad_request_no_request("secure channel required").to_vec()?;
-                c.send(m.return_route(), resp).await?;
+                c.send(m.return_route().clone(), resp).await?;
                 return Ok(());
             }
         };
 
         let from = Identifier::from(secure_channel_info.their_identifier());
-        let return_route = m.return_route();
+        let return_route = m.return_route().clone();
         let body = m.into_body()?;
         let mut dec = Decoder::new(&body);
         let req: RequestHeader = dec.decode()?;

diff --git a/implementations/rust/ockam/ockam_api/src/authenticator/direct/direct_authenticator_worker.rs b/implementations/rust/ockam/ockam_api/src/authenticator/direct/direct_authenticator_worker.rs
@@ -44,13 +44,13 @@ impl Worker for DirectAuthenticatorWorker {
             Ok(secure_channel_info) => secure_channel_info,
             Err(_e) => {
                 let resp = Response::bad_request_no_request("secure channel required").to_vec()?;
-                c.send(m.return_route(), resp).await?;
+                c.send(m.return_route().clone(), resp).await?;
                 return Ok(());
             }
         };
 
         let from = Identifier::from(secure_channel_info.their_identifier());
-        let return_route = m.return_route();
+        let return_route = m.return_route().clone();
         let body = m.into_body()?;
         let mut dec = Decoder::new(&body);
         let req: RequestHeader = dec.decode()?;

diff --git a/implementations/rust/ockam/ockam_api/src/authenticator/enrollment_tokens/acceptor_worker.rs b/implementations/rust/ockam/ockam_api/src/authenticator/enrollment_tokens/acceptor_worker.rs
@@ -35,13 +35,13 @@ impl Worker for EnrollmentTokenAcceptorWorker {
             Ok(secure_channel_info) => secure_channel_info,
             Err(_e) => {
                 let resp = Response::bad_request_no_request("secure channel required").to_vec()?;
-                c.send(m.return_route(), resp).await?;
+                c.send(m.return_route().clone(), resp).await?;
                 return Ok(());
             }
         };
 
         let from = Identifier::from(secure_channel_info.their_identifier());
-        let return_route = m.return_route();
+        let return_route = m.return_route().clone();
         let body = m.into_body()?;
         let mut dec = Decoder::new(&body);
         let req: RequestHeader = dec.decode()?;

diff --git a/implementations/rust/ockam/ockam_api/src/authenticator/enrollment_tokens/issuer_worker.rs b/implementations/rust/ockam/ockam_api/src/authenticator/enrollment_tokens/issuer_worker.rs
@@ -46,13 +46,13 @@ impl Worker for EnrollmentTokenIssuerWorker {
             Ok(secure_channel_info) => secure_channel_info,
             Err(_e) => {
                 let resp = Response::bad_request_no_request("secure channel required").to_vec()?;
-                c.send(m.return_route(), resp).await?;
+                c.send(m.return_route().clone(), resp).await?;
                 return Ok(());
             }
         };
 
         let from = Identifier::from(secure_channel_info.their_identifier());
-        let return_route = m.return_route();
+        let return_route = m.return_route().clone();
         let body = m.into_body()?;
         let mut dec = Decoder::new(&body);
         let req: RequestHeader = dec.decode()?;

diff --git a/implementations/rust/ockam/ockam_api/src/cli_state/cli_state.rs b/implementations/rust/ockam/ockam_api/src/cli_state/cli_state.rs
@@ -186,15 +186,16 @@ impl CliState {
     /// Returns the default backup directory for the CLI state.
     pub fn backup_default_dir() -> Result<PathBuf> {
         let dir = Self::default_dir()?;
-        let dir_name =
-            dir.file_name()
-                .and_then(|n| n.to_str())
-                .ok_or(CliStateError::InvalidOperation(
-                    "The $OCKAM_HOME directory does not have a valid name".to_string(),
-                ))?;
-        let parent = dir.parent().ok_or(CliStateError::InvalidOperation(
-            "The $OCKAM_HOME directory does not a valid parent directory".to_string(),
-        ))?;
+        let dir_name = dir.file_name().and_then(|n| n.to_str()).ok_or_else(|| {
+            CliStateError::InvalidOperation(
+                "The $OCKAM_HOME directory does not have a valid name".to_string(),
+            )
+        })?;
+        let parent = dir.parent().ok_or_else(|| {
+            CliStateError::InvalidOperation(
+                "The $OCKAM_HOME directory does not a valid parent directory".to_string(),
+            )
+        })?;
         Ok(parent.join(format!("{dir_name}.bak")))
     }
 }
@@ -303,7 +304,7 @@ impl CliState {
         Ok(get_env_with_default::<PathBuf>(
             "OCKAM_HOME",
             home::home_dir()
-                .ok_or(CliStateError::InvalidPath("$HOME".to_string()))?
+                .ok_or_else(|| CliStateError::InvalidPath("$HOME".to_string()))?
                 .join(".ockam"),
         )?)
     }

diff --git a/implementations/rust/ockam/ockam_api/src/cli_state/enrollments.rs b/implementations/rust/ockam/ockam_api/src/cli_state/enrollments.rs
@@ -438,9 +438,7 @@ impl ProjectRoute {
                 let project_id = dnsaddr
                     .split('.')
                     .next()
-                    .ok_or(CliStateError::InvalidData(
-                        "Invalid project route".to_string(),
-                    ))?
+                    .ok_or_else(|| CliStateError::InvalidData("Invalid project route".to_string()))?
                     .to_string();
                 Ok(Self {
                     id: project_id,
@@ -526,11 +524,11 @@ impl EnrollmentTicket {
         let project_change_history = project
             .project_change_history
             .as_ref()
-            .ok_or(ApiError::core("no project change history"))?;
+            .ok_or_else(|| ApiError::core("no project change history"))?;
         let authority_change_history = project
             .authority_identity
             .as_ref()
-            .ok_or(ApiError::core("no authority change history"))?;
+            .ok_or_else(|| ApiError::core("no authority change history"))?;
         let authority_route = project
             .authority_access_route
             .as_ref()
@@ -551,20 +549,18 @@ impl EnrollmentTicket {
         let project = ticket
             .project
             .as_ref()
-            .ok_or(ApiError::core("no project in legacy ticket"))?;
+            .ok_or_else(|| ApiError::core("no project in legacy ticket"))?;
         let project_id = project.id.clone();
         let project_name = project.name.clone();
         let project_change_history = project
             .project_change_history
             .as_ref()
-            .ok_or(ApiError::core("no project change history in legacy ticket"))?
+            .ok_or_else(|| ApiError::core("no project change history in legacy ticket"))?
             .clone();
         let authority_change_history = project
             .authority_identity
             .as_ref()
-            .ok_or(ApiError::core(
-                "no authority change history in legacy ticket",
-            ))?
+            .ok_or_else(|| ApiError::core("no authority change history in legacy ticket"))?
             .clone();
         let authority_route = project
             .authority_access_route

diff --git a/implementations/rust/ockam/ockam_api/src/cli_state/nodes.rs b/implementations/rust/ockam/ockam_api/src/cli_state/nodes.rs
@@ -369,11 +369,7 @@ impl CliState {
             .nodes_repository()
             .get_default_node()
             .await?
-            .ok_or(Error::new(
-                Origin::Api,
-                Kind::NotFound,
-                "There is no default node",
-            ))?)
+            .ok_or_else(|| Error::new(Origin::Api, Kind::NotFound, "There is no default node"))?)
     }
 
     /// Return the node information for the given node name, otherwise for the default node
@@ -416,11 +412,13 @@ impl CliState {
                 }
             })
             .max_by_key(|file| file.metadata().unwrap().modified().unwrap())
-            .ok_or(Error::new(
-                Origin::Api,
-                Kind::NotFound,
-                format!("there is no log file for the node {node_name}"),
-            ))?;
+            .ok_or_else(|| {
+                Error::new(
+                    Origin::Api,
+                    Kind::NotFound,
+                    format!("there is no log file for the node {node_name}"),
+                )
+            })?;
         Ok(current_log_file.path())
     }
 }
@@ -619,11 +617,13 @@ impl NodeInfo {
         Ok(self
             .tcp_listener_address
             .as_ref()
-            .ok_or(ockam::Error::new(
-                Origin::Api,
-                Kind::Internal,
-                "no transport has been set on the node".to_string(),
-            ))
+            .ok_or_else(|| {
+                ockam::Error::new(
+                    Origin::Api,
+                    Kind::Internal,
+                    "no transport has been set on the node".to_string(),
+                )
+            })
             .and_then(|t| t.multi_addr())?)
     }
 
@@ -656,7 +656,7 @@ impl NodeInfo {
     pub fn status(&self) -> NodeProcessStatus {
         if let Some(pid) = self.pid() {
             let mut sys = System::new();
-            sys.refresh_processes(ProcessesToUpdate::All, false);
+            sys.refresh_processes(ProcessesToUpdate::Some(&[Pid::from_u32(pid)]), false);
             if let Some(p) = sys.process(Pid::from_u32(pid)) {
                 // Under certain circumstances the process can be in a state where it's not running
                 // and we are unable to kill it. For example, `kill -9` a process created by

diff --git a/implementations/rust/ockam/ockam_api/src/cli_state/tcp_portals.rs b/implementations/rust/ockam/ockam_api/src/cli_state/tcp_portals.rs
@@ -33,11 +33,13 @@ impl CliState {
             .tcp_portals_repository()
             .get_tcp_inlet(node_name, alias)
             .await?
-            .ok_or(ockam_core::Error::new(
-                Origin::Api,
-                Kind::NotFound,
-                format!("no tcp inlet found for node {node_name}, with alias {alias}"),
-            ))?)
+            .ok_or_else(|| {
+                ockam_core::Error::new(
+                    Origin::Api,
+                    Kind::NotFound,
+                    format!("no tcp inlet found for node {node_name}, with alias {alias}"),
+                )
+            })?)
     }
 
     /// Delete a TCP inlet

diff --git a/implementations/rust/ockam/ockam_api/src/cli_state/test_support.rs b/implementations/rust/ockam/ockam_api/src/cli_state/test_support.rs
@@ -30,7 +30,7 @@ impl CliState {
     /// Return a random root directory
     pub fn test_dir() -> Result<PathBuf> {
         Ok(home::home_dir()
-            .ok_or(CliStateError::InvalidPath("$HOME".to_string()))?
+            .ok_or_else(|| CliStateError::InvalidPath("$HOME".to_string()))?
             .join(".ockam")
             .join(".tests")
             .join(random_name()))

diff --git a/implementations/rust/ockam/ockam_api/src/cli_state/trust.rs b/implementations/rust/ockam/ockam_api/src/cli_state/trust.rs
@@ -47,11 +47,13 @@ impl CliState {
             };
 
             let authority_route =
-                multiaddr_to_transport_route(authority_multiaddr).ok_or(Error::new(
-                    Origin::Api,
-                    Kind::NotFound,
-                    format!("Invalid authority route: {}", &authority_multiaddr),
-                ))?;
+                multiaddr_to_transport_route(authority_multiaddr).ok_or_else(|| {
+                    Error::new(
+                        Origin::Api,
+                        Kind::NotFound,
+                        format!("Invalid authority route: {}", &authority_multiaddr),
+                    )
+                })?;
             let info = RemoteCredentialRetrieverInfo::create_for_project_member(
                 authority_identifier.clone(),
                 authority_route,
@@ -112,14 +114,16 @@ impl CliState {
     ) -> Result<NodeManagerTrustOptions> {
         let authority_identifier = project
             .authority_identifier()
-            .ok_or(ApiError::core("no authority identifier"))?;
+            .ok_or_else(|| ApiError::core("no authority identifier"))?;
         let authority_multiaddr = project.authority_multiaddr()?;
         let authority_route =
-            multiaddr_to_transport_route(authority_multiaddr).ok_or(Error::new(
-                Origin::Api,
-                Kind::NotFound,
-                format!("Invalid authority route: {}", &authority_multiaddr),
-            ))?;
+            multiaddr_to_transport_route(authority_multiaddr).ok_or_else(|| {
+                Error::new(
+                    Origin::Api,
+                    Kind::NotFound,
+                    format!("Invalid authority route: {}", &authority_multiaddr),
+                )
+            })?;
 
         let project_id = project.project_id().to_string();
         let project_member_retriever = NodeManagerCredentialRetrieverOptions::Remote {

diff --git a/implementations/rust/ockam/ockam_api/src/cloud/share/create.rs b/implementations/rust/ockam/ockam_api/src/cloud/share/create.rs
@@ -62,11 +62,11 @@ impl CreateServiceInvitation {
             recipient_email: recipient_email.clone(),
             project_identity: project
                 .project_identifier()
-                .ok_or(ApiError::core("no project identifier"))?,
+                .ok_or_else(|| ApiError::core("no project identifier"))?,
             project_route: project.project_multiaddr()?.to_string(),
             project_authority_identity: project
                 .authority_identifier()
-                .ok_or(ApiError::core("no authority identifier"))?,
+                .ok_or_else(|| ApiError::core("no authority identifier"))?,
             project_authority_route: project_authority_route.to_string(),
             shared_node_identity: node_identifier,
             shared_node_route: service_route.as_ref().to_string(),

diff --git a/implementations/rust/ockam/ockam_api/src/echoer.rs b/implementations/rust/ockam/ockam_api/src/echoer.rs
@@ -11,8 +11,9 @@ impl Worker for Echoer {
 
     #[instrument(skip_all, name = "Echoer::handle_message")]
     async fn handle_message(&mut self, ctx: &mut Context, msg: Routed<Any>) -> Result<()> {
-        log::debug!(src = %msg.src_addr(), from = %msg.sender()?, to = %msg.return_route().step()?, "echoing back");
-        ctx.send(msg.return_route(), NeutralMessage::from(msg.into_payload()))
+        log::debug!(src = %msg.src_addr(), from = %msg.sender()?, to = %msg.return_route().next()?, "echoing back");
+        let msg = msg.into_local_message();
+        ctx.send(msg.return_route, NeutralMessage::from(msg.payload))
             .await
     }
 }