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

[Based on #22] Less inline directives, more bench determinism #24

Open
wants to merge 16 commits into
base: master
Choose a base branch
from

Conversation

HadrienG2
Copy link

@HadrienG2 HadrienG2 commented Sep 20, 2019

Following up on #21, this PR proposes to deal away with most of the manual inline annotations in Abomonation, only keeping those which have a proven positive effect on the library's performance as measured by the built-in benchmark suite (in the sense that there is a before/after statistically significant benchmark timing difference, which can be attributed to a different inlining scheme via perf).

Feel free to propose more benchmarks if you feel that the current benchmark set does not provide good enough coverage of what must be fast yet.

As too often when doing performance fine-tuning in Rust, the nice "multiple codegen units" feature that allows rustc to compile stuff fast got in the way by making compilation results (and in particular inlining decisions outside abomonation, e.g. inside of std::test::Bencher) less deterministic. I think that this feature, while undoubtedly worthwhile overall, is a pain for the purpose of precision benchmarking, so I disabled it for the "bench" build profile.

For convenience reasons, this PR is based on #22 so I would suggest reviewing #22 first, or reviewing this one commit by commit or via github's "diff range" feature in order to only consider the diff w.r.t. #22.

Fixes #21.

@@ -40,7 +39,7 @@ fn _bench_enc<T: Abomonation>(bencher: &mut Bencher, record: T) {
bencher.bytes = bytes.len() as u64;
bencher.iter(|| {
bytes.clear();
unsafe { encode(&record, &mut bytes).unwrap(); }
unsafe { encode(&record, &mut bytes).unwrap() }
Copy link
Author

Choose a reason for hiding this comment

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

Scarily enough, a semicolon is all it takes to defeat Bencher's hidden black_box mechanism.

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.

Is inline_always really needed?
1 participant