From 20277006ce318b4cdcf264579c2a6c126f67bc9f Mon Sep 17 00:00:00 2001 From: BD103 <59022059+BD103@users.noreply.github.com> Date: Sat, 21 Dec 2024 17:30:29 -0500 Subject: [PATCH] Add benchmarks and `compile_fail` tests back to workspace (#16858) # Objective - Our benchmarks and `compile_fail` tests lag behind the rest of the engine because they are not in the Cargo workspace, so not checked by CI. - Fixes #16801, please see it for further context! ## Solution - Add benchmarks and `compile_fail` tests to the Cargo workspace. - Fix any leftover formatting issues and documentation. ## Testing - I think CI should catch most things! ## Questions
Outdated issue I was having with function reflection being optional The `reflection_types` example is failing in Rust-Analyzer for me, but not a normal check. ```rust error[E0004]: non-exhaustive patterns: `ReflectRef::Function(_)` not covered --> examples/reflection/reflection_types.rs:81:11 | 81 | match value.reflect_ref() { | ^^^^^^^^^^^^^^^^^^^ pattern `ReflectRef::Function(_)` not covered | note: `ReflectRef<'_>` defined here --> /Users/bdeep/dev/bevy/bevy/crates/bevy_reflect/src/kind.rs:178:1 | 178 | pub enum ReflectRef<'a> { | ^^^^^^^^^^^^^^^^^^^^^^^ ... 188 | Function(&'a dyn Function), | -------- not covered = note: the matched value is of type `ReflectRef<'_>` help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown | 126 ~ ReflectRef::Opaque(_) => {}, 127 + ReflectRef::Function(_) => todo!() | ``` I think it is because the following line is feature-gated: https://github.com/bevyengine/bevy/blob/cc0f6a8db43581755bd84302072c7b97ea51bc0f/examples/reflection/reflection_types.rs#L117-L122 My theory for why this is happening is because the benchmarks enabled `bevy_reflect`'s `function` feature, which gets merged with the rest of the features when RA checks the workspace, but the `#[cfg(...)]` gate in the example isn't detecting it: https://github.com/bevyengine/bevy/blob/cc0f6a8db43581755bd84302072c7b97ea51bc0f/benches/Cargo.toml#L19 Any thoughts on how to fix this? It's not blocking, since the example still compiles as normal, but it's just RA and the command `cargo check --workspace --all-targets` appears to fail. --- Cargo.toml | 22 +++++------ benches/Cargo.toml | 4 -- benches/README.md | 38 +++++++++++-------- benches/benches/bevy_ecs/entity_cloning.rs | 2 +- benches/benches/bevy_reflect/map.rs | 2 +- .../bevy_derive/compile_fail/tests/derive.rs | 6 ++- examples/reflection/reflection_types.rs | 5 +++ tools/compile_fail_utils/README.md | 3 +- tools/compile_fail_utils/src/lib.rs | 11 +++++- 9 files changed, 55 insertions(+), 38 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 56aac74f53cb9..bb06e2afc7e64 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -13,20 +13,20 @@ documentation = "https://docs.rs/bevy" rust-version = "1.83.0" [workspace] -exclude = [ - "benches", - "crates/bevy_derive/compile_fail", - "crates/bevy_ecs/compile_fail", - "crates/bevy_reflect/compile_fail", - "tools/compile_fail_utils", -] +resolver = "2" members = [ + # All of Bevy's official crates are within the `crates` folder! "crates/*", + # Several crates with macros have "compile fail" tests nested inside them, also known as UI + # tests, that verify diagnostic output does not accidentally change. + "crates/*/compile_fail", + # Examples of compiling Bevy for mobile platforms. "examples/mobile", - "tools/ci", - "tools/build-templated-pages", - "tools/build-wasm-example", - "tools/example-showcase", + # Benchmarks + "benches", + # Internal tools that are not published. + "tools/*", + # Bevy's error codes. This is a crate so we can automatically check all of the code blocks. "errors", ] diff --git a/benches/Cargo.toml b/benches/Cargo.toml index b05e8bba351f5..2ed96d3a48c8c 100644 --- a/benches/Cargo.toml +++ b/benches/Cargo.toml @@ -32,10 +32,6 @@ rand_chacha = "0.3" [target.'cfg(target_os = "linux")'.dev-dependencies] bevy_winit = { path = "../crates/bevy_winit", features = ["x11"] } -[profile.release] -opt-level = 3 -lto = true - [lints.clippy] doc_markdown = "warn" manual_let_else = "warn" diff --git a/benches/README.md b/benches/README.md index 2641ab027aa72..2e91916e481f1 100644 --- a/benches/README.md +++ b/benches/README.md @@ -1,27 +1,35 @@ # Bevy Benchmarks -This is a crate with a collection of benchmarks for Bevy, separate from the rest of the Bevy crates. +This is a crate with a collection of benchmarks for Bevy. -## Running the benchmarks +## Running benchmarks -1. Setup everything you need for Bevy with the [setup guide](https://bevyengine.org/learn/book/getting-started/setup/). -2. Move into the `benches` directory (where this README is located). +Benchmarks can be run through Cargo: - ```sh - bevy $ cd benches - ``` +```sh +# Run all benchmarks. (This will take a while!) +cargo bench -p benches -3. Run the benchmarks with cargo (This will take a while) +# Just compile the benchmarks, do not run them. +cargo bench -p benches --no-run - ```sh - bevy/benches $ cargo bench - ``` +# Run the benchmarks for a specific crate. (See `Cargo.toml` for a complete list of crates +# tracked.) +cargo bench -p benches --bench ecs - If you'd like to only compile the benchmarks (without running them), you can do that like this: +# Filter which benchmarks are run based on the name. This will only run benchmarks whose name +# contains "name_fragment". +cargo bench -p benches -- name_fragment - ```sh - bevy/benches $ cargo bench --no-run - ``` +# List all available benchmarks. +cargo bench -p benches -- --list + +# Save a baseline to be compared against later. +cargo bench -p benches --save-baseline before + +# Compare the current benchmarks against a baseline to find performance gains and regressions. +cargo bench -p benches --baseline before +``` ## Criterion diff --git a/benches/benches/bevy_ecs/entity_cloning.rs b/benches/benches/bevy_ecs/entity_cloning.rs index d0c71c7c2ae8e..51af20b7b187d 100644 --- a/benches/benches/bevy_ecs/entity_cloning.rs +++ b/benches/benches/bevy_ecs/entity_cloning.rs @@ -91,7 +91,7 @@ fn hierarchy( .spawn(black_box(C::default())) .set_parent(parent_id) .id(); - hierarchy_level.push(child_id) + hierarchy_level.push(child_id); } } } diff --git a/benches/benches/bevy_reflect/map.rs b/benches/benches/bevy_reflect/map.rs index 054dcf9570da0..fc9da0aa08dcd 100644 --- a/benches/benches/bevy_reflect/map.rs +++ b/benches/benches/bevy_reflect/map.rs @@ -217,7 +217,7 @@ fn dynamic_map_get(criterion: &mut Criterion) { bencher.iter(|| { for i in 0..size as u64 { let key = black_box(i); - black_box(assert!(map.get(&key).is_some())); + black_box(map.get(&key)); } }); }, diff --git a/crates/bevy_derive/compile_fail/tests/derive.rs b/crates/bevy_derive/compile_fail/tests/derive.rs index b918abe2733de..1349ca060df26 100644 --- a/crates/bevy_derive/compile_fail/tests/derive.rs +++ b/crates/bevy_derive/compile_fail/tests/derive.rs @@ -1,4 +1,6 @@ fn main() -> compile_fail_utils::ui_test::Result<()> { - compile_fail_utils::test_multiple("derive_deref", ["tests/deref_derive", "tests/deref_mut_derive"]) + compile_fail_utils::test_multiple( + "derive_deref", + ["tests/deref_derive", "tests/deref_mut_derive"], + ) } - diff --git a/examples/reflection/reflection_types.rs b/examples/reflection/reflection_types.rs index 265e5f8adce60..689f012782840 100644 --- a/examples/reflection/reflection_types.rs +++ b/examples/reflection/reflection_types.rs @@ -124,6 +124,11 @@ fn setup() { // implementation. Opaque is implemented for opaque types like String and Instant, // but also include primitive types like i32, usize, and f32 (despite not technically being opaque). ReflectRef::Opaque(_) => {} + #[allow( + unreachable_patterns, + reason = "This example cannot always detect when `bevy_reflect/functions` is enabled." + )] + _ => {} } let mut dynamic_list = DynamicList::default(); diff --git a/tools/compile_fail_utils/README.md b/tools/compile_fail_utils/README.md index 5d811bf31712b..9fe8d28c9438c 100644 --- a/tools/compile_fail_utils/README.md +++ b/tools/compile_fail_utils/README.md @@ -1,6 +1,6 @@ # Helpers for compile fail tests -This crate contains everything needed to set up compile tests for the Bevy repo. It, like all Bevy compile test crates, is excluded from the Bevy workspace. This is done to not fail [`crater` tests](https://github.com/rust-lang/crater) for Bevy. The `CI` workflow executes these tests on the stable rust toolchain see ([tools/ci](../../tools/ci/src/main.rs)). +This crate contains everything needed to set up compile tests for the Bevy repo. The `CI` workflow executes these tests on the stable rust toolchain (see [tools/ci](../../tools/ci/src/main.rs)). ## Writing new test cases @@ -34,7 +34,6 @@ This will be a rather involved process. You'll have to: - Create a folder called `tests` within the new crate. - Add a test runner file to this folder. The file should contain a main function calling one of the test functions defined in this crate. - Add a `[[test]]` table to the `Cargo.toml`. This table will need to contain `harness = false` and `name = `. -- Add the path of the new crate under `[workspace].exclude` in the root [`Cargo.toml`](../../Cargo.toml). - Modify the [`CI`](../../tools/ci/) tool to run `cargo test` on this crate. - And finally, write your compile tests. diff --git a/tools/compile_fail_utils/src/lib.rs b/tools/compile_fail_utils/src/lib.rs index ff6383cc224cb..aadccac25b607 100644 --- a/tools/compile_fail_utils/src/lib.rs +++ b/tools/compile_fail_utils/src/lib.rs @@ -109,10 +109,17 @@ pub fn test_with_multiple_configs( test_name: impl Into, configs: impl IntoIterator>, ) -> ui_test::Result<()> { - let configs = configs.into_iter().collect::>>()?; + let configs = configs + .into_iter() + .collect::>>()?; let emitter: Box = if env::var_os("CI").is_some() { - Box::new((Text::verbose(), Gha:: { name: test_name.into() })) + Box::new(( + Text::verbose(), + Gha:: { + name: test_name.into(), + }, + )) } else { Box::new(Text::quiet()) };