marshallpierce / rust-base64

base64, in rust
Apache License 2.0
606 stars 113 forks source link

base64::decode_config excessive memory allocation #195

Closed dkomanov closed 1 year ago

dkomanov commented 1 year ago

According to the source code:

pub fn decode_config<T: AsRef<[u8]>>(input: T, config: Config) -> Result<Vec<u8>, DecodeError> {
    let mut buffer = Vec::<u8>::with_capacity(input.as_ref().len() * 4 / 3);

    decode_config_buf(input, config, &mut buffer).map(|_| buffer)
}

decode_config method allocates excessive amount of memory: len * 4 / 3 instead of (len + 3) * 3 / 4.

dkomanov commented 1 year ago

Oops. I guess I missed #179.

The fix that is done is not the most performant... I did some benchmarking with different input sizes:

method                                input  output  avg time
base64::decode_config (excessive)        16      12        86
base64::decode_config (excessive)        68      51       133
base64::decode_config (excessive)       136     102       183
base64::decode_config (excessive)       668     501       579
base64::decode_config (excessive)      1336    1002      1134
base64::decode_config_buf (no alloc)     16      12        93
base64::decode_config_buf (no alloc)     68      51       136
base64::decode_config_buf (no alloc)    136     102       191
base64::decode_config_buf (no alloc)    668     501       588
base64::decode_config_buf (no alloc)   1336    1002      1125
base64::decode_config_slice (unsafe)     16      12        79
base64::decode_config_slice (unsafe)     68      51       119
base64::decode_config_slice (unsafe)    136     102       170
base64::decode_config_slice (unsafe)    668     501       560
base64::decode_config_slice (unsafe)   1336    1002      1061
base64::decode_config_slice (safe)       16      12        93
base64::decode_config_slice (safe)       68      51       133
base64::decode_config_slice (safe)      136     102       183
base64::decode_config_slice (safe)      668     501       588
base64::decode_config_slice (safe)     1336    1002      1101

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
}
marshallpierce commented 1 year ago

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.

dkomanov commented 1 year ago

If you interested, there are few interesting comments on reddit.

marshallpierce commented 1 year ago

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.

marshallpierce commented 1 year ago

Addressed via https://github.com/marshallpierce/rust-base64/compare/master...mp/decode-vec-size-backport in 0.13.1. I'll merge it by hand.