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

Make runtime benchmark harness more flexible to use #1644

Merged
merged 2 commits into from
Jul 11, 2023

Conversation

Kobzol
Copy link
Contributor

@Kobzol Kobzol commented Jul 8, 2023

Some runtime benchmarks need only read-only access to some input data on which they operate on. Currently, it wasn't easily possible to express this in the runtime benchmark harness, and even though a benchmark only needed to read from some data, the data had to be re-created for each iteration of the benchmark (twice actually, since we run each benchmark two times, to gather instructions and time).

This meant that the data for each benchmark had to be re-created many times, which could slow down the execution of the whole benchmark group. And also the data was always created anew, possibly on a new memory address, which could introduce more noise into the benchmark, because each iteration of the benchmark was operating potentially on different regions of memory.

After this change, benchmarks that only need read-only access to some input data should only create that data once and then read it repeatedly. This can in theory mean that the data will be more available in L1/L2/L3 caches, but that's an orthogonal problem. It is even a question whether we want to avoid this, we could instead warm up the cache and perform measurements on a "prepared cache state", which could further reduce measurement variation. In any case, I have some code prepared to flush the caches if this would be considered a problem.

@Kobzol
Copy link
Contributor Author

Kobzol commented Jul 8, 2023

One slightly annoying thing is that the data will be prepared even if the runtime binary is just executed with the list parameter, which returns a list of benchmarks contained in the binary. I'm not sure what to do about that though. We only use the list command for nicer CLI output and duplicate benchmark detection, we could gather the benchmark names only when we actually run the runtime benchmark binary instead..

Copy link
Contributor

@nnethercote nnethercote left a comment

Choose a reason for hiding this comment

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

One slightly annoying thing is that the data will be prepared even if the runtime binary is just executed with the list parameter, which returns a list of benchmarks contained in the binary.

Should this be mentioned in a comment somewhere?

collector/benchlib/src/benchmark.rs Outdated Show resolved Hide resolved
@Kobzol Kobzol force-pushed the runtime-benchmark-harness branch from b723ec1 to 8b2865d Compare July 11, 2023 07:57
@Kobzol
Copy link
Contributor Author

Kobzol commented Jul 11, 2023

Thanks, I have expanded the README of the runtime benchmarks directory.

@Kobzol Kobzol enabled auto-merge July 11, 2023 07:58
@Kobzol Kobzol merged commit ca9ae60 into rust-lang:master Jul 11, 2023
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.

2 participants