-
Notifications
You must be signed in to change notification settings - Fork 118
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
base64::decode_config excessive memory allocation #195
Comments
Oops. I guess I missed #179. The fix that is done is not the most performant... I did some benchmarking with different input sizes:
And the code: // Current solution
pub fn base64_decode_config_buf_no_prealloc(s: &String) -> Vec<u8> {
let mut buffer = Vec::<u8>::new();
base64::decode_config_buf(s, base64::STANDARD_NO_PAD, &mut buffer).map(|_| buffer).unwrap()
}
// Previous solution
pub fn base64_decode_config_buf_excessive_alloc(s: &String) -> Vec<u8> {
let mut buffer = Vec::<u8>::with_capacity(s.len() * 4 / 3);
base64::decode_config_buf(s, base64::STANDARD_NO_PAD, &mut buffer).map(|_| buffer).unwrap()
}
// The most performant solution with unsafe
pub fn base64_decode_config_slice(s: &String) -> Vec<u8> {
let mut buffer = Vec::<u8>::with_capacity((s.len() + 3) * 3 / 4);
unsafe {
let mut sl = std::slice::from_raw_parts_mut(buffer.as_mut_ptr(), buffer.capacity());
let size = base64::decode_config_slice(s, base64::STANDARD_NO_PAD, &mut sl).unwrap();
buffer.set_len(size);
}
buffer
}
// Safe performant solution.
pub fn base64_decode_config_slice_memset(s: &String) -> Vec<u8> {
let mut buffer = vec![0; (s.len() + 3) * 3 / 4];
let size = base64::decode_config_slice(s, base64::STANDARD_NO_PAD, &mut buffer).unwrap();
buffer.truncate(size);
buffer
} |
Thanks for the report. I had originally held off from doing the pre-sized vec because if the input is invalid then it's wasted, but that's probably a poor trade-off. I have a bunch of 1.0 work that's been waiting until I have enough time to give it the focus it deserves, but I'll try to get this improvement out promptly in a patch release without waiting for 1.0. |
If you interested, there are few interesting comments on reddit. |
Yep, there are faster crates out there now -- the major feature of the 1.0 work is allowing pluggable "engines" so users can use CPU-specific implementations under the hood while still enjoying all the rest of the functionality of this crate so hopefully we can all stop duplicating work. |
Addressed via master...mp/decode-vec-size-backport in 0.13.1. I'll merge it by hand. |
According to the source code:
decode_config
method allocates excessive amount of memory:len * 4 / 3
instead of(len + 3) * 3 / 4
.The text was updated successfully, but these errors were encountered: