Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Don't unconditionally create temporary render entities for visible objects. #16723

Merged

Conversation

pcwalton
Copy link
Contributor

@pcwalton pcwalton commented Dec 9, 2024

PR #15756 made us create temporary render entities for all visible objects, even if they had no render world counterpart. This regressed our many_cubes time from about 3.59 ms/frame to 4.66 ms/frame.

This commit changes that behavior to use Entity::PLACEHOLDER instead of creating a temporary render entity. This improves our many_cubes time from 5.66 ms/frame to 3.96 ms/frame, a 43% speedup.

I tested 3D, 2D gizmos, and UI and they seem to work.

See the following graph of many_cubes frame time (lower is better). PR #15756 is the one in October.

Time (ms_frame) vs  Date(3)

objects.

PR bevyengine#15756 made us create temporary render entities for all visible
objects, even if they had no render world counterpart. This regressed
our `many_cubes` time from about 3.59 ms/frame to 4.66 ms/frame.

This commit changes that behavior to use `Entity::PLACEHOLDER` instead
of creating a temporary render entity. This improves our `many_cubes`
time from 5.66 ms/frame to 3.96 ms/frame, a 43% speedup.

I tested 3D, 2D gizmos, and UI and they seem to work.
@pcwalton pcwalton added C-Performance A change motivated by improving speed, memory usage or compile times S-Needs-Review Needs reviewer attention (from anyone!) to move forward A-Rendering Drawing game state to the screen P-Regression Functionality that used to work but no longer does. Add a test for this! labels Dec 9, 2024
Copy link
Contributor

@tychedelia tychedelia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for looking into this.

@pcwalton pcwalton added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Dec 9, 2024
@wilk10
Copy link
Contributor

wilk10 commented Dec 9, 2024

Does this qualify for 0.15.1 ?

@pcwalton
Copy link
Contributor Author

pcwalton commented Dec 9, 2024

We should let it bake for a bit, because this has a decent probability of breaking something.

@alice-i-cecile alice-i-cecile modified the milestone: 0.15.1 Dec 9, 2024
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Dec 10, 2024
Merged via the queue into bevyengine:main with commit a81c8f9 Dec 10, 2024
34 checks passed
BD103 pushed a commit to BD103/bevy that referenced this pull request Dec 10, 2024
…jects. (bevyengine#16723)

PR bevyengine#15756 made us create temporary render entities for all visible
objects, even if they had no render world counterpart. This regressed
our `many_cubes` time from about 3.59 ms/frame to 4.66 ms/frame.

This commit changes that behavior to use `Entity::PLACEHOLDER` instead
of creating a temporary render entity. This improves our `many_cubes`
time from 5.66 ms/frame to 3.96 ms/frame, a 43% speedup.

I tested 3D, 2D gizmos, and UI and they seem to work.

See the following graph of `many_cubes` frame time (lower is better). PR
bevyengine#15756 is the one in October.

![Time (ms_frame) vs
Date(3)](https://github.com/user-attachments/assets/2c31a893-97bd-40f6-9e89-d2195a44cf40)
ecoskey pushed a commit to ecoskey/bevy that referenced this pull request Jan 6, 2025
…jects. (bevyengine#16723)

PR bevyengine#15756 made us create temporary render entities for all visible
objects, even if they had no render world counterpart. This regressed
our `many_cubes` time from about 3.59 ms/frame to 4.66 ms/frame.

This commit changes that behavior to use `Entity::PLACEHOLDER` instead
of creating a temporary render entity. This improves our `many_cubes`
time from 5.66 ms/frame to 3.96 ms/frame, a 43% speedup.

I tested 3D, 2D gizmos, and UI and they seem to work.

See the following graph of `many_cubes` frame time (lower is better). PR
bevyengine#15756 is the one in October.

![Time (ms_frame) vs
Date(3)](https://github.com/user-attachments/assets/2c31a893-97bd-40f6-9e89-d2195a44cf40)
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-Rendering Drawing game state to the screen C-Performance A change motivated by improving speed, memory usage or compile times P-Regression Functionality that used to work but no longer does. Add a test for this! S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants