From fb77c55268dcd2e45dde45697963a14f50a81fa0 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Fri, 26 May 2023 15:19:42 +0900 Subject: [PATCH] index: use as_composite() to access to index stats The idea is that .as_composite() is equivalent to .as_index(), but for the implementation type. I'm going to add "impl Index for CompositeIndex" to clean up index references passed to revset engine. --- lib/src/default_index_store.rs | 40 +++++++++---------------- lib/tests/test_index.rs | 53 +++++++++++++++------------------- src/commands/mod.rs | 4 +-- 3 files changed, 39 insertions(+), 58 deletions(-) diff --git a/lib/src/default_index_store.rs b/lib/src/default_index_store.rs index 07b19dfeb..09141ce0e 100644 --- a/lib/src/default_index_store.rs +++ b/lib/src/default_index_store.rs @@ -450,8 +450,8 @@ impl Debug for ReadonlyIndexImpl { } impl ReadonlyIndexWrapper { - pub fn stats(&self) -> IndexStats { - self.0.stats() + pub fn as_composite(&self) -> CompositeIndex { + self.0.as_composite() } } @@ -685,14 +685,6 @@ impl MutableIndexImpl { IndexLoadError::IoError(err) => err, }) } - - pub fn stats(&self) -> IndexStats { - CompositeIndex(self).stats() - } - - pub fn num_commits(&self) -> u32 { - CompositeIndex(self).num_commits() - } } impl Index for MutableIndexImpl { @@ -779,7 +771,9 @@ impl MutableIndex for MutableIndexImpl { if own_ancestor.name == other_ancestor.name { break; } - if own_ancestor.num_commits() < other_ancestor.num_commits() { + if own_ancestor.as_composite().num_commits() + < other_ancestor.as_composite().num_commits() + { files_to_add.push(other_ancestor.clone()); maybe_other_ancestor = other_ancestor.parent_file.clone(); } else { @@ -842,11 +836,11 @@ impl<'a> CompositeIndex<'a> { ) } - fn num_commits(&self) -> u32 { + pub fn num_commits(&self) -> u32 { self.0.segment_num_parent_commits() + self.0.segment_num_commits() } - fn stats(&self) -> IndexStats { + pub fn stats(&self) -> IndexStats { let num_commits = self.num_commits(); let mut num_merges = 0; let mut max_generation_number = 0; @@ -908,7 +902,7 @@ impl<'a> CompositeIndex<'a> { /// /// If the given `commit_id` doesn't exist, this will return the prefix /// length that never matches with any commit ids. - fn shortest_unique_commit_id_prefix_len(&self, commit_id: &CommitId) -> usize { + pub fn shortest_unique_commit_id_prefix_len(&self, commit_id: &CommitId) -> usize { let (prev_id, next_id) = self.resolve_neighbor_commit_ids(commit_id); itertools::chain(prev_id, next_id) .map(|id| backend::common_hex_len(commit_id.as_bytes(), id.as_bytes()) + 1) @@ -942,7 +936,7 @@ impl<'a> CompositeIndex<'a> { .unwrap() } - fn resolve_prefix(&self, prefix: &HexPrefix) -> PrefixResolution { + pub fn resolve_prefix(&self, prefix: &HexPrefix) -> PrefixResolution { self.ancestor_index_segments() .fold(PrefixResolution::NoMatch, |acc_match, segment| { if acc_match == PrefixResolution::AmbiguousMatch { @@ -959,11 +953,11 @@ impl<'a> CompositeIndex<'a> { .map(|pos| self.entry_by_pos(pos)) } - fn has_id(&self, commit_id: &CommitId) -> bool { + pub fn has_id(&self, commit_id: &CommitId) -> bool { self.commit_id_to_pos(commit_id).is_some() } - fn is_ancestor(&self, ancestor_id: &CommitId, descendant_id: &CommitId) -> bool { + pub fn is_ancestor(&self, ancestor_id: &CommitId, descendant_id: &CommitId) -> bool { let ancestor_pos = self.commit_id_to_pos(ancestor_id).unwrap(); let descendant_pos = self.commit_id_to_pos(descendant_id).unwrap(); self.is_ancestor_pos(ancestor_pos, descendant_pos) @@ -989,7 +983,7 @@ impl<'a> CompositeIndex<'a> { false } - fn common_ancestors(&self, set1: &[CommitId], set2: &[CommitId]) -> Vec { + pub fn common_ancestors(&self, set1: &[CommitId], set2: &[CommitId]) -> Vec { let pos1 = set1 .iter() .map(|id| self.commit_id_to_pos(id).unwrap()) @@ -1113,7 +1107,7 @@ impl<'a> CompositeIndex<'a> { } /// Parents before children - fn topo_order(&self, input: &mut dyn Iterator) -> Vec { + pub fn topo_order(&self, input: &mut dyn Iterator) -> Vec { let mut ids = input.cloned().collect_vec(); ids.sort_by_cached_key(|id| self.commit_id_to_pos(id).unwrap()); ids @@ -1979,14 +1973,6 @@ impl ReadonlyIndexImpl { } } } - - pub fn stats(&self) -> IndexStats { - CompositeIndex(self).stats() - } - - pub fn num_commits(&self) -> u32 { - CompositeIndex(self).num_commits() - } } impl Index for ReadonlyIndexImpl { diff --git a/lib/tests/test_index.rs b/lib/tests/test_index.rs index 3ce375273..bc33ccc86 100644 --- a/lib/tests/test_index.rs +++ b/lib/tests/test_index.rs @@ -17,8 +17,7 @@ use std::sync::Arc; use jujutsu_lib::backend::CommitId; use jujutsu_lib::commit::Commit; use jujutsu_lib::commit_builder::CommitBuilder; -use jujutsu_lib::default_index_store::{MutableIndexImpl, ReadonlyIndexImpl}; -use jujutsu_lib::index::Index; +use jujutsu_lib::default_index_store::{CompositeIndex, MutableIndexImpl, ReadonlyIndexImpl}; use jujutsu_lib::repo::{MutableRepo, ReadonlyRepo, Repo}; use jujutsu_lib::settings::UserSettings; use test_case::test_case; @@ -35,12 +34,8 @@ fn child_commit<'repo>( } // Helper just to reduce line wrapping -fn generation_number(index: &ReadonlyIndexImpl, commit_id: &CommitId) -> u32 { - index - .as_composite() - .entry_by_id(commit_id) - .unwrap() - .generation_number() +fn generation_number(index: CompositeIndex, commit_id: &CommitId) -> u32 { + index.entry_by_id(commit_id).unwrap().generation_number() } #[test_case(false ; "local backend")] @@ -49,7 +44,7 @@ fn test_index_commits_empty_repo(use_git: bool) { let test_repo = TestRepo::init(use_git); let repo = &test_repo.repo; - let index = as_readonly_impl(repo); + let index = as_readonly_composite(repo); // There should be just the root commit assert_eq!(index.num_commits(), 1); @@ -91,7 +86,7 @@ fn test_index_commits_standard_cases(use_git: bool) { let commit_h = graph_builder.commit_with_parents(&[&commit_e]); let repo = tx.commit(); - let index = as_readonly_impl(&repo); + let index = as_readonly_composite(&repo); // There should be the root commit, plus 8 more assert_eq!(index.num_commits(), 1 + 8); @@ -151,7 +146,7 @@ fn test_index_commits_criss_cross(use_git: bool) { } let repo = tx.commit(); - let index = as_readonly_impl(&repo); + let index = as_readonly_composite(&repo); // There should the root commit, plus 2 for each generation assert_eq!(index.num_commits(), 1 + 2 * (num_generations as u32)); @@ -195,21 +190,18 @@ fn test_index_commits_criss_cross(use_git: bool) { // RevWalk deduplicates chains by entry. assert_eq!( index - .as_composite() .walk_revs(&[left_commits[num_generations - 1].id().clone()], &[]) .count(), 2 * num_generations ); assert_eq!( index - .as_composite() .walk_revs(&[right_commits[num_generations - 1].id().clone()], &[]) .count(), 2 * num_generations ); assert_eq!( index - .as_composite() .walk_revs( &[left_commits[num_generations - 1].id().clone()], &[left_commits[num_generations - 2].id().clone()] @@ -219,7 +211,6 @@ fn test_index_commits_criss_cross(use_git: bool) { ); assert_eq!( index - .as_composite() .walk_revs( &[right_commits[num_generations - 1].id().clone()], &[right_commits[num_generations - 2].id().clone()] @@ -232,7 +223,6 @@ fn test_index_commits_criss_cross(use_git: bool) { // be more expensive than RevWalk, but should still finish in reasonable time. assert_eq!( index - .as_composite() .walk_revs(&[left_commits[num_generations - 1].id().clone()], &[]) .filter_by_generation(0..(num_generations + 1) as u32) .count(), @@ -240,7 +230,6 @@ fn test_index_commits_criss_cross(use_git: bool) { ); assert_eq!( index - .as_composite() .walk_revs(&[right_commits[num_generations - 1].id().clone()], &[]) .filter_by_generation(0..(num_generations + 1) as u32) .count(), @@ -248,7 +237,6 @@ fn test_index_commits_criss_cross(use_git: bool) { ); assert_eq!( index - .as_composite() .walk_revs( &[left_commits[num_generations - 1].id().clone()], &[left_commits[num_generations - 2].id().clone()] @@ -259,7 +247,6 @@ fn test_index_commits_criss_cross(use_git: bool) { ); assert_eq!( index - .as_composite() .walk_revs( &[right_commits[num_generations - 1].id().clone()], &[right_commits[num_generations - 2].id().clone()] @@ -305,7 +292,7 @@ fn test_index_commits_previous_operations(use_git: bool) { std::fs::create_dir(&index_operations_dir).unwrap(); let repo = load_repo_at_head(&settings, repo.repo_path()); - let index = as_readonly_impl(&repo); + let index = as_readonly_composite(&repo); // There should be the root commit, plus 3 more assert_eq!(index.num_commits(), 1 + 3); @@ -342,7 +329,7 @@ fn test_index_commits_incremental(use_git: bool) { .unwrap(); let repo = tx.commit(); - let index = as_readonly_impl(&repo); + let index = as_readonly_composite(&repo); // There should be the root commit, plus 1 more assert_eq!(index.num_commits(), 1 + 1); @@ -356,7 +343,7 @@ fn test_index_commits_incremental(use_git: bool) { tx.commit(); let repo = load_repo_at_head(&settings, repo.repo_path()); - let index = as_readonly_impl(&repo); + let index = as_readonly_composite(&repo); // There should be the root commit, plus 3 more assert_eq!(index.num_commits(), 1 + 3); @@ -394,14 +381,14 @@ fn test_index_commits_incremental_empty_transaction(use_git: bool) { .unwrap(); let repo = tx.commit(); - let index = as_readonly_impl(&repo); + let index = as_readonly_composite(&repo); // There should be the root commit, plus 1 more assert_eq!(index.num_commits(), 1 + 1); repo.start_transaction(&settings, "test").commit(); let repo = load_repo_at_head(&settings, repo.repo_path()); - let index = as_readonly_impl(&repo); + let index = as_readonly_composite(&repo); // There should be the root commit, plus 1 more assert_eq!(index.num_commits(), 1 + 1); @@ -438,11 +425,11 @@ fn test_index_commits_incremental_already_indexed(use_git: bool) { let repo = tx.commit(); assert!(repo.index().has_id(commit_a.id())); - assert_eq!(as_readonly_impl(&repo).num_commits(), 1 + 1); + assert_eq!(as_readonly_composite(&repo).num_commits(), 1 + 1); let mut tx = repo.start_transaction(&settings, "test"); let mut_repo = tx.mut_repo(); mut_repo.add_head(&commit_a); - assert_eq!(as_mutable_impl(mut_repo).num_commits(), 1 + 1); + assert_eq!(as_mutable_composite(mut_repo).num_commits(), 1 + 1); } #[must_use] @@ -466,6 +453,10 @@ fn as_readonly_impl(repo: &Arc) -> &ReadonlyIndexImpl { .unwrap() } +fn as_readonly_composite(repo: &Arc) -> CompositeIndex<'_> { + as_readonly_impl(repo).as_composite() +} + fn as_mutable_impl(repo: &MutableRepo) -> &MutableIndexImpl { repo.index() .as_any() @@ -473,8 +464,12 @@ fn as_mutable_impl(repo: &MutableRepo) -> &MutableIndexImpl { .unwrap() } +fn as_mutable_composite(repo: &MutableRepo) -> CompositeIndex<'_> { + as_mutable_impl(repo).as_composite() +} + fn commits_by_level(repo: &Arc) -> Vec { - as_readonly_impl(repo) + as_readonly_composite(repo) .stats() .levels .iter() @@ -555,7 +550,7 @@ fn test_index_store_type(use_git: bool) { let test_repo = TestRepo::init(use_git); let repo = &test_repo.repo; - assert_eq!(as_readonly_impl(repo).num_commits(), 1); + assert_eq!(as_readonly_composite(repo).num_commits(), 1); let index_store_type_path = repo.repo_path().join("index").join("type"); assert_eq!( std::fs::read_to_string(&index_store_type_path).unwrap(), @@ -569,5 +564,5 @@ fn test_index_store_type(use_git: bool) { std::fs::read_to_string(&index_store_type_path).unwrap(), "default" ); - assert_eq!(as_readonly_impl(&repo).num_commits(), 1); + assert_eq!(as_readonly_composite(&repo).num_commits(), 1); } diff --git a/src/commands/mod.rs b/src/commands/mod.rs index b523a7fe8..48039d484 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -3183,7 +3183,7 @@ fn cmd_debug( let index_impl: Option<&ReadonlyIndexWrapper> = repo.readonly_index().as_any().downcast_ref(); if let Some(index_impl) = index_impl { - let stats = index_impl.stats(); + let stats = index_impl.as_composite().stats(); writeln!(ui, "Number of commits: {}", stats.num_commits)?; writeln!(ui, "Number of merges: {}", stats.num_merges)?; writeln!(ui, "Max generation number: {}", stats.max_generation_number)?; @@ -3218,7 +3218,7 @@ fn cmd_debug( writeln!( ui, "Finished indexing {:?} commits.", - index_impl.stats().num_commits + index_impl.as_composite().stats().num_commits )?; } else { return Err(user_error(format!(