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

Refactor check_light_mesh_visibility for performance #13900

Closed
wants to merge 8 commits into from

Conversation

re0312
Copy link
Contributor

@re0312 re0312 commented Jun 17, 2024

Objective

  • The current check_light_mesh_visibility system has unsatisfactory performance, as it involves multiple hash lookups for every mesh entity and lacks parallelization

Solution

  • split check_light_mesh_visibility to check_dir_light_mesh_visibility and check_point_light_mesh_visibility

  • 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.

Testing

cargo run --release --example many_cubes --features bevy/trace_tracy -- --shadows

image

command apply:
image

reduce from 4.88ms to 972+ 303 =1.2ms ,~4x speed up.

cargo run --release --example many_foxes --features bevy/trace_tracy -- --count 10000

image

command apply:
image

reduce from 646us to 142+ 97 =239us ,~3x speed up.

In the most common scenarios, there is usually one directional light and multiple point lights, and this PR should provide more advantages as direction light checking can run in parallel with point light checking.

@janhohenheim janhohenheim added A-Rendering Drawing game state to the screen C-Performance A change motivated by improving speed, memory usage or compile times D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jun 17, 2024
@IceSentry
Copy link
Contributor

Could you maybe make a separate PR for the split? It's a bit hard to review the other changes because it makes the diff very noisy

@re0312
Copy link
Contributor Author

re0312 commented Jun 18, 2024

Could you maybe make a separate PR for the split? It's a bit hard to review the other changes because it makes the diff very noisy

make sense, i split this pr to #13905 and #13906 for better review.

@alice-i-cecile
Copy link
Member

This has now been split :)

github-merge-queue bot pushed a commit that referenced this pull request Jun 18, 2024
# Objective

- first part of #13900 

## Solution

- split `check_light_mesh_visibility `into
`check_dir_light_mesh_visibility `and
`check_point_light_mesh_visibility` for better review
github-merge-queue bot pushed a commit that referenced this pull request Jun 26, 2024
# Objective

- Second part of #13900 
- based on #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
Labels
A-Rendering Drawing game state to the screen C-Performance A change motivated by improving speed, memory usage or compile times D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Needs-Review Needs reviewer attention (from anyone!) to move forward
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants