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 support for profiling unit-test style benchmarks #276

Merged
merged 3 commits into from
Aug 9, 2023
Merged

Add support for profiling unit-test style benchmarks #276

merged 3 commits into from
Aug 9, 2023

Conversation

d-e-s-o
Copy link
Contributor

@d-e-s-o d-e-s-o commented Aug 5, 2023

This change extends the program with support for profiling unit test
benchmarks, aka libtest benchmarks. Support is accomplished in a vein
similar to that of unit tests: we introduce the --unit-bench argument
that behaves very much like the existing --unit-test does, but invokes
benchmarks instead of tests.
We cannot piggy-back on the existing --bench argument for the same
reason (presumably) that --unit-test is separate from --test: --bench
and --test require a target to be provided, but --unit-test and
--unit-bench can work without an argument or can be a pattern of tests
to match against, but not a target.

There is no need to pass in an iterator to the build() function, it just
constrains how the object can be for no real benefit. Just provide a
Vec.

Signed-off-by: Daniel Müller <[email protected]>
Clippy has a few things to complain about. Address the "issues".

Signed-off-by: Daniel Müller <[email protected]>
This change extends the program with support for profiling unit test
benchmarks, aka libtest benchmarks. Support is accomplished in a vein
similar to that of unit tests: we introduce the --unit-bench argument
that behaves very much like the existing --unit-test does, but invokes
benchmarks instead of tests.
We cannot piggy-back on the existing --bench argument for the same
reason (presumably) that --unit-test is separate from --test: --bench
and --test require a target to be provided, but --unit-test and
--unit-bench can work without an argument or can be a pattern of tests
to match against, but not a target.

Closes: #275

Signed-off-by: Daniel Müller <[email protected]>
Copy link
Contributor

@djc djc left a comment

Choose a reason for hiding this comment

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

Thanks for working on this.

Are unit-test benchmarks a documented concept in Cargo?

unit_bench: Some(Some(t)),
..
} => {
trailing_arguments.push("--bench".to_string());
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this trying to achieve? It seems like a weird setup to append to a copy of trailing_arguments.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

One runs benchmarks with the --bench switch provided to the unit test binary. See <binary> --help. We were already doing a copy a few lines down, so this doesn't really change much.

@d-e-s-o
Copy link
Contributor Author

d-e-s-o commented Aug 8, 2023

Are unit-test benchmarks a documented concept in Cargo?

Yes.

Copy link
Contributor

@djc djc left a comment

Choose a reason for hiding this comment

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

Alright, thanks for the explanations!

@djc djc merged commit 200aeca into flamegraph-rs:main Aug 9, 2023
5 checks passed
@d-e-s-o d-e-s-o deleted the topic/unit-benches branch August 9, 2023 13:43
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