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

Benches: improved user experience for adding new benchmarking files #13406

Closed
Walther opened this issue Feb 5, 2024 · 3 comments
Closed

Benches: improved user experience for adding new benchmarking files #13406

Walther opened this issue Feb 5, 2024 · 3 comments

Comments

@Walther
Copy link

Walther commented Feb 5, 2024

Currently, creating benchmarks for a crate looks something like this:

# Project structure
.
├── Cargo.toml
├── README.md
├── benches
│   ├── various.rs
│   ├── file.rs
│   ├── names.rs
│   └── here.rs
├── src
│   ├── various.rs
│   ├── file.rs
│   ├── names.rs
│   └── here.rs
# Mirroring the src/ file names in benches/ makes sense, for easy discoverability
...
# Cargo.toml

[dev-dependencies]
divan = "0.1.11"

[[bench]]
name = "various"
harness = false

[[bench]]
name = "file"
harness = false

[[bench]]
name = "names"
harness = false

[[bench]]
name = "here"
harness = false

# However, here it feels like a chore

This gets repetitive very quickly, and doesn't feel like the ideal developer experience.

Would it be possible to consider:

  • automatically detecting the files under benches/ so that the [[bench]] sections aren't required
  • setting harness = false by default and letting people override it to harness = true when needed

in order to simplify the process of adding new benchmarks into just adding a new file to the benches/ directory?


A related issue and comment for historical context: rust-lang/rust#29553 (comment)

@nvzqz
Copy link

nvzqz commented Feb 5, 2024

Perhaps if a main entrypoint is detected, then harness = false becomes set? Detecting main could also remove the need to list every benchmark in Cargo.toml.

@epage
Copy link
Contributor

epage commented Feb 6, 2024

The [[bench]]s are needed only because of harness = false. if you used the unstable harness, auto-discovery would work.

I can't remember if cargo already has an issue for "defaults for target kinds" but that is how I've been talking about solving this.

Either way, this is a cargo issue, so moving it (once I'm off mobile).

@epage epage transferred this issue from rust-lang/rust Feb 6, 2024
@epage
Copy link
Contributor

epage commented Feb 6, 2024

Closing as a duplicate of #6945. If there is a reason we should keep these separate, let us know!

@epage epage closed this as not planned Won't fix, can't repro, duplicate, stale Feb 6, 2024
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

No branches or pull requests

3 participants