3
\$\begingroup\$

I need to convert f32 arrays with a fix length to base64 representation and back.

My current code looks like this. It works, but it feels way too complicated.

How can I improve it?

main.rs:

extern crate base64;

fn read_float_vec_from_byte_vec(v: Vec<u8>) -> Result<Vec<f32>, String> {
    match v.len() % 4 {
        0 => {
            use std::ptr;
            let p = v.as_ptr();
            let dest_len = v.len() / 4;
            let mut result = Vec::<f32>::with_capacity(dest_len);
            for i in 0..dest_len as isize {
                unsafe {
                    let f_ptr = p.offset(4 * i) as *const f32;
                    result.push(ptr::read::<f32>(f_ptr));
                }
            }
            Ok(result)
        }
        _ => Err("Number of bytes not divisible by 4.".to_string())
    }
}

fn float_vec_as_byte_vec(values: &Vec<f32>) -> Vec<u8> {
    let dest_len = values.len() * 4;
    let mut result = Vec::<u8>::with_capacity(dest_len);
    result.resize(dest_len, 0);
    for (i, x) in values.iter().enumerate() {
        let bytes: [u8; 4] = unsafe { std::mem::transmute(*x) };
        result[i*4..i*4+4].copy_from_slice(&bytes[..]);
    }
    result
}

fn show_base64_decode_error(e: base64::DecodeError) -> String {
    use std::error::Error;
    format!("{:?}", e.description())
}

fn base64_to_f32_16_array(base64_str: &str) -> Result<[f32; 16], String>
{
    base64::decode(base64_str
        ).map_err(show_base64_decode_error
        ).and_then(read_float_vec_from_byte_vec
        ).map(
        |float_vec| {
            let mut result: [f32; 16] = [0.0; 16];
            result.copy_from_slice(&float_vec[..]);
            result
        }
    )
}

fn f32_16_array_to_base64(vals: &[f32; 16]) -> String
{
    base64::encode(&float_vec_as_byte_vec(&vals.to_vec()))
}

#[cfg(test)]
mod tests {
    static VALUES: [f32; 16] = [-0.35, -0.97, 0.05, 0.49, 1.35, 1.08, -0.71, 0.27, -0.44, 0.22, -1.34, -1.17, 0.68, 0.02, -0.52, 0.83];
    static BASE64_STR: &str = "MzOzvuxReL/NzEw9SOH6Ps3MrD9xPYo/j8I1v3E9ij6uR+G+rkdhPh+Fq7+PwpW/exQuPwrXozy4HgW/4XpUPw==";
    #[test]
    fn base64_to_f32_16_array_test() {
        assert_eq!(super::base64_to_f32_16_array(&BASE64_STR), Ok(VALUES));
        assert_eq!(
            super::base64_to_f32_16_array("foo"),
            Err("Number of bytes not divisible by 4.".to_string()));
    }
    #[test]
    fn f32_16_array_to_base64_test() {
        assert_eq!(super::f32_16_array_to_base64(&VALUES), BASE64_STR);
    }
}

cargo.toml (for completeness):

[package]
name = "base64_f32_test"
version = "0.1.0"

[dependencies]
base64 = "0.9.0"
\$\endgroup\$
  • \$\begingroup\$ Not enough content for a full answer, but can't the first match just be an if? \$\endgroup\$ – Challenger5 Feb 7 '18 at 15:58
  • \$\begingroup\$ @Challenger5 Yes, I guess it can. :) \$\endgroup\$ – Tobias Hermann Feb 8 '18 at 6:28
2
\$\begingroup\$

You should use iterators: when you use a line like this for i in 0..dest_len as isize, the compiler cannot optimize when going through the slice.

A better example:

fn read_float_vec_from_byte_vec(v: Vec<u8>) -> Result<Vec<f32>, &'static str> {
    #[inline(always)]
    fn float_from_u8_slice(s: &[u8]) -> f32 {
        let array = [s[0], s[1], s[2], s[3]];
        unsafe { std::mem::transmute(array) }
    }

    match v.len() % 4 {
        0 => Ok(v.chunks(4).map(float_from_u8_slice).collect()),
        _ => Err("Number not divisible by 4"),
    }
}

With the iteration over v.chunks(4).map(float_from_u8_slice).collect(), the code is smaller and more readable. Generally speaking, iterators are always better than indexing because indexing operation performs bounds checks.

Unfortunately, you cannot do the same with float_vec_as_byte_vec because (for now) arrays in Rust are not a first-class citizen. Due to the fact that genericity on integers does not exist, you cannot consume an array into an iterator. If it did, we could write something like:

// This code does not work currently
fn float_vec_as_byte_vec(s: &[f32]) -> Vec<u8> {
    s.iter().flat_map(|&f| unsafe { transmute::<f32, [u8; 4]>(f)).collect()
}

In this code, I accept the type &[f32], instead of &Vec<f32>. Slices are a view of memory, and that is what we want. &Vec<f32> does not really make sense, semantically.

In the first function I rewrote, I used &'static str and not String because we do not need an allocated string since it is literal.

\$\endgroup\$
  • \$\begingroup\$ This looks better, thanks a lot. Just as a remark: Simply replacing read_float_vec_from_byte_vec with your version like this results in an error in line 36 (expected reference, found struct std::string::String). However, changing your function signature to fn read_float_vec_from_byte_vec(v: Vec<u8>) -> Result<Vec<f32>, String> { and adding to_string in the right place makes it work. :) \$\endgroup\$ – Tobias Hermann Feb 13 '18 at 10:18
  • \$\begingroup\$ Oh, and clippy does not like the call to transmute and gives the following warning: transmute from [u8; 4] to a less-aligned type (f32) \$\endgroup\$ – Tobias Hermann Feb 13 '18 at 10:31
  • \$\begingroup\$ @TobiasHermann Sure, that's because you need a String after that. This modification was kind of generic and I did not took into account your particular usage after that. \$\endgroup\$ – French Boiethios Feb 13 '18 at 10:41
  • \$\begingroup\$ @TobiasHermann the lint is buggy and wrong in this case (see github.com/rust-lang-nursery/rust-clippy/pull/…) \$\endgroup\$ – Stefan Feb 13 '18 at 11:05
  • \$\begingroup\$ "the compiler cannot optimize" - why not? It certainly may be able to see all indexes in range looped over are valid (I'm not saying there aren't other good reasons to use iterators). \$\endgroup\$ – Stefan Feb 13 '18 at 11:08

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.