Fix stealing #2
Merged
Fix stealing #2
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
james7132
added a commit
that referenced
this pull request
Jan 24, 2023
# Objective > ℹ️ **This is an adoption of bevyengine#4081 by @james7132** Fixes bevyengine#4080. Provide a way to pre-parse reflection paths so as to avoid having to parse at each call to `GetPath::path` (or similar method). ## Solution Adds the `ParsedPath` struct (named `FieldPath` in the original PR) that parses and caches the sequence of accesses to a reflected element. This is functionally similar to the `GetPath` trait, but removes the need to parse an unchanged path more than once. ### Additional Changes Included in this PR from the original is cleaner code as well as the introduction of a new pathing operation: field access by index. This allows struct and struct variant fields to be accessed in a more performant (albeit more fragile) way if needed. This operation is faster due to not having to perform string matching. As an example, if we wanted the third field on a struct, we'd write `#2`—where `#` denotes indexed access and `2` denotes the desired field index. This PR also contains improved documentation for `GetPath` and friends, including renaming some of the methods to be more clear to the end-user with a reduced risk of getting them mixed up. ### Future Work There are a few things that could be done as a separate PR (order doesn't matter— they could be followup PRs or done in parallel). These are: - [x] ~~Add support for `Tuple`. Currently, we hint that they work but they do not.~~ See bevyengine#7324 - [ ] Cleanup `ReflectPathError`. I think it would be nicer to give `ReflectPathError` two variants: `ReflectPathError::ParseError` and `ReflectPathError::AccessError`, with all current variants placed within one of those two. It's not obvious when one might expect to receive one type of error over the other, so we can help by explicitly categorizing them. --- ## Changelog - Cleaned up `GetPath` logic - Added `ParsedPath` for cached reflection paths - Added new reflection path syntax: struct field access by index (example syntax: `foo#1`) - Renamed methods on `GetPath`: - `path` -> `reflect_path` - `path_mut` -> `reflect_path_mut` - `get_path` -> `path` - `get_path_mut` -> `path_mut` ## Migration Guide `GetPath` methods have been renamed according to the following: - `path` -> `reflect_path` - `path_mut` -> `reflect_path_mut` - `get_path` -> `path` - `get_path_mut` -> `path_mut` Co-authored-by: Gino Valente <gino.valente.code@gmail.com>
james7132
pushed a commit
that referenced
this pull request
Aug 2, 2024
…3906) # Objective - Second part of bevyengine#13900 - based on bevyengine#13905 ## Solution - check_dir_light_mesh_visibility defers setting the entity's `ViewVisibility `so that Bevy can schedule it to run in parallel with `check_point_light_mesh_visibility`. - Reduce HashMap lookups for directional light checking as much as possible - Use `par_iter `to parallelize the checking process within each system. --------- Co-authored-by: Kristoffer Søholm <k.soeholm@gmail.com>
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.
Objective
runon the compute threads and scope try_tick were preventing task stealing by the i/o and async compute theadsSolution
Looking in
trace_chromeand perfetto, I can see that the async compute and i/o threads now have systems running on them. I don't see compute 0 or compute 1 for the app pool, but the os probably doesn't want to give them any time. My machine has 12 logical cores. Technically I guess we're overprovisioning threads by 1, since systems are allowed to run on the main thread.