Skip to content

Commit

Permalink
dag: make set intersection and difference use fast paths more aggress…
Browse files Browse the repository at this point in the history
…ively

Summary:
In `xs - ys` and `xs & ys` cases, the order of `ys` does not matter. Let's just
try aggressively flatten them. This should not affect correctness.

Reviewed By: zzl0

Differential Revision: D58442045

fbshipit-source-id: af5fcb82221173a6e0a6dce57564a735b667e8d2
  • Loading branch information
quark-zju authored and facebook-github-bot committed Jun 13, 2024
1 parent 84576dc commit 58a1c65
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 10 deletions.
17 changes: 12 additions & 5 deletions eden/scm/lib/dag/src/nameset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,8 @@ impl NameSet {
}
if let (Some(this), Some(other)) = (
self.as_any().downcast_ref::<IdStaticSet>(),
other.as_any().downcast_ref::<IdStaticSet>(),
// xs - ys; the order of ys does not matter
other.specialized_flatten_id(),
) {
let order = this.map.map_version().partial_cmp(other.map.map_version());
if order.is_some() {
Expand Down Expand Up @@ -277,7 +278,8 @@ impl NameSet {
}
if let (Some(this), Some(other)) = (
self.as_any().downcast_ref::<IdStaticSet>(),
other.as_any().downcast_ref::<IdStaticSet>(),
// xs & ys; the order of ys does not matter
other.specialized_flatten_id(),
) {
let order = this.map.map_version().partial_cmp(other.map.map_version());
if let Some(order) = order {
Expand Down Expand Up @@ -339,18 +341,23 @@ impl NameSet {
if let Some(set) = self.union_fast_paths(other) {
return set;
}

// This fast path aggressively flatten the sets. It does not preserve order.
if let (Some(this), Some(other)) = (
self.as_any().downcast_ref::<IdStaticSet>(),
other.as_any().downcast_ref::<IdStaticSet>(),
self.specialized_flatten_id(),
other.specialized_flatten_id(),
) {
let order = this.map.map_version().partial_cmp(other.map.map_version());
if let Some(order) = order {
// Fast path for IdStaticSet
let result = Self::from_spans_idmap_dag(
let mut result = Self::from_spans_idmap_dag(
this.spans.union(&other.spans),
pick(order, &this.map, &other.map).clone(),
pick(order, &this.dag, &other.dag).clone(),
);
if this.is_reversed() {
result = result.reverse();
}
tracing::debug!(
target: "dag::algo::union",
"union(x={:.6?}, y={:.6?}) = {:.6?} (fast path 3)",
Expand Down
29 changes: 25 additions & 4 deletions eden/scm/lib/dag/src/nameset/id_static.rs
Original file line number Diff line number Diff line change
Expand Up @@ -457,18 +457,34 @@ pub(crate) mod tests {
let abcd = r(dag.ancestors("D".into()))?.reverse();
let unordered = abcd.take(2).union_zip(&abcd.skip(3));

// Intersection and difference can flatten the "unordered" set because rhs order does
// not matter.
assert_eq!(
format!("{:?}", abcd.intersection(&unordered)),
"<and <spans [A:D+0:3] +> <or <spans [A:B+0:1] +> <spans [D+3] +> (order=Zip)>>"
"<spans [D+3, A:B+0:1] +>"
);
assert_eq!(
format!("{:?}", abcd.difference(&unordered)),
"<diff <spans [A:D+0:3] +> <or <spans [A:B+0:1] +> <spans [D+3] +> (order=Zip)>>"
"<spans [C+2] +>"
);

// but lhs order matters.
assert_eq!(
format!("{:?}", unordered.intersection(&abcd)),
"<and <or <spans [A:B+0:1] +> <spans [D+3] +> (order=Zip)> <spans [A:D+0:3] +>>"
);
assert_eq!(
format!("{:?}", unordered.difference(&abcd)),
"<diff <or <spans [A:B+0:1] +> <spans [D+3] +> (order=Zip)> <spans [A:D+0:3] +>>"
);

// Union drops order (by flattening) aggresively on both sides.
assert_eq!(
format!("{:?}", abcd.union(&unordered)),
"<or <spans [A:D+0:3] +> <or <spans [A:B+0:1] +> <spans [D+3] +> (order=Zip)>>"
"<spans [A:D+0:3] +>"
);

// Union (preserving order) cannot flatten sets for fast paths.
assert_eq!(
format!("{:?}", abcd.union_preserving_order(&unordered)),
"<or <spans [A:D+0:3] +> <or <spans [A:B+0:1] +> <spans [D+3] +> (order=Zip)>>"
Expand Down Expand Up @@ -621,7 +637,12 @@ pub(crate) mod tests {
"[B, C]"
);
assert_eq!(dbg_flat(&slice12(&abcd.union_zip(&unordered))), "[B, C]");
assert_eq!(dbg_flat(&slice12(&abcd.union(&unordered))), "[B, C]");

// "union" does not promise order and might have a fast path.
assert_eq!(
dbg_flat(&slice12(&abcd.union(&unordered))),
"[B, C] flat:[B, C]"
);

Ok(())
})
Expand Down
3 changes: 2 additions & 1 deletion eden/scm/lib/dag/src/nameset/intersection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,8 @@ impl StopCondition {

impl IntersectionSet {
pub fn new(lhs: NameSet, rhs: NameSet) -> Self {
// More efficient if `lhs` is smaller. Swap `lhs` and `rhs` if `lhs` is `FULL`.
// More efficient if `lhs` is smaller. But `lhs` order matters.
// Swap `lhs` and `rhs` if `lhs` is `FULL`.
let (lhs, rhs) = if lhs.hints().contains(Flags::FULL)
&& !rhs.hints().contains(Flags::FULL)
&& !rhs.hints().contains(Flags::FILTER)
Expand Down

0 comments on commit 58a1c65

Please sign in to comment.