Replace usage of push_all and resize with extend #25
Conversation
This allows removal of the collections feature gate
| //let mut v = Vec::with_capacity(self.size); | ||
| //self.v.clear(); | ||
| self.v.resize(self.size, 0); | ||
| self.v.extend(repeat(0).take(self.size - len)); |
There was a problem hiding this comment.
I'm making the assumption here that resize only every grows the vector. I might be mistaken there - I only glanced at the surroundings.
If I'm mistaken, I'll add the "grow or shrink?"-dance.
There was a problem hiding this comment.
yes, it can only grow the vector
|
I think it can work, but it may be a good idea to wait a bit for extend to get faster, or push_all to be reimplemented, what do you think? |
|
Hm, I tried finding any substance to that claim of extend/iter being slower, and I didn't find any. Considering that push_all() also does all the clone work hidden away and will probably be a shim over extend anyways, I think this is a good way to go. I ran your mp4 parser on bigbuck.mp4 and the beta version with this implementation is in the same range, even a bit faster in debug mode (??). Take it with a grain of salt, as I hacked around nom_benchmark a little, but given that In the end, I intend to use nom with a piece of software I'm developing on top of beta, so my agenda is clear there ;). |
Replace usage of push_all and resize with extend
|
I think now it is a better idea to support beta. The performance hit will only affect producers and consumers, and there are other ways to optimize them. Thank you for fixing this part of the code :) |
This allows removal of the collections feature gate.
Builds and tests on beta-1.0.0.
Please note that extend is slower then
push_allcurrently: rust-lang/rust#24028 (comment), but the error message says that extend will probably be the future way to go.