Automatically add systems to sets based on their world accesses #8595

joseph-gio · 2023-05-11T19:16:13Z

Objective

Resolves #7857.

Add a new type of SystemSet that is automatically populated with systems based on the world data they access. This is a powerful tool for ordering systems based on logical constraints rather than global reasoning. As an example of what this allows, you could schedule an EventReader system to automatically run after any system that contains an EventWriter. This has a few advantages:

Easier to write -- no need to manually add dependencies or assign systems to sets.
Captures programmer intent -- if you write system_a.after(system_b), it isn't immediately clear why this dependency exists. If you write system_a.after(SendsEvent<T>), it is much more clear what purpose the dependency serves.
Maintainability -- if you add a new EventWriter system, you don't have to worry about forgetting to add a dependency to the relevant systems.

Solution

Systems are automatically added to an "access set" if a system accesses the component associated with the set. Like system type sets, outside configuration is not allowed, and arbitrary systems may not be added to them. Access sets are created as-needed, meaning they will not be initialized or populated unless you actually use them.

schedule.add_systems((
    move_player_system,
    move_enemy_system,
    update_global_transform.after(ComponentAccessSet::Writes::<Transform>),
));

Unresolved Questions

Should there be a way of removing/hiding a system from an access set? How would this work and what API should we use?
Should &World/&mut World count as reading/writing every single component? No, it should not.

Changelog

Added the system sets ComponentAccessSet and ResourceAccessSet. Any systems that access a given type of component or resource are automatically added to the corresponding system set.

alice-i-cecile · 2023-05-16T13:23:23Z

I really, really like these. As I've built out my game, and wrestled with questions of system ordering at scale, I've frequently found myself defining abstractions like this on my own.

Furthermore, this information is invaluable for schedule visualization and inspection: the list of "all systems that write to X" is incredibly useful to plan out orderings and debug problems.

Can we get some quick before / after benchmarking of schedule init time to make sure there's no huge regression?

joseph-gio · 2023-05-16T16:00:13Z

I ran our existing benchmarks related to the schedule, and there's just a couple of +-1% differences.

schedule/base time: [45.093 µs 45.184 µs 45.281 µs] change: [-1.9576% -1.6382% -1.3208%] (p = 0.00 < 0.05) Performance has improved. Found 5 outliers among 100 measurements (5.00%) 4 (4.00%) high mild 1 (1.00%) high severe

build_schedule/100_schedule_noconstraints
time: [594.28 µs 596.49 µs 599.15 µs]
change: [+1.8818% +2.5395% +3.1193%] (p = 0.00 < 0.05)
Performance has regressed.
Found 6 outliers among 100 measurements (6.00%)
1 (1.00%) low mild
3 (3.00%) high mild
2 (2.00%) high severe
build_schedule/100_schedule
time: [14.612 ms 14.709 ms 14.822 ms]
change: [+1.0021% +1.7401% +2.5998%] (p = 0.00 < 0.05)
Performance has regressed.
Found 6 outliers among 100 measurements (6.00%)
2 (2.00%) high mild
4 (4.00%) high severe
build_schedule/500_schedule_noconstraints
time: [8.4230 ms 8.4956 ms 8.5714 ms]
change: [-1.5862% -0.3335% +0.9023%] (p = 0.61 > 0.05)
No change in performance detected.
Found 1 outliers among 100 measurements (1.00%)
1 (1.00%) high mild
Benchmarking build_schedule/500_schedule: Warming up for 500.00 ms
Warning: Unable to complete 100 samples in 15.0s. You may wish to increase target time to 58.4s, or reduce sample count to 20.
build_schedule/500_schedule
time: [573.97 ms 576.25 ms 578.70 ms]
change: [+0.2155% +0.7398% +1.2416%] (p = 0.01 < 0.05)
Change within noise threshold.
Found 6 outliers among 100 measurements (6.00%)
6 (6.00%) high mild
build_schedule/1000_schedule_noconstraints
time: [31.979 ms 32.344 ms 32.721 ms]
change: [-2.0313% -0.4846% +1.2116%] (p = 0.57 > 0.05)
No change in performance detected.
Benchmarking build_schedule/1000_schedule: Warming up for 500.00 ms
Warning: Unable to complete 100 samples in 15.0s. You may wish to increase target time to 355.9s, or reduce sample count to 10.
build_schedule/1000_schedule
time: [3.4761 s 3.4805 s 3.4851 s]
change: [+0.0899% +0.2712% +0.4590%] (p = 0.00 < 0.05)
Change within noise threshold.
Found 4 outliers among 100 measurements (4.00%)
4 (4.00%) high mild

However, note that our current benchmarks aren't a very good fit for measuring this since they use empty systems (no world accesses).

joseph-gio · 2023-05-19T20:34:24Z

I've thought of another unresolved question:

Should &World/&mut World count as reading/writing every single component?

Adding an &World system to the read-access set for every single component seems like the most obviously correct approach, however I feel it would make this feature less useful since potentially many unrelated systems will be grouped into an access set.

I think this question boils down to: Are access sets for systems that may access a component? Or is it for systems where the programmer intends to access a component? I'm leaning towards the second option personally.

(Currently, this PR has an incorrect combination of both behaviors which I will need to fix.)

alice-i-cecile · 2023-05-19T20:55:42Z

I think this question boils down to: Are access sets for systems that may access a component? Or is it for systems where the programmer intends to access a component? I'm leaning towards the second option personally.

I think the second one is much more useful. We could add the alternative mode as well under a different name if there's demand.

cBournhonesque · 2023-09-10T18:04:43Z

crates/bevy_ecs/src/schedule/schedule.rs

+                            .push(id);
+
+                        // If true, that means there are old systems that need to be initialized.
+                        let mut init_old = false;


(i'm new to bevy and i'm going through this PR to understand better the ECS)

Why can there be old systems that need to be initialized?
Is it be cause the AccessSet initialization runs before some system initialization?
Would it make sense to always make sure that AccessSet initialization runs at the very end of the initialization?

It's to handle a situation like this

schedule.add_systems(my_system); schedule.run(&mut world); schedule.add_systems(other_system.after(Writes::<T>));

Diddykonga

LGTM, The idea is sound, and the implementation seems to be as well.
(Future Note: Once Relations are obtained, this will need to be reimplemented using them)

joseph-gio · 2023-10-10T00:37:14Z

I'd like extra attention to be paid to the implementation details used here, as I'm not 100% sure that the changes I made to ComponentDescriptor are correct.

alice-i-cecile · 2023-10-10T10:20:38Z

crates/bevy_ecs/src/component.rs

@@ -378,6 +380,7 @@ impl ComponentDescriptor {
            storage_type,
            is_send_and_sync: true,
            type_id: None,
+            is_resource: false,


Unlike new, there's no requirement that this method only be used for component types. I think we should pass in an argument to let users control that field directly.

Yeah I can add an argument for that. Seems like every one of my PRs is bound to have at least one breaking change at this point 😄

hymm · 2023-10-21T19:13:44Z

from cart on discord — Yesterday at 4:11 PM
I'm a bit worried about this pattern. Seems like it would be very easy to create invalid schedules. Ex: Something that needs to run after Reads but before a specific system that also happens to read T. It also seems lossy because exclusive systems wouldn't report what they read to the scheduler

I have the same concerns with this PR. Whenever you use a Writes, and then add a new system that writes T. You now have to know where each Writes is used if you accidentally create an invalid schedule. This is especially a problem when you start including plugins, which should be considered as opaque containers.

joseph-gio · 2024-03-24T14:05:27Z

Closing this since I suspect the use cases will be very limited in real applications. If someone else wants to pursue this, feel free

alice-i-cecile · 2025-03-02T21:50:14Z

A third party version of this was recently published in https://crates.io/crates/bevy_sorting

joseph-gio added 30 commits March 1, 2023 15:44

allow system sets that are automatically added based on world accesses

cf88758

add extension methods for making an access set

cf0453c

add a test case

af6f6cf

don't re-initialize components made via descriptors

3b374e2

ensure access set components are initialized

dbef14c

fix ci

8a59837

simplify test

3855257

add a test case for immutable access sets

77d6f31

add extension methods for resource access sets

0e314f2

track whether a component descriptor is a resource

ad50b81

add a test case for resource access sets

6742f7e

Merge remote-tracking branch 'upstream/main' into access-system-sets

345f5f7

Merge remote-tracking branch 'upstream/main' into access-system-sets

8c3377e

merge errors

d046de1

Update lib.rs

b97e6d8

use primitive types instead of extension methods

e381a9d

disallow outside configuration

18c1e39

use a macro to deduplicate structs

7417f19

clarify behavior of read sets

e507e0b

add sets for reads or writes

80e6669

Merge remote-tracking branch 'upstream/main' into access-system-sets

46a463e

store all resources in tables

4c1966d

remove useless system sets

7c0458f

simplify loops for set population

0f15e93

make access sets consistent

eab5712

simplify ComponentDescriptor::new_with_layout

20fea21

don't use a moved value

b58b7bc

simplify access set initialization

d6d0044

fix a borrow checker error

4320711

remove an unused method

50c7c68

alice-i-cecile added the X-Controversial There is active debate or serious implications around merging this PR label May 16, 2023

Merge remote-tracking branch 'upstream/main' into access-system-sets

4d40962

alice-i-cecile approved these changes May 16, 2023

View reviewed changes

joseph-gio added 5 commits May 19, 2023 17:45

add a unit test for &World behavior

1c2e582

don't include &World in read access sets

968a0a3

add a unit test for access sets added after a system is initialized

f66f09c

add a fixme

b4b8027

fix a bug with sets added after systems are initialized

276ad90

cBournhonesque reviewed Sep 10, 2023

View reviewed changes

rj00a mentioned this pull request Sep 21, 2023

Ignore system order ambiguities from specific components and resources. #9884

Closed

joseph-gio added 3 commits September 21, 2023 22:14

Merge branch 'main' into access-system-sets

723ae2a

Schedule::default

47854c6

cargo fmt

5fb8033

alice-i-cecile requested a review from hymm September 22, 2023 09:58

Diddykonga approved these changes Oct 9, 2023

View reviewed changes

alice-i-cecile 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 Oct 9, 2023

alice-i-cecile added this to the 0.13 milestone Oct 9, 2023

alice-i-cecile reviewed Oct 10, 2023

View reviewed changes

alice-i-cecile mentioned this pull request Nov 17, 2023

RFC: Barriers with Before/After markers #10618

Closed

Testare mentioned this pull request Dec 13, 2023

Teardown/exit systems #7067

Open

alice-i-cecile removed this from the 0.13 milestone Jan 24, 2024

joseph-gio closed this Mar 24, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Automatically add systems to sets based on their world accesses #8595

Automatically add systems to sets based on their world accesses #8595

joseph-gio commented May 11, 2023 •

edited

Loading

alice-i-cecile commented May 16, 2023

joseph-gio commented May 16, 2023

joseph-gio commented May 19, 2023

alice-i-cecile commented May 19, 2023

cBournhonesque Sep 10, 2023

joseph-gio Sep 10, 2023 •

edited

Loading

Diddykonga left a comment •

edited

Loading

joseph-gio commented Oct 10, 2023

alice-i-cecile Oct 10, 2023 •

edited

Loading

joseph-gio Oct 11, 2023

hymm commented Oct 21, 2023

joseph-gio commented Mar 24, 2024 •

edited

Loading

alice-i-cecile commented Mar 2, 2025

Automatically add systems to sets based on their world accesses #8595

Automatically add systems to sets based on their world accesses #8595

Conversation

joseph-gio commented May 11, 2023 • edited Loading

Objective

Solution

Unresolved Questions

Changelog

alice-i-cecile commented May 16, 2023

joseph-gio commented May 16, 2023

joseph-gio commented May 19, 2023

alice-i-cecile commented May 19, 2023

cBournhonesque Sep 10, 2023

Choose a reason for hiding this comment

joseph-gio Sep 10, 2023 • edited Loading

Choose a reason for hiding this comment

Diddykonga left a comment • edited Loading

Choose a reason for hiding this comment

joseph-gio commented Oct 10, 2023

alice-i-cecile Oct 10, 2023 • edited Loading

Choose a reason for hiding this comment

joseph-gio Oct 11, 2023

Choose a reason for hiding this comment

hymm commented Oct 21, 2023

joseph-gio commented Mar 24, 2024 • edited Loading

alice-i-cecile commented Mar 2, 2025

joseph-gio commented May 11, 2023 •

edited

Loading

joseph-gio Sep 10, 2023 •

edited

Loading

Diddykonga left a comment •

edited

Loading

alice-i-cecile Oct 10, 2023 •

edited

Loading

joseph-gio commented Mar 24, 2024 •

edited

Loading