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

Split ScheduleGraph::process_configs function (adopted) #12435

Merged
merged 4 commits into from
Mar 17, 2024
Merged

Split ScheduleGraph::process_configs function (adopted) #12435

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
dde8fb5
Split ScheduleGraph::process_configs function
stepancheg Nov 17, 2023
e46e7e7
Merge branch 'main' into pr/stepancheg/10617
yyogo Mar 12, 2024
9f9d643
re-combine process_configs and refactor
yyogo Mar 13, 2024
b8b6710
Merge branch 'main' into process-configs
yyogo Mar 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
224 changes: 89 additions & 135 deletions crates/bevy_ecs/src/schedule/schedule.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -661,6 +661,42 @@ impl ScheduleGraph {
&self.conflicting_systems
}

fn process_config<T: ProcessNodeConfig>(
&mut self,
config: NodeConfig<T>,
collect_nodes: bool,
) -> ProcessConfigsResult {
ProcessConfigsResult {
densely_chained: true,
nodes: collect_nodes
.then_some(T::process_config(self, config))
.into_iter()
.collect(),
}
}

fn apply_collective_conditions<T: ProcessNodeConfig>(
&mut self,
configs: &mut [NodeConfigs<T>],
collective_conditions: Vec<BoxedCondition>,
) {
if !collective_conditions.is_empty() {
if let [config] = configs {
for condition in collective_conditions {
config.run_if_dyn(condition);
}
} else {
let set = self.create_anonymous_set();
for config in configs.iter_mut() {
config.in_set_inner(set.intern());
}
let mut set_config = SystemSetConfig::new(set.intern());
set_config.conditions.extend(collective_conditions);
self.configure_set_inner(set_config).unwrap();
}
}
}

/// Adds the config nodes to the graph.
///
/// `collect_nodes` controls whether the `NodeId`s of the processed config nodes are stored in the returned [`ProcessConfigsResult`].
Expand All @@ -676,157 +712,75 @@ impl ScheduleGraph {
collect_nodes: bool,
) -> ProcessConfigsResult {
match configs {
NodeConfigs::NodeConfig(config) => {
let node_id = T::process_config(self, config);
if collect_nodes {
ProcessConfigsResult {
densely_chained: true,
nodes: vec![node_id],
}
} else {
ProcessConfigsResult {
densely_chained: true,
nodes: Vec::new(),
}
}
}
NodeConfigs::NodeConfig(config) => self.process_config(config, collect_nodes),
NodeConfigs::Configs {
mut configs,
collective_conditions,
chained,
} => {
let more_than_one_entry = configs.len() > 1;
if !collective_conditions.is_empty() {
if more_than_one_entry {
let set = self.create_anonymous_set();
for config in &mut configs {
config.in_set_inner(set.intern());
}
let mut set_config = SystemSetConfig::new(set.intern());
set_config.conditions.extend(collective_conditions);
self.configure_set_inner(set_config).unwrap();
} else {
for condition in collective_conditions {
configs[0].run_if_dyn(condition);
}
}
}
let mut config_iter = configs.into_iter();
let mut nodes_in_scope = Vec::new();
let mut densely_chained = true;
if chained == Chain::Yes || chained == Chain::YesIgnoreDeferred {
let Some(prev) = config_iter.next() else {
return ProcessConfigsResult {
nodes: Vec::new(),
densely_chained: true,
};
self.apply_collective_conditions(&mut configs, collective_conditions);

let ignore_deferred = matches!(chained, Chain::YesIgnoreDeferred);
let chained = matches!(chained, Chain::Yes | Chain::YesIgnoreDeferred);

// Densely chained if
// * chained and all configs in the chain are densely chained, or
// * unchained with a single densely chained config
let mut densely_chained = chained || configs.len() == 1;
let mut configs = configs.into_iter();
let mut nodes = Vec::new();

let Some(first) = configs.next() else {
return ProcessConfigsResult {
nodes: Vec::new(),
densely_chained,
};
let mut previous_result = self.process_configs(prev, true);
densely_chained = previous_result.densely_chained;
for current in config_iter {
let current_result = self.process_configs(current, true);
densely_chained = densely_chained && current_result.densely_chained;
match (
previous_result.densely_chained,
current_result.densely_chained,
) {
// Both groups are "densely" chained, so we can simplify the graph by only
// chaining the last in the previous list to the first in the current list
(true, true) => {
let last_in_prev = previous_result.nodes.last().unwrap();
let first_in_current = current_result.nodes.first().unwrap();
self.dependency.graph.add_edge(
*last_in_prev,
*first_in_current,
(),
);

if chained == Chain::YesIgnoreDeferred {
self.no_sync_edges
.insert((*last_in_prev, *first_in_current));
}
}
// The previous group is "densely" chained, so we can simplify the graph by only
// chaining the last item from the previous list to every item in the current list
(true, false) => {
let last_in_prev = previous_result.nodes.last().unwrap();
for current_node in &current_result.nodes {
self.dependency.graph.add_edge(
*last_in_prev,
*current_node,
(),
);

if chained == Chain::YesIgnoreDeferred {
self.no_sync_edges.insert((*last_in_prev, *current_node));
}
}
}
// The current list is currently "densely" chained, so we can simplify the graph by
// only chaining every item in the previous list to the first item in the current list
(false, true) => {
let first_in_current = current_result.nodes.first().unwrap();
for previous_node in &previous_result.nodes {
self.dependency.graph.add_edge(
*previous_node,
*first_in_current,
(),
);

if chained == Chain::YesIgnoreDeferred {
self.no_sync_edges
.insert((*previous_node, *first_in_current));
}
}
}
// Neither of the lists are "densely" chained, so we must chain every item in the first
// list to every item in the second list
(false, false) => {
for previous_node in &previous_result.nodes {
for current_node in &current_result.nodes {
self.dependency.graph.add_edge(
*previous_node,
*current_node,
(),
);

if chained == Chain::YesIgnoreDeferred {
self.no_sync_edges
.insert((*previous_node, *current_node));
}
}
};
let mut previous_result = self.process_configs(first, collect_nodes || chained);
densely_chained &= previous_result.densely_chained;

for current in configs {
let current_result = self.process_configs(current, collect_nodes || chained);
densely_chained &= current_result.densely_chained;

if chained {
// if the current result is densely chained, we only need to chain the first node
let current_nodes = if current_result.densely_chained {
&current_result.nodes[..1]
} else {
&current_result.nodes
};
// if the previous result was densely chained, we only need to chain the last node
let previous_nodes = if previous_result.densely_chained {
&previous_result.nodes[previous_result.nodes.len() - 1..]
} else {
&previous_result.nodes
};

for previous_node in previous_nodes {
for current_node in current_nodes {
self.dependency
.graph
.add_edge(*previous_node, *current_node, ());

if ignore_deferred {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if ignore_deferred {
if matches!(chained, Chain::YesIgnoreDeferred) {

ignore_deferred variable seems to only be used here.

self.no_sync_edges.insert((*previous_node, *current_node));
}
}
}

if collect_nodes {
nodes_in_scope.append(&mut previous_result.nodes);
}

previous_result = current_result;
}

// ensure the last config's nodes are added
if collect_nodes {
nodes_in_scope.append(&mut previous_result.nodes);
}
} else {
for config in config_iter {
let result = self.process_configs(config, collect_nodes);
densely_chained = densely_chained && result.densely_chained;
if collect_nodes {
nodes_in_scope.extend(result.nodes);
}
nodes.append(&mut previous_result.nodes);
}

// an "unchained" SystemConfig is only densely chained if it has exactly one densely chained entry
if more_than_one_entry {
densely_chained = false;
}
previous_result = current_result;
}
if collect_nodes {
nodes.append(&mut previous_result.nodes);
}

ProcessConfigsResult {
nodes: nodes_in_scope,
nodes,
densely_chained,
}
}
Expand Down