Overlap system set executions

[JMS55]

What problem does this solve or what need does it fill?

Imagine an app with two systems, A and B, who don't have any conflicting resource accesses. System A is scheduled in system set X, and system B is scheduled in system set Y. The system sets are ordered (X, Y).chain().

Execution for A and B is currently done serially, even though there's nothing in theory disallowing overlap between them.

The render world heavily uses system sets to order execution, as otherwise it would be very difficult to always manually specify that bind_foo systems need to run after prepare_a, prepare_b, prepare_c, etc.

However this leads to unfortunate gaps where we have idle threads waiting for a few final systems to finish before starting the next system set:

The caveat is dependencies that Bevy can't track, such as interior mutability on read-only component/resource accesses, or global variables.

What solution would you like?

Allow execution of systems in different system sets to overlap.

What alternative(s) have you considered?

Rework the render world to not use system sets.

[alice-i-cecile]

The term of art that we've used for this before is "if-needed system ordering". This is approximately one X-Uncontroversial PR away from implementation, and one X-Controversial PR away from making the default.

[alice-i-cecile]

Quick notes on implementation:

[21:14]Alice 🌹
BEVY
: You're going to want to look into how we resolve before / after currently when constructing a https://dev-docs.bevy.org/bevy/ecs/schedule/struct.ScheduleGraph.html
ScheduleGraph in bevy::ecs::schedule - Rust
Metadata for a Schedule.
[21:15]Alice 🌹
BEVY
: Basically, you'll want to add an additional check to remove edges that don't have any conflicting accesses (you can use the ambiguity code for that)
[21:16]Alice 🌹
BEVY
: But I suspect we'll want to have both strict and non-strict orderings
[21:16]Alice 🌹
BEVY
: So you'll need to store information about that distinction, and only do the cleanup pass on non-strict edges
@jasmine
But there's no actual need to serialize a lot of the systems. The sets exist just to make system dependencies easier.
[21:17]Alice 🌹
BEVY
: This doesn't surprise me I thought the same thing when I was poking at your system sets when investigating system ambiguities
NEW
@quartermeister
I haven't looked at the code in 19602, but "extended borrows from State to Fetch to Item" sounds a bit like what I had tried to do in #15396, where the QueryData::Item could borrow from WorldQuery::State by threading the 's lifetime through queries. I let it stall out because it has some tricky interactions with #18162, and I thought 18162 would be more valuable and less controversial so I wanted to let it go through first. I could try to get 15396 working again next week if we think it would help!

[vonforum]

I feel like this is a wrong solution to the problem.

Personally, I'm against this, because I expect code to do what I told it to. If I specify .chain() between system sets, then I want the systems in those sets to run after one another, because I explicitly asked for it. I wouldn't be happy if bevy decided it knew better.

As I understand the problem, it seems what the real issue is, is a lack of unnamed sets? I'm imagining the issue is that we have a PrepareSet and a BindSet and all prepare_* functions are in the former and all the bind_* in the latter. But if prepare_a is done, bind_a can't run until prepare_c is also finished?

I feel like the solution to that would be some sort of dynamic anonymous set, something like

let prepare_set = vec![prepare_a_a];
let bind_set = vec![bind_a];
app.configure_sets((prepare_set, bind_set).chain());

[JMS55]

The problem is that it would be a lot of dependencies if we had to manually specify every system ordering. It would break really easily and be hard for 3rd party plugins to extend.

I'd be fine if we made this an optional mode, but I do think it's the best solution we have.

[vonforum]

Okay, fair enough. I think I visualize the problem more clearly now.

Maybe something like

let (my_prepare, my_bind) = app.clone_sets((Prepare, Bind));

which would maintain the relationships between the sets. So the user knows that my_bind will run after my_prepare, but can run at the same time as any other Prepare.

Though it's still an extra line of code for every group of systems, so might still be a headache.

In general though, I'm no longer opposed to this, but I do think it needs to be optional.

[JMS55]

I'm not sure what the clone_sets() API would do in your proposal. Can you elaborate?

I was thinking we could add an IntoScheduleConfigs::chain_loose() method, and use that when setting up RenderSystems.

// Current
schedule.configure_sets(
    (
        ExtractCommands,
        PrepareMeshes,
        ManageViews,
        Queue,
        PhaseSort,
        Prepare,
        Render,
        Cleanup,
        PostCleanup,
    )
        .chain(),
);

// Proposed
schedule.configure_sets(
    (
        ExtractCommands,
        PrepareMeshes,
        ManageViews,
        Queue,
        PhaseSort,
        Prepare,
        Render,
        Cleanup,
        PostCleanup,
    )
        .chain_loose(),
);