Skip to content

Commit 075b0c4

Browse files
quark-zjumeta-codesync[bot]
authored andcommitted
linelog: cache calculate_dep_map result in linelog field
Summary: I'd like to use the dependency map in more places. For example, to check if remap_revs (re-order) is valid or not. If reorders get frequently tested, it seems nice to cache the dependency map result. Reviewed By: muirdm Differential Revision: D103701815 fbshipit-source-id: eed78ca8fffb4fc78e18b940c740dee8480b37b7
1 parent 2073672 commit 075b0c4

3 files changed

Lines changed: 27 additions & 9 deletions

File tree

eden/scm/lib/linelog/src/linelog.rs

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use std::collections::HashSet;
99
use std::collections::VecDeque;
1010
use std::fmt;
1111
use std::sync::Arc;
12+
use std::sync::OnceLock;
1213
use std::sync::atomic::AtomicUsize;
1314
use std::sync::atomic::Ordering;
1415

@@ -25,6 +26,7 @@ pub struct AbstractLineLog<T> {
2526
pub(crate) dag: NanoDag,
2627

2728
a_lines_cache: Option<(Rev, ImVec<LineInfo<T>>)>,
29+
deps_map_cache: OnceLock<Arc<NanoDag>>,
2830
perf_stats: Option<Arc<PerfStats>>,
2931
}
3032

@@ -46,6 +48,7 @@ impl<T> Clone for AbstractLineLog<T> {
4648
max_rev: self.max_rev,
4749
dag: self.dag.clone(),
4850
a_lines_cache: self.a_lines_cache.clone(),
51+
deps_map_cache: self.deps_map_cache.clone(),
4952
perf_stats: self.perf_stats.clone(),
5053
}
5154
}
@@ -112,6 +115,7 @@ impl<T> Default for AbstractLineLog<T> {
112115
max_rev: 0,
113116
dag: Default::default(),
114117
a_lines_cache: None,
118+
deps_map_cache: OnceLock::new(),
115119
perf_stats: None,
116120
}
117121
}
@@ -225,6 +229,7 @@ impl<T: Default + PartialEq + fmt::Debug> AbstractLineLog<T> {
225229
dag: new_dag,
226230
max_rev: self.max_rev.max(b_rev),
227231
a_lines_cache,
232+
deps_map_cache: Default::default(),
228233
..self
229234
}
230235
}
@@ -586,6 +591,7 @@ impl<T: Default + PartialEq + fmt::Debug> AbstractLineLog<T> {
586591
code,
587592
max_rev: self.max_rev.max(b_rev),
588593
a_lines_cache: None,
594+
deps_map_cache: Default::default(),
589595
..self
590596
}
591597
}
@@ -662,7 +668,7 @@ impl<T> AbstractLineLog<T> {
662668
/// the dag will resize to make sure all revs are present in the dag.
663669
///
664670
/// Note: There are no checks about whether the reordering is meaningful.
665-
/// The callsite should use `calculate_dep_map` to check dependencies
671+
/// The callsite should use `can_reorder` to check dependencies
666672
/// and avoid troublesome reorders, like moving a change to before its
667673
/// dependency.
668674
pub fn remap_revs(self, rev_map: &dyn Fn(Rev) -> Rev) -> Self {
@@ -689,6 +695,7 @@ impl<T> AbstractLineLog<T> {
689695
max_rev,
690696
dag: self.dag.with_edge(max_rev, max_rev),
691697
a_lines_cache: None,
698+
deps_map_cache: Default::default(),
692699
..self
693700
}
694701
}
@@ -716,6 +723,7 @@ impl<T> AbstractLineLog<T> {
716723
code,
717724
max_rev,
718725
a_lines_cache: None,
726+
deps_map_cache: Default::default(),
719727
..self
720728
}
721729
}
@@ -724,6 +732,16 @@ impl<T> AbstractLineLog<T> {
724732
pub fn nanodag(&self) -> &NanoDag {
725733
&self.dag
726734
}
735+
736+
/// Get the dependency dag. If `rev1` has parent `rev2`, then `rev1`
737+
/// textually depend on `rev2` and cannot be moved to an ancestor of `rev2`.
738+
///
739+
/// Only includes revs explicitly appear in linelog instructions, i.e. revs
740+
/// that actually edit the lines.
741+
pub fn dep_map(&self) -> &Arc<NanoDag> {
742+
self.deps_map_cache
743+
.get_or_init(|| Arc::new(self.calculate_dep_map()))
744+
}
727745
}
728746

729747
impl<T: AsRef<str> + Default + PartialEq + fmt::Debug> AbstractLineLog<T> {

eden/scm/lib/linelog/src/stacks.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ impl<T> AbstractLineLog<T> {
127127
/// deletions) might be skipped incorrectly after `remap_revs`.
128128
/// Practically, LineLog might allow reorder cases that would be disallowed
129129
/// by traditional context-line dependencies.
130-
pub fn calculate_dep_map(&self) -> NanoDag {
130+
pub(crate) fn calculate_dep_map(&self) -> NanoDag {
131131
// With the insertion and deletion stacks (see explanation in
132132
// visit_with_ins_del_stacks), when we see a new insertion block, or deletion
133133
// block, we add two dependencies:

eden/scm/lib/linelog/src/tests.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -366,7 +366,7 @@ fn test_remap_revs() {
366366
fn test_remap_revs_reorder_insertions() {
367367
let log = log_from_texts(&["a\n".into(), "a\nb\n".into(), "a\nb\nc\n".into()]);
368368

369-
let dep_map = log.calculate_dep_map();
369+
let dep_map = log.dep_map();
370370
for rev in 1..=3 {
371371
assert_eq!(dep_map.parents(rev), Some(&[0][..]), "rev={rev}");
372372
}
@@ -438,7 +438,7 @@ fn test_reorder_insertions(lines: &[&str], line_added_order: &[usize]) {
438438
let log = log_from_texts(&texts);
439439

440440
// Verify dep map.
441-
let deps = log.calculate_dep_map();
441+
let deps = log.dep_map();
442442
assert!(
443443
deps.iter().all(|(rev, deps)| rev == 0 || deps == &[0]),
444444
"order={line_added_order:?}"
@@ -578,14 +578,14 @@ fn test_flatten() {
578578
}
579579

580580
#[test]
581-
fn test_calculate_dep_map() {
582-
let deps = |text_list: &[&str]| -> NanoDag {
581+
fn test_dep_map() {
582+
let deps = |text_list: &[&str]| -> Arc<NanoDag> {
583583
let texts: Vec<String> = text_list
584584
.iter()
585585
.map(|t| t.chars().map(|c| format!("{c}\n")).collect::<String>())
586586
.collect();
587587
let log = log_from_texts(&texts);
588-
log.calculate_dep_map()
588+
log.dep_map().clone()
589589
};
590590

591591
assert_eq!(deps(&[]).to_string(), "");
@@ -785,7 +785,7 @@ fn test_block_shift_effectiveness() {
785785
let lines = expected_rev3_lines[a1..a1 + n].to_vec();
786786
let log = base.clone().edit_chunk(2, a1, a1, 3, lines, flags);
787787
assert_eq!(log.checkout_text(3), expected_rev3_text);
788-
let dep = log.calculate_dep_map();
788+
let dep = log.dep_map();
789789
let dep = format!("DepMap({})", dep);
790790
grouped.entry(dep).or_default().push(a1);
791791
}
@@ -831,7 +831,7 @@ fn test_block_shift_overflow() {
831831
let log = base
832832
.clone()
833833
.edit_chunk(1, a1, a1, 1, vec![""], EditFlags::default());
834-
let dep = log.calculate_dep_map();
834+
let dep = log.dep_map();
835835
assert!(dep.iter().all(|(rev, deps)| rev == 0 || deps == &[0]))
836836
}
837837
}

0 commit comments

Comments
 (0)