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.

Sign up for GitHub

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

Jump to bottom

[Merged by Bors] - bevy_transform: Use Changed in the query for much faster transform_propagate_system #4180

Closed
wants to merge 1 commit into from
Closed

[Merged by Bors] - bevy_transform: Use Changed in the query for much faster transform_propagate_system #4180

wants to merge 1 commit into from

Conversation

superdump
Copy link
Contributor

Objective

  • Improve transform propagation performance

Solution

  • Use Changed<Transform> as part of the root_query and transform_query to avoid the indirection of having to look up the Entity in the changed_transform_query
  • Get rid of the changed_transform_query entirely
  • transform_propagate_system execution time for many_cubes -- sphere dropped from 1.07ms to 0.159ms, an 85% reduction for this system. Frame rate increased from ~42fps to ~44fps

@superdump superdump added the A-Rendering Drawing game state to the screen label Mar 11, 2022
@github-actions github-actions bot added the S-Needs-Triage This issue needs to be labelled label Mar 11, 2022
@superdump superdump added C-Performance A change motivated by improving speed, memory usage or compile times and removed S-Needs-Triage This issue needs to be labelled labels Mar 11, 2022
@james7132 james7132 added A-Transform Translations, rotations and scales A-Animation Make things move and change over time labels Mar 11, 2022
@james7132 james7132 mentioned this pull request Mar 13, 2022
Merged
@superdump superdump requested a review from mockersf March 13, 2022 18:04
This was referenced Mar 14, 2022
Closed
Closed
@superdump superdump mentioned this pull request Mar 20, 2022
Closed
@superdump
Copy link
Contributor Author

I've tested this with transform_hierarchy -- humanoids_mixed (modified to add back DefaultPlugins so that Tracy profiling works) which has 4000 humanoid transform hierarchies where 2000 are stationary and 2000 are actively being transformed each frame. On main the transform_propagate_system takes 8.27ms and on this PR it takes 6.68ms.

@superdump superdump force-pushed the transform-propagate-changed branch from 59ef108 to 5901315 Compare March 24, 2022 11:46
@jakobhellermann jakobhellermann added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Mar 25, 2022
@cart
Copy link
Member

cart commented Mar 29, 2022

bors r+

bors bot pushed a commit that referenced this pull request Mar 29, 2022
@superdump
bevy_transform: Use Changed in the query for much faster transform_pr…
54d2e86 
…opagate_system (#4180)

# Objective

- Improve transform propagation performance

## Solution

- Use `Changed<Transform>` as part of the `root_query` and `transform_query` to avoid the indirection of having to look up the `Entity` in the `changed_transform_query`
- Get rid of the `changed_transform_query` entirely
- `transform_propagate_system` execution time for `many_cubes -- sphere` dropped from 1.07ms to 0.159ms, an 85% reduction for this system. Frame rate increased from ~42fps to ~44fps
@bors
Copy link
Contributor

bors bot commented Mar 29, 2022

Pull request successfully merged into main.

Build succeeded:

@bors bors bot changed the title bevy_transform: Use Changed in the query for much faster transform_propagate_system [Merged by Bors] - bevy_transform: Use Changed in the query for much faster transform_propagate_system Mar 29, 2022
@bors bors bot closed this Mar 29, 2022
@DJMcNab DJMcNab mentioned this pull request May 7, 2022
Closed
bors bot pushed a commit that referenced this pull request May 9, 2022
@DJMcNab
Update layout/style when scale factor changes too (#4689)
a8bc123 
# Objective

- Fix #4688

## Solution

- Fixes #4688
- This raises an interesting question about our change detection system - is filtered queries actually a good UX for this? They're ergonomic in the easy case, but what do we recommend when it's not so.
- In this case, the system should have been migrated similary to #4180 anyway, so I've done that.
bors bot pushed a commit that referenced this pull request May 9, 2022
@DJMcNab
Update layout/style when scale factor changes too (#4689)
33a4df8 
# Objective

- Fix #4688

## Solution

- Fixes #4688
- This raises an interesting question about our change detection system - is filtered queries actually a good UX for this? They're ergonomic in the easy case, but what do we recommend when it's not so.
- In this case, the system should have been migrated similary to #4180 anyway, so I've done that.
robtfm pushed a commit to robtfm/bevy that referenced this pull request May 10, 2022
@DJMcNab @robtfm
Update layout/style when scale factor changes too (bevyengine#4689)
803c873 
# Objective

- Fix bevyengine#4688

## Solution

- Fixes bevyengine#4688
- This raises an interesting question about our change detection system - is filtered queries actually a good UX for this? They're ergonomic in the easy case, but what do we recommend when it's not so.
- In this case, the system should have been migrated similary to bevyengine#4180 anyway, so I've done that.
exjam pushed a commit to exjam/bevy that referenced this pull request May 22, 2022
@DJMcNab @exjam
Update layout/style when scale factor changes too (bevyengine#4689)
5a193ee 
# Objective

- Fix bevyengine#4688

## Solution

- Fixes bevyengine#4688
- This raises an interesting question about our change detection system - is filtered queries actually a good UX for this? They're ergonomic in the easy case, but what do we recommend when it's not so.
- In this case, the system should have been migrated similary to bevyengine#4180 anyway, so I've done that.
aevyrie pushed a commit to aevyrie/bevy that referenced this pull request Jun 7, 2022
@superdump @aevyrie
bevy_transform: Use Changed in the query for much faster transform_pr…
e1e7a79 
…opagate_system (bevyengine#4180)

# Objective

- Improve transform propagation performance

## Solution

- Use `Changed<Transform>` as part of the `root_query` and `transform_query` to avoid the indirection of having to look up the `Entity` in the `changed_transform_query`
- Get rid of the `changed_transform_query` entirely
- `transform_propagate_system` execution time for `many_cubes -- sphere` dropped from 1.07ms to 0.159ms, an 85% reduction for this system. Frame rate increased from ~42fps to ~44fps
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
@superdump @ItsDoot
bevy_transform: Use Changed in the query for much faster transform_pr…
05b0892 
…opagate_system (bevyengine#4180)

# Objective

- Improve transform propagation performance

## Solution

- Use `Changed<Transform>` as part of the `root_query` and `transform_query` to avoid the indirection of having to look up the `Entity` in the `changed_transform_query`
- Get rid of the `changed_transform_query` entirely
- `transform_propagate_system` execution time for `many_cubes -- sphere` dropped from 1.07ms to 0.159ms, an 85% reduction for this system. Frame rate increased from ~42fps to ~44fps
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
@DJMcNab @ItsDoot
Update layout/style when scale factor changes too (bevyengine#4689)
3a00d54 
# Objective

- Fix bevyengine#4688

## Solution

- Fixes bevyengine#4688
- This raises an interesting question about our change detection system - is filtered queries actually a good UX for this? They're ergonomic in the easy case, but what do we recommend when it's not so.
- In this case, the system should have been migrated similary to bevyengine#4180 anyway, so I've done that.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jakobhellermann jakobhellermann jakobhellermann approved these changes

@james7132 james7132 james7132 approved these changes

@mockersf mockersf Awaiting requested review from mockersf

Assignees
No one assigned
Labels
A-Animation Make things move and change over time A-Rendering Drawing game state to the screen A-Transform Translations, rotations and scales C-Performance A change motivated by improving speed, memory usage or compile times S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
@cart's attention
Archived in project
Rendering
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@superdump @cart @james7132 @jakobhellermann