Avoid 64-bit arithmetic in LEB encoding #345
Conversation
| let mut low = x as u32; | ||
| let mut high = (x >> 32) as u32; | ||
| let mut high = ((x >> 16) >> 16) as u32; |
There was a problem hiding this comment.
why the double-shift instead of going for >>32 right away?
There was a problem hiding this comment.
Oh, right, should've added a comment for that. Pushed a commit that explains this.
| @@ -58,8 +61,9 @@ pub(crate) fn leb64(x: u64, buf: &mut [u8; 10]) -> usize { | |||
| } | |||
| } | |||
|
|
|||
| pub fn zigzag_encode(v: i64) -> u64 { | |||
| ((v << 1) ^ (v >> 63)) as u64 | |||
| pub fn zigzag_encode(v: isize) -> usize { | |||
There was a problem hiding this comment.
nit: a 1 sentence doc on what's happening here for those could be helpful who are not familiar with zigzag encoding (not blocking though)
src/leb.rs
Outdated
| let i = leb64((1 << 32) - 1, &mut buf); | ||
| assert_eq!(buf[..i], [0xff, 0xff, 0xff, 0xff, 0xf]); | ||
| buf.iter_mut().for_each(|b| *b = 0x55); | ||
| if cfg!(target_pointer_width = "64") { |
There was a problem hiding this comment.
another nitpick: it's not very intuitive to figure out what's being tested here... Personally I'd put each check in a dedicated test case anyway (this way they're independent and their purpose is documented on the fly) but in this constellation comments would be helpful and time-saving
|
bors r=Lotterleben |
|
Build succeeded: |
Fixes #75
This uses
usizeinstead ofu64inleb64, which is now possible since timestamps aren't hard-coded to LEB64-encoded-u64anymore.Before:
After:
So, ~200 bytes of code saved.