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 README to benches #11508

Merged
merged 3 commits into from
Jan 24, 2024
Merged

Conversation

doonv
Copy link
Contributor

@doonv doonv commented Jan 24, 2024

Objective

It is unclear how to run Bevy's benchmarks

Solution

Add a README to the benches, with documentation that tells you what the benchmarks are, and how to run them.


## Running the benchmarks

1. If cargo isn't already installed, [install it](https://www.rust-lang.org/tools/install).
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really need to walk users through installing cargo here? If they don't have cargo, then they likely also don't have other required dependencies.

Maybe just mention that bevy has some dependencies and link off to https://bevyengine.org/learn/book/getting-started/setup/ or something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed the wording and made it link to the website.


## Criterion

Bevy's benchmarks use [Criterion](https://crates.io/crates/criterion) to chart and track benchmarks over time. If you want to learn more about using Criterion for benchmarking, you can read the [Criterion.rs documentation](https://bheisler.github.io/criterion.rs/book/criterion_rs.html).
Copy link
Contributor

Choose a reason for hiding this comment

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

"benchmarks use criterion to chart and track benchmarks over time" feels a bit misleading.

Individual developers can generate charts and/or compare a particular set of changes against a baseline.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed the wording a bit.

benches/README.md Show resolved Hide resolved
@rparrett rparrett added the C-Docs An addition or correction to our documentation label Jan 24, 2024
@alice-i-cecile alice-i-cecile added the C-Benchmarks Stress tests and benchmarks used to measure how fast things are label Jan 24, 2024
@doonv doonv requested a review from rparrett January 24, 2024 15:39
benches/README.md Outdated Show resolved Hide resolved
Co-authored-by: Rob Parrett <[email protected]>
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

I'm happy enough with this, and it's good to get something there to iterate on.

@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jan 24, 2024
@alice-i-cecile alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Jan 24, 2024
Merged via the queue into bevyengine:main with commit 9944993 Jan 24, 2024
23 checks passed
tjamaan pushed a commit to tjamaan/bevy that referenced this pull request Feb 6, 2024
# Objective

It is unclear how to run Bevy's benchmarks

## Solution

Add a README to the benches, with documentation that tells you what the
benchmarks are, and how to run them.

---------

Co-authored-by: Rob Parrett <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-Benchmarks Stress tests and benchmarks used to measure how fast things are C-Docs An addition or correction to our documentation S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants