Skip to content

Commit ec7e88f

Browse files
authored
Remove extra lookups and memory allocations from tsort graph construction (#8694)
1 parent 33e37c6 commit ec7e88f

File tree

1 file changed

+20
-19
lines changed

1 file changed

+20
-19
lines changed

src/uu/tsort/src/tsort.rs

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -63,11 +63,23 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
6363

6464
// Create the directed graph from pairs of tokens in the input data.
6565
let mut g = Graph::new(input.to_string_lossy().to_string());
66-
for ab in data.split_whitespace().collect::<Vec<&str>>().chunks(2) {
67-
match ab {
68-
[a, b] => g.add_edge(a, b),
69-
_ => return Err(TsortError::NumTokensOdd(input.to_string_lossy().to_string()).into()),
70-
}
66+
// Input is considered to be in the format
67+
// From1 To1 From2 To2 ...
68+
// with tokens separated by whitespaces
69+
let mut edge_tokens = data.split_whitespace();
70+
// Note: this is equivalent to iterating over edge_tokens.chunks(2)
71+
// but chunks() exists only for slices and would require unnecessary Vec allocation.
72+
// Itertools::chunks() is not used due to unnecessary overhead for internal RefCells
73+
loop {
74+
// Try take next pair of tokens
75+
let Some(from) = edge_tokens.next() else {
76+
// no more tokens -> end of input. Graph constructed
77+
break;
78+
};
79+
let Some(to) = edge_tokens.next() else {
80+
return Err(TsortError::NumTokensOdd(input.to_string_lossy().to_string()).into());
81+
};
82+
g.add_edge(from, to);
7183
}
7284

7385
g.run_tsort();
@@ -127,19 +139,11 @@ impl<'input> Graph<'input> {
127139
}
128140
}
129141

130-
fn add_node(&mut self, name: &'input str) {
131-
self.nodes.entry(name).or_default();
132-
}
133-
134142
fn add_edge(&mut self, from: &'input str, to: &'input str) {
135-
self.add_node(from);
143+
let from_node = self.nodes.entry(from).or_default();
136144
if from != to {
137-
self.add_node(to);
138-
139-
let from_node = self.nodes.get_mut(from).unwrap();
140145
from_node.add_successor(to);
141-
142-
let to_node = self.nodes.get_mut(to).unwrap();
146+
let to_node = self.nodes.entry(to).or_default();
143147
to_node.predecessor_count += 1;
144148
}
145149
}
@@ -233,10 +237,7 @@ impl<'input> Graph<'input> {
233237

234238
fn detect_cycle(&self) -> Vec<&'input str> {
235239
// Sort the nodes just to make this function deterministic.
236-
let mut nodes = Vec::new();
237-
for node in self.nodes.keys() {
238-
nodes.push(node);
239-
}
240+
let mut nodes: Vec<_> = self.nodes.keys().collect();
240241
nodes.sort_unstable();
241242

242243
let mut visited = HashSet::new();

0 commit comments

Comments
 (0)