Skip to content

Refactor/benches#3505

Closed
danieleades wants to merge 6 commits intobevyengine:mainfrom
danieleades:refactor/benches
Closed

Refactor/benches#3505
danieleades wants to merge 6 commits intobevyengine:mainfrom
danieleades:refactor/benches

Conversation

@danieleades
Copy link
Contributor

i realised that my PR over at #2934 didn't include the benches/ crate. this PR covers that.

@github-actions github-actions bot added the S-Needs-Triage This issue needs to be labelled label Dec 31, 2021
let mut v = (0..10000).collect::<Vec<usize>>();
c.bench_function("for_each_iter", |b| {
b.iter(|| {
v.iter_mut().for_each(|x| {
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'm suddenly realising that the point of this bench test is to bench the for_each function...
i'll add an explicit "allow" here

@mockersf mockersf added C-Code-Quality A section of code that is hard to understand or change and removed S-Needs-Triage This issue needs to be labelled labels Dec 31, 2021
@mockersf
Copy link
Member

mockersf commented Jan 2, 2022

Looking at the changes, it probably won't have any impacts, but did you try running the benches before / after?

@danieleades
Copy link
Contributor Author

Looking at the changes, it probably won't have any impacts, but did you try running the benches before / after?

i didn't, no. As you say, there are no changes here that could affect performance

@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 Jul 4, 2022
@alice-i-cecile
Copy link
Member

alice-i-cecile commented Jul 4, 2022

@danieleades these changes rock and we overlooked them, sorry.

This is about to conflict massively with #5110; can you update this PR after that? If you're too busy just let us know and I'll get someone else to tackle it.

@alice-i-cecile alice-i-cecile added A-Diagnostics Logging, crash handling, error reporting and performance analysis S-Adopt-Me The original PR author has no intent to complete this work. Pick me up! labels Jul 4, 2022
bors bot pushed a commit that referenced this pull request Jan 11, 2023
# Objective

- #3505 marked `S-Adopt-Me` , this pr is to continue his work.

## Solution

- run `cargo clippy --workspace --all-targets --all-features -- -Aclippy::type_complexity -Wclippy::doc_markdown -Wclippy::redundant_else -Wclippy::match_same_arms -Wclippy::semicolon_if_nothing_returned -Wclippy::explicit_iter_loop -Wclippy::map_flatten -Dwarnings` under benches dir.
- fix issue according to suggestion.
@lewiszlw
Copy link
Member

This pr should be closed as #6806 has been merged. @alice-i-cecile

@danieleades danieleades deleted the refactor/benches branch January 11, 2023 12:18
alradish pushed a commit to alradish/bevy that referenced this pull request Jan 22, 2023
# Objective

- bevyengine#3505 marked `S-Adopt-Me` , this pr is to continue his work.

## Solution

- run `cargo clippy --workspace --all-targets --all-features -- -Aclippy::type_complexity -Wclippy::doc_markdown -Wclippy::redundant_else -Wclippy::match_same_arms -Wclippy::semicolon_if_nothing_returned -Wclippy::explicit_iter_loop -Wclippy::map_flatten -Dwarnings` under benches dir.
- fix issue according to suggestion.
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
# Objective

- bevyengine#3505 marked `S-Adopt-Me` , this pr is to continue his work.

## Solution

- run `cargo clippy --workspace --all-targets --all-features -- -Aclippy::type_complexity -Wclippy::doc_markdown -Wclippy::redundant_else -Wclippy::match_same_arms -Wclippy::semicolon_if_nothing_returned -Wclippy::explicit_iter_loop -Wclippy::map_flatten -Dwarnings` under benches dir.
- fix issue according to suggestion.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Diagnostics Logging, crash handling, error reporting and performance analysis C-Code-Quality A section of code that is hard to understand or change S-Adopt-Me The original PR author has no intent to complete this work. Pick me up! 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.

4 participants