-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
Avoid unnecessary copying with BufferVec::swap #4902
Conversation
I think my main concern with this would be people expecting the extracted joints to be populated and their system being scheduled for after the vec was swapped and their thing doesn’t work. It’sa bit of a hidden gotcha. Is there some way to document it on the extracted joints type perhaps? |
An alternative would be just to have a clear/append option that utilizes a faster batch memcpy of the entire thing. Or just write directly from the ExtractedJoints instead of copying it into the BufferVec first. It'd be much faster than pushing one by one. With that said, I'm not sure what kind of use case requires reading the extracted joints in prepare that isn't just skinned meshes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is fine for now. If someone wants access to the extracted joints after prepare_skinned_meshes has been run we can deal with that later. Otherwise they can schedule their system to run first.
Closing in favor of #6833 for the target use case. |
# Objective Following #4402, extract systems run on the render world instead of the main world, and allow retained state operations on it's resources. We're currently extracting to `ExtractedJoints` and then copying it twice during Prepare. Once into `SkinnedMeshJoints` and again into the actual GPU buffer. This makes #4902 obsolete. ## Solution Cut out the middle copy and directly extract joints into `SkinnedMeshJoints` and remove `ExtractedJoints` entirely. This also removes the per-frame allocation that is being made to send `ExtractedJoints` into the render world. ## Performance On my local machine, this halves the time for `prepare_skinned _meshes` on `many_foxes` (195.75us -> 93.93us on average). ![image](https://user-images.githubusercontent.com/3137680/205427455-ab91a8a3-a6b0-4f0a-bd48-e54482c563b2.png) --- ## Changelog Added: `BufferVec::truncate` Added: `BufferVec::extend` Changed: `SkinnedMeshJoints::build` now takes a `&mut BufferVec` instead of a `&mut Vec` as a parameter. Removed: `ExtractedJoints`. ## Migration Guide `ExtractedJoints` has been removed. Read the bound bones from `SkinnedMeshJoints` instead.
# Objective Following #4402, extract systems run on the render world instead of the main world, and allow retained state operations on it's resources. We're currently extracting to `ExtractedJoints` and then copying it twice during Prepare. Once into `SkinnedMeshJoints` and again into the actual GPU buffer. This makes #4902 obsolete. ## Solution Cut out the middle copy and directly extract joints into `SkinnedMeshJoints` and remove `ExtractedJoints` entirely. This also removes the per-frame allocation that is being made to send `ExtractedJoints` into the render world. ## Performance On my local machine, this halves the time for `prepare_skinned _meshes` on `many_foxes` (195.75us -> 93.93us on average). ![image](https://user-images.githubusercontent.com/3137680/205427455-ab91a8a3-a6b0-4f0a-bd48-e54482c563b2.png) --- ## Changelog Added: `BufferVec::truncate` Added: `BufferVec::extend` Changed: `SkinnedMeshJoints::build` now takes a `&mut BufferVec` instead of a `&mut Vec` as a parameter. Removed: `ExtractedJoints`. ## Migration Guide `ExtractedJoints` has been removed. Read the bound bones from `SkinnedMeshJoints` instead.
# Objective Following #4402, extract systems run on the render world instead of the main world, and allow retained state operations on it's resources. We're currently extracting to `ExtractedJoints` and then copying it twice during Prepare. Once into `SkinnedMeshJoints` and again into the actual GPU buffer. This makes #4902 obsolete. ## Solution Cut out the middle copy and directly extract joints into `SkinnedMeshJoints` and remove `ExtractedJoints` entirely. This also removes the per-frame allocation that is being made to send `ExtractedJoints` into the render world. ## Performance On my local machine, this halves the time for `prepare_skinned _meshes` on `many_foxes` (195.75us -> 93.93us on average). ![image](https://user-images.githubusercontent.com/3137680/205427455-ab91a8a3-a6b0-4f0a-bd48-e54482c563b2.png) --- ## Changelog Added: `BufferVec::truncate` Added: `BufferVec::extend` Changed: `SkinnedMeshJoints::build` now takes a `&mut BufferVec` instead of a `&mut Vec` as a parameter. Removed: `ExtractedJoints`. ## Migration Guide `ExtractedJoints` has been removed. Read the bound bones from `SkinnedMeshJoints` instead.
# Objective Following #4402, extract systems run on the render world instead of the main world, and allow retained state operations on it's resources. We're currently extracting to `ExtractedJoints` and then copying it twice during Prepare. Once into `SkinnedMeshJoints` and again into the actual GPU buffer. This makes #4902 obsolete. ## Solution Cut out the middle copy and directly extract joints into `SkinnedMeshJoints` and remove `ExtractedJoints` entirely. This also removes the per-frame allocation that is being made to send `ExtractedJoints` into the render world. ## Performance On my local machine, this halves the time for `prepare_skinned _meshes` on `many_foxes` (195.75us -> 93.93us on average). ![image](https://user-images.githubusercontent.com/3137680/205427455-ab91a8a3-a6b0-4f0a-bd48-e54482c563b2.png) --- ## Changelog Added: `BufferVec::truncate` Added: `BufferVec::extend` Changed: `SkinnedMeshJoints::build` now takes a `&mut BufferVec` instead of a `&mut Vec` as a parameter. Removed: `ExtractedJoints`. ## Migration Guide `ExtractedJoints` has been removed. Read the bound bones from `SkinnedMeshJoints` instead.
# Objective Following #4402, extract systems run on the render world instead of the main world, and allow retained state operations on it's resources. We're currently extracting to `ExtractedJoints` and then copying it twice during Prepare. Once into `SkinnedMeshJoints` and again into the actual GPU buffer. This makes #4902 obsolete. ## Solution Cut out the middle copy and directly extract joints into `SkinnedMeshJoints` and remove `ExtractedJoints` entirely. This also removes the per-frame allocation that is being made to send `ExtractedJoints` into the render world. ## Performance On my local machine, this halves the time for `prepare_skinned _meshes` on `many_foxes` (195.75us -> 93.93us on average). ![image](https://user-images.githubusercontent.com/3137680/205427455-ab91a8a3-a6b0-4f0a-bd48-e54482c563b2.png) --- ## Changelog Added: `BufferVec::truncate` Added: `BufferVec::extend` Changed: `SkinnedMeshJoints::build` now takes a `&mut BufferVec` instead of a `&mut Vec` as a parameter. Removed: `ExtractedJoints`. ## Migration Guide `ExtractedJoints` has been removed. Read the bound bones from `SkinnedMeshJoints` instead.
# Objective Following bevyengine#4402, extract systems run on the render world instead of the main world, and allow retained state operations on it's resources. We're currently extracting to `ExtractedJoints` and then copying it twice during Prepare. Once into `SkinnedMeshJoints` and again into the actual GPU buffer. This makes bevyengine#4902 obsolete. ## Solution Cut out the middle copy and directly extract joints into `SkinnedMeshJoints` and remove `ExtractedJoints` entirely. This also removes the per-frame allocation that is being made to send `ExtractedJoints` into the render world. ## Performance On my local machine, this halves the time for `prepare_skinned _meshes` on `many_foxes` (195.75us -> 93.93us on average). ![image](https://user-images.githubusercontent.com/3137680/205427455-ab91a8a3-a6b0-4f0a-bd48-e54482c563b2.png) --- ## Changelog Added: `BufferVec::truncate` Added: `BufferVec::extend` Changed: `SkinnedMeshJoints::build` now takes a `&mut BufferVec` instead of a `&mut Vec` as a parameter. Removed: `ExtractedJoints`. ## Migration Guide `ExtractedJoints` has been removed. Read the bound bones from `SkinnedMeshJoints` instead.
# Objective Following bevyengine#4402, extract systems run on the render world instead of the main world, and allow retained state operations on it's resources. We're currently extracting to `ExtractedJoints` and then copying it twice during Prepare. Once into `SkinnedMeshJoints` and again into the actual GPU buffer. This makes bevyengine#4902 obsolete. ## Solution Cut out the middle copy and directly extract joints into `SkinnedMeshJoints` and remove `ExtractedJoints` entirely. This also removes the per-frame allocation that is being made to send `ExtractedJoints` into the render world. ## Performance On my local machine, this halves the time for `prepare_skinned _meshes` on `many_foxes` (195.75us -> 93.93us on average). ![image](https://user-images.githubusercontent.com/3137680/205427455-ab91a8a3-a6b0-4f0a-bd48-e54482c563b2.png) --- ## Changelog Added: `BufferVec::truncate` Added: `BufferVec::extend` Changed: `SkinnedMeshJoints::build` now takes a `&mut BufferVec` instead of a `&mut Vec` as a parameter. Removed: `ExtractedJoints`. ## Migration Guide `ExtractedJoints` has been removed. Read the bound bones from `SkinnedMeshJoints` instead.
Objective
prepare_skinned_meshes
is repeatedly copying values fromExtractedJoints
when both end up identical to each other and cleared every frame.Solution
BufferVec::swap
to directly replace the backingVec
with a pre-prepared one. Avoid the copy.Performance
On
many_foxes
,this halves the time spent inprepare_skinned_meshes
:Almost all of the time spent in the system is now spent writing the buffer out.
Changelog
Added:
BufferVec::swap
.