4 ms·

Posted in a sibling comment, but I repeat it because I think it's a very important point. > I cannot say much more about Rust syntax since I am even more of a

Posted in a sibling comment, but I repeat it because I think it's a very important point. > I cannot say much more about Rust syntax since I am even more of a Rust newbie than the author - perhaps someone can have some further comments on the quality of the resultant Rust code. The author is using an improper data type - a vector of borrowed strings (`Vec<&String>`), which forces the dev to use lifetimes. The owned version, `Vec<String>` doesn't suffer from this problem. More in general, this is a lack of experience problem - designing ownership is difficult at the beginning. And there's no problem with it! But don't publish a (micro)benchmark, please :) In particular, the author's conclusion on the necessity of lifetimes: > This means that, yes, writing efficient code in Rust requires a lot of forethought. In this case, our tiny modification to use a mutable Vec actually requires us to only deal with strings whose life times match or exceed that of our Vec. assigns a complexity to Rust that in this case doesn't exist. Rust is complex, but this is not one of those cases.

brabel 5y ago
You keep repeating your wrong argument, so please allow me to correct you. If you knew any Rust, you would immediately see that the owner of the words is the HashMap, which represents the dictionary of valid words to use. If the author had used a `Vec<String>` to represent the possible solutions, every single String would need to be either cloned or removed from the dictionary. A String cannot have two owners. This is basic Rust. If you disagree, please show me the magic code that can get around that.
- pizza234 5y ago
  Here's the diff in Pastebin: https://pastebin.com/wT8gXtBJ https://pastebin.com/wT8gXtBJ One just clones the single word, rather than cloning the whole words vector. Something that can also be improved in the code above (I've done the most minimal change) is to move the Vector instantiation outside the for loop, since print_translations() guarantees that the vector has the same content before/after invocation.
  - brabel 5y ago
    You cloned every word, which is much worse than cloning the vector (which only clones a couple of pointers). Here's the performance of the original code: ===> ./phone_encoder Proc,Memory(bytes),Time(ms) ./phone_encoder,24584192,198 Proc,Memory(bytes),Time(ms) ./phone_encoder,24584192,461 Proc,Memory(bytes),Time(ms) ./phone_encoder,24612864,1735 Proc,Memory(bytes),Time(ms) ./phone_encoder,24612864,3207 After your change: Proc,Memory(bytes),Time(ms) ./phone_encoder,24608768,205 Proc,Memory(bytes),Time(ms) ./phone_encoder,24592384,490 Proc,Memory(bytes),Time(ms) ./phone_encoder,24600576,1745 Proc,Memory(bytes),Time(ms) ./phone_encoder,24608768,3388 It's actually surprising that your change didn't make it much worse, but compared with the version where the lifetimes are used to avoid cloning anything at all, both of these are extremely slow.

brabel 5y ago

You keep repeating your wrong argument, so please allow me to correct you. If you knew any Rust, you would immediately see that the owner of the words is the HashMap, which represents the dictionary of valid words to use. If the author had used a `Vec<String>` to represent the possible solutions, every single String would need to be either cloned or removed from the dictionary. A String cannot have two owners. This is basic Rust. If you disagree, please show me the magic code that can get around that.

pizza234 5y ago
Here's the diff in Pastebin: https://pastebin.com/wT8gXtBJ https://pastebin.com/wT8gXtBJ One just clones the single word, rather than cloning the whole words vector. Something that can also be improved in the code above (I've done the most minimal change) is to move the Vector instantiation outside the for loop, since print_translations() guarantees that the vector has the same content before/after invocation.
- brabel 5y ago
  You cloned every word, which is much worse than cloning the vector (which only clones a couple of pointers). Here's the performance of the original code: ===> ./phone_encoder Proc,Memory(bytes),Time(ms) ./phone_encoder,24584192,198 Proc,Memory(bytes),Time(ms) ./phone_encoder,24584192,461 Proc,Memory(bytes),Time(ms) ./phone_encoder,24612864,1735 Proc,Memory(bytes),Time(ms) ./phone_encoder,24612864,3207 After your change: Proc,Memory(bytes),Time(ms) ./phone_encoder,24608768,205 Proc,Memory(bytes),Time(ms) ./phone_encoder,24592384,490 Proc,Memory(bytes),Time(ms) ./phone_encoder,24600576,1745 Proc,Memory(bytes),Time(ms) ./phone_encoder,24608768,3388 It's actually surprising that your change didn't make it much worse, but compared with the version where the lifetimes are used to avoid cloning anything at all, both of these are extremely slow.