From 68cff2fa22248875a9f4956312f0ca8a942879ff Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Mon, 20 Mar 2023 22:04:21 -0700 Subject: [PATCH] repo: get change id index from revset instead of building it in repo This replaces the direct use of `IdIndex` in `ReadonlyRepo` by use of `Revset::change_id_index()`. I made the `Index` trait require `Send` and `Sync` in order to be able to store an instance of it in `ReadonlyRepo` (via `ChangeIdIndex`) and still have that be `Send` and `Sync`. We could alternatively store the `ChangeIdIndex` in a `Mutex`. Now that will be up to the `ChangeIdIndex` instead. --- lib/src/default_index_store.rs | 2 +- lib/src/index.rs | 2 +- lib/src/repo.rs | 48 +++++++++++++++++++--------------- lib/src/revset.rs | 2 +- 4 files changed, 30 insertions(+), 24 deletions(-) diff --git a/lib/src/default_index_store.rs b/lib/src/default_index_store.rs index d4429267f..d0c7fffe6 100644 --- a/lib/src/default_index_store.rs +++ b/lib/src/default_index_store.rs @@ -799,7 +799,7 @@ impl MutableIndex for MutableIndexImpl { } } -trait IndexSegment { +trait IndexSegment: Send + Sync { fn segment_num_parent_commits(&self) -> u32; fn segment_num_commits(&self) -> u32; diff --git a/lib/src/index.rs b/lib/src/index.rs index 5e68c410a..ec19df57b 100644 --- a/lib/src/index.rs +++ b/lib/src/index.rs @@ -47,7 +47,7 @@ pub trait IndexStore: Send + Sync + Debug { ) -> Result, IndexWriteError>; } -pub trait Index { +pub trait Index: Send + Sync { fn as_any(&self) -> &dyn Any; fn commit_id_to_pos(&self, commit_id: &CommitId) -> Option; diff --git a/lib/src/repo.rs b/lib/src/repo.rs index a92846351..53b1a9deb 100644 --- a/lib/src/repo.rs +++ b/lib/src/repo.rs @@ -15,7 +15,9 @@ use std::collections::{HashMap, HashSet}; use std::fmt::{Debug, Formatter}; use std::io::ErrorKind; +use std::ops::Deref; use std::path::{Path, PathBuf}; +use std::pin::Pin; use std::sync::Arc; use std::{fs, io}; @@ -28,7 +30,7 @@ use crate::backend::{Backend, BackendError, BackendResult, ChangeId, CommitId, O use crate::commit::Commit; use crate::commit_builder::CommitBuilder; use crate::dag_walk::topo_order_reverse; -use crate::default_index_store::{DefaultIndexStore, IndexPosition}; +use crate::default_index_store::DefaultIndexStore; use crate::git_backend::GitBackend; use crate::index::{HexPrefix, Index, IndexStore, MutableIndex, PrefixResolution, ReadonlyIndex}; use crate::local_backend::LocalBackend; @@ -36,6 +38,7 @@ use crate::op_heads_store::{self, OpHeadResolutionError, OpHeadsStore}; use crate::op_store::{BranchTarget, OpStore, OperationId, RefTarget, WorkspaceId}; use crate::operation::Operation; use crate::refs::merge_ref_targets; +use crate::revset::{ChangeIdIndex, Revset, RevsetExpression}; use crate::rewrite::DescendantRebaser; use crate::settings::{RepoSettings, UserSettings}; use crate::simple_op_heads_store::SimpleOpHeadsStore; @@ -77,9 +80,10 @@ pub struct ReadonlyRepo { operation: Operation, settings: RepoSettings, index_store: Arc, - index: OnceCell>, + index: OnceCell>>, + // Declared after `change_id_index` since it must outlive it on drop. + change_id_index: OnceCell>, // TODO: This should eventually become part of the index and not be stored fully in memory. - change_id_index: OnceCell, view: View, } @@ -209,21 +213,27 @@ impl ReadonlyRepo { pub fn readonly_index(&self) -> &dyn ReadonlyIndex { self.index .get_or_init(|| { - self.index_store - .get_index_at_op(&self.operation, &self.store) + Box::into_pin( + self.index_store + .get_index_at_op(&self.operation, &self.store), + ) }) - .as_ref() + .deref() } - fn change_id_index(&self) -> &ChangeIdIndex { - self.change_id_index.get_or_init(|| { - let heads = self.view().heads().iter().cloned().collect_vec(); - let walk = self.readonly_index().as_index().walk_revs(&heads, &[]); - IdIndex::from_vec( - walk.map(|entry| (entry.change_id(), entry.position())) - .collect(), - ) - }) + fn change_id_index<'a>(&'a self) -> &'a (dyn ChangeIdIndex + 'a) { + let change_id_index: &'a (dyn ChangeIdIndex + 'a) = self + .change_id_index + .get_or_init(|| { + let revset: Box> = RevsetExpression::all().evaluate(self).unwrap(); + let change_id_index: Box = revset.change_id_index(); + // evaluate() above only borrows the index, not the whole repo + let change_id_index: Box = + unsafe { std::mem::transmute(change_id_index) }; + change_id_index + }) + .as_ref(); + change_id_index } pub fn op_heads_store(&self) -> &Arc { @@ -277,9 +287,7 @@ impl Repo for ReadonlyRepo { } fn resolve_change_id_prefix(&self, prefix: &HexPrefix) -> PrefixResolution> { - let index = self.index(); - self.change_id_index() - .resolve_prefix_with(prefix, |&pos| index.entry_by_pos(pos).commit_id()) + self.change_id_index().resolve_prefix(prefix) } fn shortest_unique_change_id_prefix_len(&self, target_id: &ChangeId) -> usize { @@ -578,7 +586,7 @@ impl RepoLoader { operation, settings: self.repo_settings.clone(), index_store: self.index_store.clone(), - index: OnceCell::with_value(index), + index: OnceCell::with_value(Box::into_pin(index)), change_id_index: OnceCell::new(), view, }; @@ -1272,8 +1280,6 @@ mod dirty_cell { } } -type ChangeIdIndex = IdIndex; - #[derive(Debug, Clone)] pub struct IdIndex(Vec<(K, V)>); diff --git a/lib/src/revset.rs b/lib/src/revset.rs index 6e79e147d..41ce86a6b 100644 --- a/lib/src/revset.rs +++ b/lib/src/revset.rs @@ -1582,7 +1582,7 @@ pub trait Revset<'index> { fn is_empty(&self) -> bool; } -pub trait ChangeIdIndex { +pub trait ChangeIdIndex: Send + Sync { /// Resolve an unambiguous change ID prefix to the commit IDs in the revset. fn resolve_prefix(&self, prefix: &HexPrefix) -> PrefixResolution>;