From ec7e88ff5b6ff07fa52633d94d826bbc7b38aa02 Mon Sep 17 00:00:00 2001 From: Dmitry Sviridkin Date: Sun, 28 Sep 2025 21:46:55 +0100 Subject: [PATCH] Remove extra lookups and memory allocations from tsort graph construction (#8694) --- src/uu/tsort/src/tsort.rs | 39 ++++++++++++++++++++------------------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/src/uu/tsort/src/tsort.rs b/src/uu/tsort/src/tsort.rs index 748e8e88d..85380bf40 100644 --- a/src/uu/tsort/src/tsort.rs +++ b/src/uu/tsort/src/tsort.rs @@ -63,11 +63,23 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { // Create the directed graph from pairs of tokens in the input data. let mut g = Graph::new(input.to_string_lossy().to_string()); - for ab in data.split_whitespace().collect::>().chunks(2) { - match ab { - [a, b] => g.add_edge(a, b), - _ => return Err(TsortError::NumTokensOdd(input.to_string_lossy().to_string()).into()), - } + // Input is considered to be in the format + // From1 To1 From2 To2 ... + // with tokens separated by whitespaces + let mut edge_tokens = data.split_whitespace(); + // Note: this is equivalent to iterating over edge_tokens.chunks(2) + // but chunks() exists only for slices and would require unnecessary Vec allocation. + // Itertools::chunks() is not used due to unnecessary overhead for internal RefCells + loop { + // Try take next pair of tokens + let Some(from) = edge_tokens.next() else { + // no more tokens -> end of input. Graph constructed + break; + }; + let Some(to) = edge_tokens.next() else { + return Err(TsortError::NumTokensOdd(input.to_string_lossy().to_string()).into()); + }; + g.add_edge(from, to); } g.run_tsort(); @@ -127,19 +139,11 @@ impl<'input> Graph<'input> { } } - fn add_node(&mut self, name: &'input str) { - self.nodes.entry(name).or_default(); - } - fn add_edge(&mut self, from: &'input str, to: &'input str) { - self.add_node(from); + let from_node = self.nodes.entry(from).or_default(); if from != to { - self.add_node(to); - - let from_node = self.nodes.get_mut(from).unwrap(); from_node.add_successor(to); - - let to_node = self.nodes.get_mut(to).unwrap(); + let to_node = self.nodes.entry(to).or_default(); to_node.predecessor_count += 1; } } @@ -233,10 +237,7 @@ impl<'input> Graph<'input> { fn detect_cycle(&self) -> Vec<&'input str> { // Sort the nodes just to make this function deterministic. - let mut nodes = Vec::new(); - for node in self.nodes.keys() { - nodes.push(node); - } + let mut nodes: Vec<_> = self.nodes.keys().collect(); nodes.sort_unstable(); let mut visited = HashSet::new();