Generalize function argument as optionally a reference

[eee]

Hi, this might be blaspheme using generics to represent different reference types, but I'm noticing code smell having to copy and past functions so that they support different reference types:

pub fn f32_bytes_val(fs: &Vec<f32>) -> Vec<u8> {
  fs.iter().flat_map(|&f| f.to_le_bytes()).collect()
} 

pub fn f32_bytes_ref(fs: Vec<&f32>) -> Vec<u8> {
  fs.iter().flat_map(|&f| f.to_le_bytes()).collect()
} 

Same code, different type arguments. Is there a way to supporting both argument types? I thought I could pass an iter() instead, but that would still need to support Iter<&f32> also...

[kornel]

You shouldn't use the &Vec<f32> type at all. That type doesn't make sense: you're asking for a type that can be grown, but then you're asking for it to be immutable, so you can't grow it. Take &[f32] instead.

If you want to be maximally flexible you could use:

fn whatever(fs: impl IntoIterator<Item=impl AsRef<f32>>)

That pattern is useful when you need to work with various string types, but if your code literally uses the &f32 type, then that's another mistake. The reference indirection makes access slower and prevents most optimizations, and (on 64-bit architectures) references are double the size of f32. You should dereference the &f32 type as soon as possible, and never allow Vec<&f32> to be created.

[skysch]

Define a trait and make your functions generic over it:

pub fn bytes_val<T>(fs: &[T]) -> Vec<u8> where T: ToLeBytes {
  fs.iter().flat_map(|f| f.to_le_bytes()).collect()
} 

pub trait ToLeBytes {
    fn to_le_bytes(self) -> [u8; 4];
}

impl ToLeBytes for f32 {
    fn to_le_bytes(self) -> [u8; 4] {
        f32::to_le_bytes(self)
    }
}

impl<T> ToLeBytes for &T where T: ToLeBytes {
    fn to_le_bytes(self) -> [u8; 4] {
        <Self as ToLeBytes>::to_le_bytes(*self)
    }
}

Alternatively, you can provide a single interface and let your users fix up their data before giving it to you. This is probably the correct solution most of the time.

[eee]

@kornel thanks, one quick follow up though: is it IntoIterator (couldn't find import for IntoIter...)?

...and then one long follow up too: As usual this community has surprised me with even more than I asked for!

The reason I have a Vec<&f32> is because I actually flatten a Vec<Vec<f32>> and that is the result, so to get Vec<f32> I think I'll need to clone after the flatten, which works, but I was avoiding this since I thought cloning was expensive. I guess you're saying while cloning is expensive, at a certain point looking up references for each element will hurt the CPU badly too?

So thank you, your solution is marvelously concise! But now that I've properly avoided reference indirection I no longer need it!

[kpreid]

@eee

The reason I have a Vec<&f32> is because I actually flatten a Vec<Vec<f32>> and that is the result, so to get Vec<f32> I think I'll need to clone after the flatten ,

References aren't a consequence of flattening, they're a consequence of iterating over a collection by-reference (or with .iter()). For example, this compiles and does not introduce references:

let vv: Vec<Vec<i32>> = vec![vec![1], vec![2]];
    
let v: Vec<i32> = vv.into_iter().flatten().collect();

@eee

I guess you're saying while cloning is expensive, at a certain point looking up references for each element will hurt the CPU badly too?

Yes. A reference is a sort of pointer, and a pointer is generally itself much like an integer. Copying an integer or a reference is very cheap — basically the cheapest thing a CPU can possibly do. But accessing the integer value the reference points to requires reading the memory at whatever address the reference contains — relatively, an extremely slow operation. Therefore, working with &i32 is entirely added expense over i32. The optimizer can often improve on naive reference-using code, so you shouldn't strive hard to avoid ever having a reference to an integer, but you should not choose to use references to integers (or any comparably small data type) with the goal of efficiency — it will be either neutral or worse.

The kinds of clones you should consider avoiding are cloning things like Strings or Vecs which may be arbitrarily large and require both making a heap allocation and copying large amounts of data.

[eee]

@kpreid you're right, but actually I forgot this Vec<Vec<f32>> is owned by a struct...

let flattened = data.b.into_iter().flatten().collect_vec() // move occurs since data.b has type Vec<Vec<f32>> that does not implement Copy trait

This caught me off guard too, but it seems I need to tell my struct to implement Copy and then it should work except it tells me in the struct definition Vec<Vec<f32>> does not implement Copy..

Some also have type Option<Vec<Vec<f32>>>, which I still use clone on since Option doesn't implement it (I don't think there's a cleaner way..?)

#[derive(ParialEq, Debug, Copy)]
pub struct Data {
  pub a: Option<Vec<Vec<f32>>>,
  pub b: Vec<Vec<f32>>
}

So that's why I'm stuck cloning...

[bluss]

@eee

data.b.into_iter().flatten().collect_vec()

I'll just zoom in on one particular thing here and give you an idea to think about (maybe there's something better big picture).

You can just as well use data.b.iter().flatten().copied().collect_vec() here - you get a Vec<f32> as a result. The thinking here is to: iterate by reference, dereference &f32 -> f32 and store in the new vec.

copied is just short for dereferencing each iterator element. Yes, f32 implements Copy.

[kpreid]

@eee

This caught me off guard too, but it seems I need to tell my struct to implement Copy and then it should work except it tells me in the struct definition Vec<Vec<f32>> does not implement Copy..

“Move occurs because type does not implement Copy” almost never means that you should implement Copy. It's just telling you why there is a move. When the compiler reports an error, the actual problem it is telling you about is on the first line of the message:

error[E0382]: use of moved value: `name_of_variable`

The other lines like move occurs because... are all context/hints, not the primary problem it wants to tell you about.

In this case, the solution is likely to iterate with copying the elements, as @bluss's answer showed. The .into_iter() version would be appropriate if you are done with the struct and the Vec and therefore don't mind destroying it (moving it out of your struct into the iterator).

[kornel]

Cost of cloning is proportional to the size/complexity of the item being cloned. A 4-byte integer is very small and simple, and therefore very cheap to clone. It's so cheap to clone, that it's cheaper to clone it than to store a reference to it.

There's .iter().cloned() and .iter().copied(). For item types like &f32 both of them are exactly the same method, but the copied one exists to make it clear in the source code that in this case cloning is cheap.

[Phlopsi]

@kornel

the copied one exists to make it clear in the source code that in this case cloning is cheap

The copied one exists to signal a trivial, (not cheap) clone. Calling Clone::clone may involve additional code to be run other than simply copying the bytes from one location to another, while using Copy semantics guarantees no additional code to be run than is necessary to copy the bytes from one location to another.

An example of a type that involves non-trivial cloning is Box<T>, which requires allocating memory before copying the underlying bytes. An example of a type, that can be copied, but is not cheap to copy is [u8; 16384]. At that size, passing around pointers is recommended where possible.