Skip to content

Optimize integer decoding a bit #109867

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

Optimize integer decoding a bit #109867

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
69f33e5
Optimize integer decoding a bit
saethlin Apr 2, 2023
d8877cd
Peel the first two iterations, outline the rest
saethlin Apr 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 26 additions & 19 deletions compiler/rustc_serialize/src/leb128.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,28 +51,35 @@ macro_rules! impl_read_unsigned_leb128 {
($fn_name:ident, $int_ty:ty) => {
#[inline]
pub fn $fn_name(slice: &[u8], position: &mut usize) -> $int_ty {
// The first iteration of this loop is unpeeled. This is a
// performance win because this code is hot and integer values less
// than 128 are very common, typically occurring 50-80% or more of
// the time, even for u64 and u128.
let byte = slice[*position];
*position += 1;
if (byte & 0x80) == 0 {
return byte as $int_ty;
}
let mut result = (byte & 0x7F) as $int_ty;
let mut shift = 7;
loop {
let byte = slice[*position];
*position += 1;
#[inline]
fn inner(slice: &[u8], position: &mut usize) -> Option<$int_ty> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the inner? Couldn't you just use indexing syntax for the panicking gets? Do you want to minimize the number of panic paths to minimize code size?

Copy link
Member Author

@saethlin saethlin Apr 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. The code that I'm porting this from is a must-never-panic parser, so these failures are hoisted to an outer layer with ?. Then in picking through the existing codegen in rustc, I noticed that we have two separate calls to core::panicking::panic_bounds_check at the bottom of this function, because the use of panicking indexing requires rustc to preserve the line numbers in the panic message. But nobody cares exactly which line panicked, only that leb128 decoding failed (not even that actually, but our hands are tied by the Decodable interface)

I'm pushing a commit to establish if this matters.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, looks like it matters 🙃

let mut pos = *position;
// The first iteration of this loop is unpeeled. This is a
// performance win because this code is hot and integer values less
// than 128 are very common, typically occurring 50-80% or more of
// the time, even for u64 and u128.
let byte = *slice.get(pos)?;
pos += 1;
if (byte & 0x80) == 0 {
result |= (byte as $int_ty) << shift;
return result;
} else {
result |= ((byte & 0x7F) as $int_ty) << shift;
*position = pos;
return Some(byte as $int_ty);
}
let mut result = (byte & 0x7F) as $int_ty;
let mut shift = 7;
loop {
let byte = *slice.get(pos)?;
pos += 1;
if (byte & 0x80) == 0 {
result |= (byte as $int_ty) << shift;
*position = pos;
return Some(result);
} else {
result |= ((byte & 0x7F) as $int_ty) << shift;
}
shift += 7;
}
shift += 7;
}
inner(slice, position).unwrap()
}
};
}
Expand Down