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

System Yielding #17036

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Neo-Zhixing
Copy link
Contributor

@Neo-Zhixing Neo-Zhixing commented Dec 30, 2024

Objective

  • In some cases, systems may not be immediately ready for execution due to unmet constraints. For example, the execution of a certain system may be dependent upon the completion of an asynchronous GPU submission.
  • In those cases, we may not want to skip the execution of this system for the current frame altogether. Instead, we would like to defer the execution of this system without violating system dependencies. We schedule all other systems first, and once we're done, we come back and pick up where we left off.
  • This is also particularly useful for the scheduling of async systems. We can create a system wrapper that wraps an async system. Each time the system gets scheduled, we call poll on the future. And we don't mark the system as completed until the future returns Poll::Ready.

Solution

  • Added a new method, yielded, on the System trait. If yielded returns true, the scheduler defer the execution of the system until all other currently ready systems have completed execution.
  • Maybe pending is a better name for this method?

Testing

Added new unit tests. Also tested as a part of a custom implementation of a Vulkan-based rendering backend that returns GPUFutures.

Draft Release Notes

System Deferral: Introduced a new mechanism in the ECS scheduler to defer the execution of systems. This allows systems to temporarily defer their execution by allowing the scheduler to execute other systems first. This is useful in cases where the execution of a system is dependent upon the completion of an asynchronous operation. It is also useful for implementing asynchronous systems.

@ItsDoot ItsDoot added A-ECS Entities, components, systems, and events C-Performance A change motivated by improving speed, memory usage or compile times C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes labels Dec 30, 2024
@BenjaminBrienen BenjaminBrienen added the S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged label Dec 30, 2024
@alice-i-cecile alice-i-cecile added M-Needs-Release-Note Work that should be called out in the blog due to impact S-Needs-Review Needs reviewer attention (from anyone!) to move forward S-Needs-Design This issue requires design work to think about how it would best be accomplished and removed S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Dec 30, 2024
@mrchantey mrchantey self-requested a review December 31, 2024 22:53
@mrchantey
Copy link
Contributor

Awesome! Great to see the building blocks for async systems being put into place.
I guess the provided test demonstrates a low-level implementation of yielding, and implementing IntoSystemConfigs for systems that return futures (or generators?) would be a seperate pr?

@Neo-Zhixing
Copy link
Contributor Author

Implementing IntoSystemConfigs for systems that return futures turns out to be a bit difficult due to #17037

@Neo-Zhixing Neo-Zhixing force-pushed the rhyolite/system-yielding branch from 0120896 to 696b483 Compare January 1, 2025 05:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events C-Performance A change motivated by improving speed, memory usage or compile times C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes M-Needs-Release-Note Work that should be called out in the blog due to impact S-Needs-Design This issue requires design work to think about how it would best be accomplished
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants