Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

validate_prefix_length can avoid creating new Strings #943

Closed
rnbguy opened this issue Nov 4, 2023 · 0 comments · Fixed by #941
Closed

validate_prefix_length can avoid creating new Strings #943

rnbguy opened this issue Nov 4, 2023 · 0 comments · Fixed by #941
Assignees
Labels
O: optimization Objective: aims to optimize performance, allocations and computations
Milestone

Comments

@rnbguy
Copy link
Collaborator

rnbguy commented Nov 4, 2023

Bug Summary

validate_prefix_length unnecessarily creates a new String using u64::MIN and u64::MAX which can be avoided.

Details

The current implementation looks as follows.

pub fn validate_prefix_length(
prefix: &str,
min_id_length: u64,
max_id_length: u64,
) -> Result<(), Error> {
// Checks if the prefix forms a valid identifier length when constructed with `u64::MIN`
validate_identifier_length(
&format!("{prefix}-{}", u64::MIN),
min_id_length,
max_id_length,
)?;
// Checks if the prefix forms a valid identifier length when constructed with `u64::MAX`
validate_identifier_length(
&format!("{prefix}-{}", u64::MAX),
min_id_length,
max_id_length,
)?;
Ok(())
}

Instead of doing

validate_identifier_length( 
         &format!("{prefix}-{}", u64::MIN), 
         min_id_length, 
         max_id_length, 
     )

we can directly pass prefix with {min,max}_id_length with negative offset for format!("-{}", u64::MIN).

validate_identifier_length( 
         prefix,
         min_id_length - 2,
         max_id_length - 2,
     )

And, similar for u64::MAX with offset 21. But we need to be careful that these subtractions don't underflow.

Version

v0.47.0

@github-project-automation github-project-automation bot moved this to 📥 To Do in ibc-rs Nov 4, 2023
@Farhad-Shabani Farhad-Shabani added the O: optimization Objective: aims to optimize performance, allocations and computations label Nov 4, 2023
@github-project-automation github-project-automation bot moved this from 📥 To Do to ✅ Done in ibc-rs Nov 6, 2023
@Farhad-Shabani Farhad-Shabani added this to the v0.48.0 milestone Nov 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O: optimization Objective: aims to optimize performance, allocations and computations
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants