Tune flow of inline methods in symbol_with_update - #2671
Conversation
barrbrain
commented
Feb 15, 2021
barrbrain
commented
- Mostly avoid calling Vec::reserve() in CDFContextLogOps::push().
- Manually elide bounds checks in WriterBase::symbol() as they are not easily inferred by the compiler although statically known.
- Rewrite ec::rust::update_cdf() to be panic-free and hint to the compiler not to unroll beyond the maximum CDF length.
lu-zero
left a comment
lu-zero
left a comment
There was a problem hiding this comment.
Looks fine, if you could address the 2 nits on landing would be nice.
| @@ -524,9 +524,12 @@ where | |||
| #[inline(always)] | |||
| fn symbol(&mut self, s: u32, cdf: &[u16]) { | |||
There was a problem hiding this comment.
Add a TODO: rewrite once const generics are stable
There was a problem hiding this comment.
Isn't the scope of rewriting using const generics much broader than this function?
There was a problem hiding this comment.
yes, but this is one that would be good to not forget :)
| } | ||
| // Single loop (faster) | ||
| for (i, v) in cdf[..nsymbs - 1].iter_mut().enumerate() { | ||
| for (i, v) in cdf[..nsymbs - 1].iter_mut().enumerate().take(15) { |
There was a problem hiding this comment.
15 shouldn't be expressed using Self::CDF_LEN_MAX ?
There was a problem hiding this comment.
Self::CDF_LEN_MAX is defined for CDFContextLogOps rather than WriterBase. We could reference context::CDF_LEN_MAX after making it accessible.
There was a problem hiding this comment.
Probably it would be good or having ec::CDF_LEN_MAX defined.
* Mostly avoid calling Vec::reserve() in CDFContextLogOps::push(). * Manually elide bounds checks in WriterBase::symbol() as they are not easily inferred by the compiler although statically known. * Rewrite ec::rust::update_cdf() to be panic-free and hint to the compiler not to unroll beyond the maximum CDF length.
ca324e7 to
e9be6c9
Compare
3 participants
