Skip to content

Commit 117eb0a

Browse files
Aaron Hometa-codesync[bot]
authored andcommitted
Batch target removal to eliminate repeated rdeps scans
Summary: When btd-v2 removes targets during diff updates, each removed target individually scans its deps' rdeps lists to clean up references. This changes the approach to collect all targets to remove first, then deduplicate their deps and do a single parallel retain pass per dep. Before: (Timeout) https://www.internalfb.com/sandcastle/workflow/1166432303503000741 After: (3min 12s) https://www.internalfb.com/sandcastle/workflow/1868993845372898255 Before: (Timeout) https://www.internalfb.com/sandcastle/workflow/2445454597676228967 After: (2min 37s), user failure intentional https://www.internalfb.com/sandcastle/workflow/3107483742899743095 Before: (Timeout) https://www.internalfb.com/sandcastle/workflow/1166432303503029126 After: (2min 1s) https://www.internalfb.com/sandcastle/workflow/666532744864971972 Reviewed By: RiskRunner0 Differential Revision: D94911981 fbshipit-source-id: 22bcefdba92b66523758f85b13e31dcac0267c1e
1 parent 7c48337 commit 117eb0a

File tree

2 files changed

+166
-4
lines changed

2 files changed

+166
-4
lines changed

td_util/src/buck/Cargo.toml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,22 +8,22 @@ path = "lib.rs"
88

99
[dependencies]
1010
anyhow = "1.0"
11+
audit = { path = "../../../audit" }
1112
dashmap = { version = "5.5", features = ["serde"] }
13+
fbinit = { workspace = true }
1214
fnv = "1.0"
1315
glob = "0.3.0"
1416
globset = "0.4.10"
1517
itertools = "0.14.0"
1618
parse-display = "0.8.2"
19+
rayon = "1.6.1"
1720
regex = "1.5.4"
1821
serde = { version = "1.0", features = ["derive"] }
1922
serde_json = "1.0.66"
23+
td_util = { path = "../.." }
2024
tempfile = "3.1.0"
2125
thiserror = "1.0.36"
2226
tracing = "0.1.22"
23-
fbinit = { workspace = true }
24-
25-
td_util = { path = "../.." }
26-
audit = { path = "../../../audit" }
2727

2828
[dev-dependencies]
2929
rstest = "0.18"

td_util/src/buck/target_graph.rs

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,14 @@
88
* above-listed licenses.
99
*/
1010

11+
use std::collections::HashSet;
1112
use std::hash::Hasher;
1213
use std::str::FromStr;
1314

1415
use dashmap::DashMap;
1516
use dashmap::DashSet;
1617
use fnv::FnvHasher;
18+
use rayon::prelude::*;
1719
use serde::Deserialize;
1820
use serde::Serialize;
1921

@@ -455,6 +457,71 @@ impl TargetGraph {
455457
// to removed targets are preserved for detection.
456458
}
457459

460+
pub fn remove_targets_batch(&self, targets_to_remove: &HashSet<TargetId>) {
461+
if targets_to_remove.is_empty() {
462+
return;
463+
}
464+
465+
self.clean_ci_hint_edges_for_removed_targets(targets_to_remove);
466+
self.clean_rdeps_for_removed_targets(targets_to_remove);
467+
self.clean_per_target_data(targets_to_remove);
468+
}
469+
470+
fn clean_ci_hint_edges_for_removed_targets(&self, targets_to_remove: &HashSet<TargetId>) {
471+
for &target_id in targets_to_remove {
472+
if !self.is_ci_hint_target(target_id) {
473+
continue;
474+
}
475+
476+
for affected_target in self.get_ci_hint_affected(target_id).unwrap_or_default() {
477+
let Some(mut ci_hints) = self.affected_to_ci_hints.get_mut(&affected_target) else {
478+
continue;
479+
};
480+
ci_hints.retain(|id| !targets_to_remove.contains(id));
481+
if ci_hints.is_empty() {
482+
drop(ci_hints);
483+
self.affected_to_ci_hints.remove(&affected_target);
484+
}
485+
}
486+
487+
self.ci_hint_to_affected.remove(&target_id);
488+
}
489+
}
490+
491+
fn clean_rdeps_for_removed_targets(&self, targets_to_remove: &HashSet<TargetId>) {
492+
let unique_deps: Vec<TargetId> = targets_to_remove
493+
.iter()
494+
.flat_map(|&tid| self.get_deps(tid).unwrap_or_default())
495+
.collect::<HashSet<_>>()
496+
.into_iter()
497+
.collect();
498+
499+
unique_deps.par_iter().for_each(|&dep_id| {
500+
let Some(mut rdeps) = self.target_id_to_rdeps.get_mut(&dep_id) else {
501+
return;
502+
};
503+
rdeps.retain(|id| !targets_to_remove.contains(id));
504+
if rdeps.is_empty() {
505+
drop(rdeps);
506+
self.target_id_to_rdeps.remove(&dep_id);
507+
}
508+
});
509+
}
510+
511+
fn clean_per_target_data(&self, targets_to_remove: &HashSet<TargetId>) {
512+
for &target_id in targets_to_remove {
513+
self.target_id_to_deps.remove(&target_id);
514+
self.target_id_to_ci_srcs.remove(&target_id);
515+
self.target_id_to_ci_srcs_must_match.remove(&target_id);
516+
self.target_id_to_ci_deps_package_patterns
517+
.remove(&target_id);
518+
self.target_id_to_ci_deps_recursive_patterns
519+
.remove(&target_id);
520+
self.targets_with_sudo_label.remove(&target_id);
521+
self.minimized_targets.remove(&target_id);
522+
}
523+
}
524+
458525
pub fn get_all_targets(&self) -> impl Iterator<Item = TargetId> + '_ {
459526
self.target_id_to_label.iter().map(|entry| *entry.key())
460527
}
@@ -1452,4 +1519,99 @@ mod tests {
14521519
}
14531520
}
14541521
}
1522+
1523+
fn store_minimized_stub(graph: &TargetGraph, target_id: TargetId) {
1524+
let rule_type_id = graph.store_rule_type("cpp_library");
1525+
graph.store_minimized_target(
1526+
target_id,
1527+
MinimizedBuckTarget {
1528+
rule_type: rule_type_id,
1529+
oncall: None,
1530+
labels: vec![],
1531+
target_hash: TargetHash::new("hash"),
1532+
},
1533+
);
1534+
}
1535+
1536+
#[test]
1537+
fn batch_remove_cleans_rdeps_same_as_individual_removal() {
1538+
let graph = TargetGraph::new();
1539+
1540+
let shared_dep = graph.store_target("fbcode//lib:shared");
1541+
let target_a = graph.store_target("fbcode//a:target_a");
1542+
let target_b = graph.store_target("fbcode//b:target_b");
1543+
let survivor = graph.store_target("fbcode//c:survivor");
1544+
1545+
graph.add_rdep(shared_dep, target_a);
1546+
graph.add_rdep(shared_dep, target_b);
1547+
graph.add_rdep(shared_dep, survivor);
1548+
1549+
for &id in &[target_a, target_b, shared_dep, survivor] {
1550+
store_minimized_stub(&graph, id);
1551+
}
1552+
1553+
let to_remove: HashSet<TargetId> = [target_a, target_b].into_iter().collect();
1554+
graph.remove_targets_batch(&to_remove);
1555+
1556+
assert_eq!(graph.get_rdeps(shared_dep).unwrap(), vec![survivor]);
1557+
assert!(graph.get_minimized_target(target_a).is_none());
1558+
assert!(graph.get_minimized_target(target_b).is_none());
1559+
assert!(graph.get_deps(target_a).is_none());
1560+
assert!(graph.get_deps(target_b).is_none());
1561+
assert!(graph.get_minimized_target(shared_dep).is_some());
1562+
assert!(graph.get_minimized_target(survivor).is_some());
1563+
}
1564+
1565+
#[test]
1566+
fn batch_remove_handles_ci_hint_targets() {
1567+
let graph = TargetGraph::new();
1568+
1569+
let ci_hint_id =
1570+
store_target_with_rule_type(&graph, "fbcode//foo:ci_hint@test", CI_HINT_RULE_TYPE);
1571+
let dest_id = store_target_with_rule_type(&graph, "fbcode//foo:test", "python_test");
1572+
graph.add_ci_hint_edge(ci_hint_id, dest_id);
1573+
1574+
let to_remove: HashSet<TargetId> = [ci_hint_id].into_iter().collect();
1575+
graph.remove_targets_batch(&to_remove);
1576+
1577+
assert!(graph.get_ci_hint_affected(ci_hint_id).is_none());
1578+
assert!(graph.get_affecting_ci_hints(dest_id).is_none());
1579+
assert!(graph.get_minimized_target(ci_hint_id).is_none());
1580+
}
1581+
1582+
#[test]
1583+
fn batch_remove_cleans_dep_rdeps_for_targets_with_different_deps() {
1584+
let graph = TargetGraph::new();
1585+
1586+
let dep_x = graph.store_target("fbcode//lib:dep_x");
1587+
let dep_y = graph.store_target("fbcode//lib:dep_y");
1588+
let target_a = graph.store_target("fbcode//a:target_a");
1589+
let target_b = graph.store_target("fbcode//b:target_b");
1590+
1591+
graph.add_rdep(dep_x, target_a);
1592+
graph.add_rdep(dep_y, target_b);
1593+
1594+
for &id in &[dep_x, dep_y, target_a, target_b] {
1595+
store_minimized_stub(&graph, id);
1596+
}
1597+
1598+
let to_remove: HashSet<TargetId> = [target_a, target_b].into_iter().collect();
1599+
graph.remove_targets_batch(&to_remove);
1600+
1601+
assert!(graph.get_rdeps(dep_x).is_none());
1602+
assert!(graph.get_rdeps(dep_y).is_none());
1603+
}
1604+
1605+
#[test]
1606+
fn batch_remove_empty_set_is_noop() {
1607+
let graph = TargetGraph::new();
1608+
1609+
let target = graph.store_target("fbcode//a:target");
1610+
store_minimized_stub(&graph, target);
1611+
1612+
let to_remove: HashSet<TargetId> = HashSet::new();
1613+
graph.remove_targets_batch(&to_remove);
1614+
1615+
assert!(graph.get_minimized_target(target).is_some());
1616+
}
14551617
}

0 commit comments

Comments
 (0)