Skip to content

Refactor check_light_mesh_visibility for performance #13900

Closed
re0312 wants to merge 8 commits intobevyengine:mainfrom
re0312:split
Closed

Refactor check_light_mesh_visibility for performance #13900
re0312 wants to merge 8 commits intobevyengine:mainfrom
re0312:split

Conversation

@re0312
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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 <[email protected]>
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