Skip to content
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.

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

Add leb128 module and stop using io::Read #216

Merged
merged 2 commits into from
Jul 6, 2017
Merged

Conversation

philipc
Copy link
Collaborator

@philipc philipc commented Jul 6, 2017

This recovers some of the performance we lost during the Reader conversion (numbers in commit).

 name                                                       before ns/iter  after ns/iter  diff ns/iter   diff %  speedup
 bench_executing_line_number_programs                       495,431         431,543             -63,888  -12.90%   x 1.15
 bench_parsing_debug_abbrev                                 33,541          22,362              -11,179  -33.33%   x 1.50
 bench_parsing_debug_aranges                                33,222          33,106                 -116   -0.35%   x 1.00
 bench_parsing_debug_info                                   2,112,157       1,419,135          -693,022  -32.81%   x 1.49
 bench_parsing_debug_info_tree                              2,262,366       1,617,873          -644,493  -28.49%   x 1.40
 bench_parsing_debug_loc                                    317,102         324,836               7,734    2.44%   x 0.98
 bench_parsing_debug_pubnames                               77,572          77,421                 -151   -0.19%   x 1.00
 bench_parsing_debug_pubtypes                               38,339          41,099                2,760    7.20%   x 0.93
 bench_parsing_debug_ranges                                 180,564         181,408                 844    0.47%   x 1.00
 bench_parsing_line_number_program_opcodes                  577,134         528,243             -48,891   -8.47%   x 1.09
 cfi::eval_longest_fde_instructions_new_ctx_everytime       669             645                     -24   -3.59%   x 1.04
 cfi::eval_longest_fde_instructions_same_ctx                382             334                     -48  -12.57%   x 1.14
 cfi::iterate_entries_and_do_not_parse_any_fde              108,375         109,769               1,394    1.29%   x 0.99
 cfi::iterate_entries_and_parse_every_fde                   799,175         653,689            -145,486  -18.20%   x 1.22
 cfi::iterate_entries_and_parse_every_fde_and_instructions  1,347,116       1,087,315          -259,801  -19.29%   x 1.24
 cfi::iterate_entries_evaluate_every_fde                    1,773,783       1,519,466          -254,317  -14.34%   x 1.17
 cfi::parse_longest_fde_instructions                        235             187                     -48  -20.43%   x 1.26
@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 93.665% when pulling 0910ca3 on philipc:leb128 into 1b0ce73 on gimli-rs:master.

Copy link
Member

@fitzgen fitzgen left a comment

Choose a reason for hiding this comment

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

Can't argue with the numbers!

Thanks @philipc :)

src/leb128.rs Outdated

#[doc(hidden)]
#[inline]
pub fn low_bits_of_u64(val: u64) -> u8 {
Copy link
Member

Choose a reason for hiding this comment

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

All of these doc(hidden) things can be pub(crate) now, instead. Obviously doesn't need to block anything from landing.

src/leb128.rs Outdated
#[doc(hidden)]
pub const SIGN_BIT: u8 = 1 << 6;
const CONTINUATION_BIT: u8 = 1 << 7;
const SIGN_BIT: u8 = 1 << 6;
Copy link
Member

Choose a reason for hiding this comment

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

Ok, there we go :)

}
Err(_) => Err(Error::BadSignedLeb128),
}
leb128::read::signed(self)
Copy link
Member

Choose a reason for hiding this comment

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

Might make sense for these functions to either go away or be #[inline] too now (although I suspect rustc/llvm is doing the expected inlining anyways).

@fitzgen fitzgen merged commit fdd6feb into gimli-rs:master Jul 6, 2017
@philipc philipc deleted the leb128 branch July 9, 2017 03:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants