Overhaul bezier curve benchmarks #7
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Apply `bench!` macro to all names - Rename benchmarks to be more descriptive of what they do - Deduplicate `build_pos_cubic()` and `build_accel_cubic()`, since they both benchmark the exact same thing! - Move calculation of time for curve easing outside main routine - Move `curve.position()` benchmarks under the same group, and make their routine generic - Remove unnecessary `Vec` allocation from `build_pos_cubic()`
# Objective - Contributes to bevyengine#15460 ## Solution - Added the following features: - `std` (default) ## Testing - CI ## Notes - There was a minor issue with `bevy_reflect`'s `smallvec` feature noticed in this PR which I have also resolved here. I can split this out if desired, but I've left it here for now as it's a very small change and I don't consider this PR itself to be very controversial.
…#16881) # Objective - Updates the linux-dependencies docs to 1) fix problems with nix `alsa-lib`, and 2) include additional documentation to run bevy with nix on non NixOS systems. (nix is a package manager that can be run outside of NixOS). ## Solution 1. the nix `alsa-lib` package doesn't include `alsa-plugins`, which bevy depends on. Instead, use the `alsa-lib-with-plugins` package, which wraps `alsa-plugins` into `alsa-lib`. For more information see: NixOS/nixpkgs#277180 2. using nix on non NixOS systems, software like `nixGL` is required to correctly link graphics drivers. ## Testing - Tested on ubuntu 22.04 with nix.
CodSpeed Performance ReportMerging #7 will degrade performances by 22.84%Comparing Summary
Benchmarks breakdown
|
# Objective The way `Curve` presently achieves dyn-compatibility involves shoving `Self: Sized` bounds on a bunch of methods to forbid them from appearing in vtables. (This is called *explicit non-dispatchability*.) The `Curve` trait probably also just has way too many methods on its own. In the past, using extension traits instead to achieve similar functionality has been discussed. The upshot is that this would allow the "core" of the curve trait, on which all the automatic methods rely, to live in a very simple dyn-compatible trait, while other functionality is implemented by extensions. For instance, `dyn Curve<T>` cannot use the `Sized` methods, but `Box<dyn Curve<T>>` is `Sized`, hence would automatically implement the extension trait, containing the methods which are currently non-dispatchable. Other motivations for this include modularity and code organization: the `Curve` trait itself has grown quite large with the addition of numerous adaptors, and refactoring it to demonstrate the separation of functionality that is already present makes a lot of sense. Furthermore, resampling behavior in particular is dependent on special traits that may be mimicked or analogized in user-space, and creating extension traits to achieve similar behavior in user-space is something we ought to encourage by example. ## Solution `Curve` now contains only `domain` and the `sample` methods. `CurveExt` has been created, and it contains all adaptors, along with the other sampling convenience methods (`samples`, `sample_iter`, etc.). It is implemented for all `C` where `C: Curve<T> + Sized`. `CurveResampleExt` has been created, and it contains all resampling methods. It is implemented for all `C` where `C: Curve<T> + ?Sized`. ## Testing It compiles and `cargo doc` succeeds. --- ## Future work - Consider writing extension traits for resampling curves in related domains (e.g. resampling for `Curve<T>` where `T: Animatable` into an `AnimatableKeyframeCurve`). - `CurveExt` might be further broken down to separate the adaptor and sampling methods. --- ## Migration Guide `Curve` has been refactored so that much of its functionality is now in extension traits. Adaptors such as `map`, `reparametrize`, `reverse`, and so on now require importing `CurveExt`, while the resampling methods `resample_*` require importing `CurveResampleExt`. Both of these new traits are exported through `bevy::math::curve` and through `bevy::math::prelude`.
# Objective Follow-up to bevyengine#16984 ## Solution Fix the lint ## Testing ``` PS C:\Users\BenjaminBrienen\source\bevy> cargo clippy Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.71s PS C:\Users\BenjaminBrienen\source\bevy> cargo clippy -p bevy_ecs Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.21s ```
…_tasks` (bevyengine#16951) # Objective - Related to bevyengine#11478 ## Solution - Moved `futures.rs`, `ConditionalSend` `ConditionalSendFuture` and `BoxedFuture` from `bevy_utils` to `bevy_tasks`. ## Testing - CI checks ## Migration Guide - Several modules were moved from `bevy_utils` into `bevy_tasks`: - Replace `bevy_utils::futures` imports with `bevy_tasks::futures`. - Replace `bevy_utils::ConditionalSend` with `bevy_tasks::ConditionalSend`. - Replace `bevy_utils::ConditionalSendFuture` with `bevy_tasks::ConditionalSendFuture`. - Replace `bevy_utils::BoxedFuture` with `bevy_tasks::BoxedFuture`.
…bevyengine#16994) # Objective Incorrect default value for ChromatticAberration intensity, missing a zero. Bevy 0.15
# Objective - This reverts bevyengine#16833, and completely goes against bevyengine#16803. - Turns out running `cargo test --benches` runs each benchmark once, without timing it, just to ensure nothing panics. This is actually desired because we can use it to verify benchmarks are working correctly without all the time constraints of actual benchmarks. ## Solution - Add the `--benches` flag to the CI test command. ## Testing - `cargo run -p ci -- test`
# Objective Many of our benchmarks use [`criterion::black_box()`](https://docs.rs/criterion/latest/criterion/fn.black_box.html), which is used to prevent the compiler from optimizing away computation that we're trying to time. This can be slow, though, because `criterion::black_box()` forces a point read each time it is called through [`ptr::road_volatile()`](https://doc.rust-lang.org/stable/std/ptr/fn.read_volatile.html). In Rust 1.66, the standard library introduced [`core::hint::black_box()`](https://doc.rust-lang.org/nightly/std/hint/fn.black_box.html) (and `std::hint::black_box()`). This is an intended replacement for `criterion`'s version that uses compiler intrinsics instead of volatile pointer reads, and thus has no runtime overhead. This increases benchmark accuracy, which is always nice 👍 Note that benchmarks may _appear_ to improve in performance after this change, but that's just because we are eliminating the pointer read overhead. ## Solution - Deny `criterion::black_box` in `clippy.toml`. - Fix all imports. ## Testing - `cargo clippy -p benches --benches`
…yengine#16958) # Objective Fixes bevyengine#16879 ## Solution Moved the construction of the root path of the assets folder out of `FileWatcher::new()` and into `source.rs`, as the path is checked there with `path.exists()` and fails in certain configurations eg., virtual workspaces. ## Testing Applied fix to a private fork and tested against both standard project setups and virtual workspaces. Works without issue on both. Have tested under macOS and Arch Linux. --------- Co-authored-by: JP Stringham <[email protected]> Co-authored-by: Alice Cecile <[email protected]>
…evyengine#16968) # Objective Fixes bevyengine#16683 ## Solution Make all fields ine `RawHandleWrapper` private. ## Testing - CI - `cargo clippy` - The lightmaps example --- ## Migration Guide The `window_handle` and `dispay_handle` fields on `RawHandleWrapper` are no longer public. Use the newly added getters and setters to manipulate them instead.
# Objective The rust-versions are out of date. Fixes bevyengine#17008 ## Solution Update the values Cherry-picked from bevyengine#17006 in case it is controversial ## Testing Validated locally and in bevyengine#17006 --------- Co-authored-by: Alice Cecile <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Original PR: bevyengine#17016
This is a test to see how Codspeed reacts to benchmarking pull requests.