From 70d517e9f9f453ad6f780bf7935a7db09fa114a3 Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Sat, 24 Oct 2026 08:32:41 -0400 Subject: [PATCH 1/3] Share cached rule edges with hash output Hash traversal keeps each completed rule in its cache until every parent has visited it. Returning an owned digest cloned every dependency label at insertion and again when an already-visited rule was emitted. A generated file also discarded those copies to record its generator. Share nonempty edge lists while traversing the graph and release the cache before collecting output. The last owner can then move the vector into aggregation. Labels used by multiple configured instances still retain their original ordering and deduplication. Codex for Tamir. --- src/hash.rs | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/src/hash.rs b/src/hash.rs index a9b8f6c2..8c44d42b 100644 --- a/src/hash.rs +++ b/src/hash.rs @@ -60,7 +60,8 @@ pub struct HashOptions { struct TargetDigest { overall: DigestBytes, direct: DigestBytes, - deps: Vec, + // The traversal cache and the output share these labels until all parents have been hashed. + deps: Option>>, } struct TargetDigestBuilder { @@ -97,7 +98,7 @@ impl TargetDigestBuilder { TargetDigest { overall: self.overall.finalize().into(), direct, - deps: self.deps, + deps: (!self.deps.is_empty()).then(|| Rc::new(self.deps)), } } } @@ -376,11 +377,7 @@ fn hash_targets_with_environment( generating_rule, } => { let mut digest = context.rule_digest(&generating_rule, &mut Vec::new())?; - digest.deps = if options.track_deps { - vec![generating_rule] - } else { - Vec::new() - }; + digest.deps = options.track_deps.then(|| Rc::new(vec![generating_rule])); (name, "GeneratedFile", digest) } HashTarget::Source(label) => { @@ -399,13 +396,14 @@ fn hash_targets_with_environment( TargetDigest { overall: digest, direct: digest, - deps: Vec::new(), + deps: None, }, ) } }; digests.push((key, kind, digest)); } + drop(context); let (hashes, dep_edges) = aggregate_by_label(digests, options.track_deps); Ok(HashFileData { hashes, @@ -475,7 +473,7 @@ fn aggregate_by_label( } fn label_hash( - mut instances: Vec<(String, &'static str, TargetDigest)>, + instances: Vec<(String, &'static str, TargetDigest)>, ) -> (String, TargetHash, Vec) { let label = target_label(&instances[0].0).to_owned(); let kind = instances[0].1; @@ -492,8 +490,9 @@ fn label_hash( }; let mut seen = HashSet::new(); let deps = instances - .iter_mut() - .flat_map(|(_, _, digest)| digest.deps.drain(..)) + .into_iter() + .filter_map(|(_, _, digest)| digest.deps) + .flat_map(Rc::unwrap_or_clone) .map(|mut dependency| { let length = target_label(&dependency).len(); dependency.truncate(length); @@ -586,7 +585,7 @@ impl HashContext<'_> { return Ok(TargetDigest { overall: placeholder, direct: placeholder, - deps: Vec::new(), + deps: None, }); } path.push(canonical_key.clone()); @@ -2227,13 +2226,13 @@ esac tracked.direct(b"direct"); tracked.transitive("//dep:a", b"dependency"); let tracked = tracked.finish(); - assert_eq!(tracked.deps, ["//dep:a"]); + assert_eq!(tracked.deps.unwrap().as_slice(), ["//dep:a"]); assert_ne!(tracked.direct, tracked.overall); let mut untracked = TargetDigestBuilder::new(false); untracked.direct(b"direct"); untracked.transitive("//dep:a", b"dependency"); - assert!(untracked.finish().deps.is_empty()); + assert!(untracked.finish().deps.is_none()); assert_eq!(crate::model::Digest::from([0; 32]).to_string().len(), 64); } From b0c2f227f32847d68b7d1cd235cbad22156d0b2b Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Sat, 24 Oct 2026 08:51:33 -0400 Subject: [PATCH 2/3] Reuse sorted edge lists when grouping labels Rule inputs normally reach the output as one sorted list. Moving that list preserves its allocation. Its strict label ordering also proves the labels are unique, avoiding a hash set and an extra string copy for every dependency. Keep the first-seen order when several instances or rewritten configured labels produce an unordered or repeated edge. Codex for Tamir. --- src/hash.rs | 52 ++++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 42 insertions(+), 10 deletions(-) diff --git a/src/hash.rs b/src/hash.rs index 8c44d42b..c81f68c9 100644 --- a/src/hash.rs +++ b/src/hash.rs @@ -488,18 +488,22 @@ fn label_hash( } (overall.finalize().into(), direct.finalize().into()) }; - let mut seen = HashSet::new(); - let deps = instances + let mut edge_lists = instances .into_iter() .filter_map(|(_, _, digest)| digest.deps) - .flat_map(Rc::unwrap_or_clone) - .map(|mut dependency| { - let length = target_label(&dependency).len(); - dependency.truncate(length); - dependency - }) - .filter(|dependency| seen.insert(dependency.clone())) - .collect(); + .map(Rc::unwrap_or_clone); + let mut deps = edge_lists.next().unwrap_or_default(); + for mut edges in edge_lists { + deps.append(&mut edges); + } + for dependency in &mut deps { + dependency.truncate(target_label(dependency).len()); + } + // Plain-query rule inputs are normally already unique and in label order. + if !deps.is_sorted_by(|left, right| left < right) { + let mut seen = HashSet::new(); + deps.retain(|dependency| seen.insert(dependency.clone())); + } ( label, TargetHash { @@ -2338,6 +2342,34 @@ esac } #[cfg(unix)] + #[test] + fn generated_alias_keeps_the_shared_rules_dependencies() { + let workspace = tempfile::tempdir().unwrap(); + let rule = |name: &str, inputs: &[&str]| { + rule_target(Rule { + name: name.into(), + rule_class: "genrule".into(), + rule_input: inputs.iter().map(|input| (*input).to_owned()).collect(), + ..Default::default() + }) + }; + // Raw targets can list a rule under a generated file's label. Both rule outputs + // then read the generating rule's cached digest, including its edge list. + let data = hash_targets_for_test( + &hash_options(workspace.path()), + vec![ + rule("//:a", &["//:dep"]), + rule("//:dep", &[]), + rule("//:b", &[]), + generated_target("//:b", "//:a"), + ], + workspace.path(), + ) + .unwrap(); + assert_eq!(data.dep_edges["//:a"], ["//:dep"]); + assert_eq!(data.dep_edges["//:b"], ["//:dep", "//:a"]); + } + #[test] fn hashes_rules_sources_generated_files_and_visibility_groups() { let workspace = tempfile::tempdir().unwrap(); From 5116704e4e6629a8ac4900f2634340fcff52a241 Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Sat, 24 Oct 2026 13:22:30 -0400 Subject: [PATCH 3/3] Keep the Unix guard on the filesystem hashing test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The generated-alias regression constructs its graph in memory and can run on Windows. Inserting it above the filesystem hashing test moved that test’s existing Unix guard onto the alias case. --- src/hash.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hash.rs b/src/hash.rs index c81f68c9..be187e05 100644 --- a/src/hash.rs +++ b/src/hash.rs @@ -2341,7 +2341,6 @@ esac assert_ne!(included.finalize().to_vec(), ignored.finalize().to_vec()); } - #[cfg(unix)] #[test] fn generated_alias_keeps_the_shared_rules_dependencies() { let workspace = tempfile::tempdir().unwrap(); @@ -2370,6 +2369,7 @@ esac assert_eq!(data.dep_edges["//:b"], ["//:dep", "//:a"]); } + #[cfg(unix)] #[test] fn hashes_rules_sources_generated_files_and_visibility_groups() { let workspace = tempfile::tempdir().unwrap();