Skip to content

Commit

Permalink
feat(rust): avoiding memory fragmentation by reducing allocations
Browse files Browse the repository at this point in the history
  • Loading branch information
davide-baldo committed Nov 14, 2024
1 parent a1c47c1 commit 05fbf4b
Show file tree
Hide file tree
Showing 73 changed files with 441 additions and 362 deletions.
2 changes: 1 addition & 1 deletion examples/rust/get_started/examples/bob.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}

Expand Down
2 changes: 1 addition & 1 deletion examples/rust/get_started/src/echoer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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" {
Expand Down
11 changes: 7 additions & 4 deletions implementations/rust/ockam/ockam/src/remote/worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)?,
Expand All @@ -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(),
Expand Down
6 changes: 3 additions & 3 deletions implementations/rust/ockam/ockam_api/src/address.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"))),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()?;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()?;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()?;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()?;
Expand Down
21 changes: 11 additions & 10 deletions implementations/rust/ockam/ockam_api/src/cli_state/cli_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")))
}
}
Expand Down Expand Up @@ -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"),
)?)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -441,9 +441,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,
Expand Down Expand Up @@ -529,11 +527,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()
Expand All @@ -558,14 +556,12 @@ impl EnrollmentTicket {
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
Expand Down
32 changes: 16 additions & 16 deletions implementations/rust/ockam/ockam_api/src/cli_state/nodes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -372,11 +372,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
Expand Down Expand Up @@ -419,11 +415,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())
}
}
Expand Down Expand Up @@ -622,11 +620,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())?)
}

Expand Down Expand Up @@ -659,7 +659,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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()))
Expand Down
26 changes: 15 additions & 11 deletions implementations/rust/ockam/ockam_api/src/cli_state/trust.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
5 changes: 3 additions & 2 deletions implementations/rust/ockam/ockam_api/src/echoer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ impl Worker for InfluxDBTokenLessorWorker {
SecureChannelLocalInfo::find_info(msg.local_message())?.their_identifier(),
);

let return_route = msg.return_route();
let return_route = msg.return_route().clone();
let body = msg.into_body()?;
let mut dec = Decoder::new(&body);
let req: RequestHeader = match dec.decode() {
Expand Down
Loading

0 comments on commit 05fbf4b

Please sign in to comment.