4 ms·
The example at the end of that section: pub fn encrypted(&self) -> Vec<u8> { encrypt(self.payload.as_ref().unwrap_or(&vec![])) } N
by weavie 3y ago
The example at the end of that section:
pub fn encrypted(&self) -> Vec<u8> {
encrypt(self.payload.as_ref().unwrap_or(&vec![]))
}
Needlessly allocates a `Vec` if `self.payload` is `Some`. Really you should do:
encrypt(&self.payload.as_ref().unwrap_or_else(|| &vec![]))
But that isn't going to work because the `else` Vec will be dropped at the end of the closure.
In this situation (if the code was on a hot path where every bit of performance mattered) I would probably do something like:
match self.payload.as_ref() {
Some(payload) => encrypt(payload),
None => encrypt(&vec![])
}
There is a slight bit of duplicated code. I'd be curious if anyone could come up with a better way.
- tveita 3y agoFrom https://doc.rust-lang.org/std/vec/struct.Vec.html https://doc.rust-lang.org/std/vec/struct.Vec.html if you construct a Vec with capacity 0 via Vec::new, vec![], Vec::with_capacity(0), or by calling shrink_to_fit on an empty Vec, it will not allocate memory. So an empty vec![] is just a struct on the stack; very cheap to make, and easy for the compiler to optimize out if it can see that it's not used in some paths.
- deleted 3y ago[deleted]
- laeri 3y agoCan you also use `unwrap_or_default` in this example?
- Omin 3y agoNo, because it requires an &Vec<_> and that doesn't implement Default and for good reason. Just ask yourself, where would the default empty vec live so that you could create a reference to it. When using unwrap_or(&vec![]), it lives in the enclosing stack. Without the reference, you could use unwrap_or_default().
- laeri 3y agoThanks for your reply that cleared it up. Was a bit confused why the `unwrap_or(&vec![])` worked but as you said it lives in the enclosing stack.
- jpochyla 3y agoThis is... not how I would really write it. If `encrypt` takes a `&[u8]`, as it should, you don't need a `&vec![]` at all, and can do: self.payload.as_deref().unwrap_or_default() ...which I think is the best here.
- School-Cotton 3y ago> Needlessly allocates a `Vec` if `self.payload` is `Some`. Empty vecs don’t require a heap allocation, so your original code is actually fine. In release mode it should compile to exactly the same instructions as your last example.
- HankB99 3y agoI lack Rust experience to comment on your suggestion but it does make me wonder if the author has provided a means for feedback in order to make this "More Effective Rust" (No pun intended if you're familiar with Meyers' followup book.)
- weavie 3y agoThe other commenters to this are correct, `Vec::new()` doesn't actually allocate so the code in the book is fine. I had a brain fart. If that function was taking a reference to an object that was fairly expensive to create a default value for then my conundrum would still exist.
- masklinn 3y ago> Needlessly allocates a `Vec` if `self.payload` is `Some`. Vec is documented not to allocate if empty (unless created with a set capacity). So all the code does is store 3 u64 on the stack.