index: move Revset::change_id_index() to Index

We current have `Revset::change_id_index()` for creating a
`ChangeIdIndex` for a given revset. I think it will be hard to make it
performant for general revsets, especially in very large repos and
with custom index implementations, like the one we have at Google. If
we instead restrict it to including all ancestors of a set of heads, I
think it will be much easier to implement. We only use
`Revset::change_id_index()` with revsets including all visible commits
today, so we won't lose any current functionality by making it more
restricted.

lib/src/default_index/composite.rs

@@ -296,6 +296,13 @@ impl<'a> CompositeIndex<'a> {
         candidate_positions
     }
 
+    pub(super) fn change_id_index(
+        &self,
+        heads: &mut dyn Iterator<Item = &CommitId>,
+    ) -> Box<dyn ChangeIdIndex + 'a> {
+        Box::new(ChangeIdIndexImpl::new(*self, heads))
+    }
+
     pub(super) fn evaluate_revset(
         &self,
         expression: &ResolvedExpression,
@@ -382,6 +389,13 @@ impl Index for CompositeIndex<'_> {
         ids
     }
 
+    fn change_id_index(
+        &self,
+        heads: &mut dyn Iterator<Item = &CommitId>,
+    ) -> Box<dyn ChangeIdIndex + '_> {
+        CompositeIndex::change_id_index(self, heads)
+    }
+
     fn evaluate_revset<'index>(
         &'index self,
         expression: &ResolvedExpression,
@@ -392,8 +406,8 @@ impl Index for CompositeIndex<'_> {
 }
 
 pub(super) struct ChangeIdIndexImpl<I> {
-    pub index: I,
-    pub pos_by_change: IdIndex<ChangeId, IndexPosition, 4>,
+    index: I,
+    pos_by_change: IdIndex<ChangeId, IndexPosition, 4>,
 }
 
 impl<I: AsCompositeIndex> ChangeIdIndexImpl<I> {

lib/src/default_index/mutable.rs

@@ -35,7 +35,7 @@ use super::readonly::{DefaultReadonlyIndex, ReadonlyIndexSegment};
 use crate::backend::{ChangeId, CommitId};
 use crate::commit::Commit;
 use crate::file_util::persist_content_addressed_temp_file;
-use crate::index::{Index, MutableIndex, ReadonlyIndex};
+use crate::index::{ChangeIdIndex, Index, MutableIndex, ReadonlyIndex};
 use crate::object_id::{HexPrefix, ObjectId, PrefixResolution};
 use crate::revset::{ResolvedExpression, Revset, RevsetEvaluationError};
 use crate::store::Store;
@@ -442,6 +442,13 @@ impl Index for DefaultMutableIndex {
         self.as_composite().topo_order(input)
     }
 
+    fn change_id_index(
+        &self,
+        heads: &mut dyn Iterator<Item = &CommitId>,
+    ) -> Box<dyn ChangeIdIndex + '_> {
+        self.as_composite().change_id_index(heads)
+    }
+
     fn evaluate_revset<'index>(
         &'index self,
         expression: &ResolvedExpression,

lib/src/default_index/readonly.rs

@@ -509,6 +509,14 @@ impl Index for DefaultReadonlyIndex {
         self.as_composite().topo_order(input)
     }
 
+    // TODO: Create a persistent lookup from change id to commit ids.
+    fn change_id_index(
+        &self,
+        heads: &mut dyn Iterator<Item = &CommitId>,
+    ) -> Box<dyn ChangeIdIndex + '_> {
+        self.as_composite().change_id_index(heads)
+    }
+
     fn evaluate_revset<'index>(
         &'index self,
         expression: &ResolvedExpression,

lib/src/default_index/revset_engine.rs

@@ -23,12 +23,9 @@ use std::sync::Arc;
 
 use itertools::Itertools;
 
-use super::composite::ChangeIdIndexImpl;
 use super::revset_graph_iterator::RevsetGraphIterator;
 use crate::backend::{ChangeId, CommitId, MillisSinceEpoch};
 use crate::default_index::{AsCompositeIndex, CompositeIndex, IndexEntry, IndexPosition};
-use crate::id_prefix::IdIndex;
-use crate::index::ChangeIdIndex;
 use crate::matchers::{EverythingMatcher, Matcher, PrefixMatcher, Visit};
 use crate::repo_path::RepoPath;
 use crate::revset::{
@@ -133,18 +130,6 @@ where
         Box::new(self.iter_graph_impl())
     }
 
-    fn change_id_index(&self) -> Box<dyn ChangeIdIndex + 'index> {
-        // TODO: Create a persistent lookup from change id to commit ids.
-        let mut pos_by_change = IdIndex::builder();
-        for entry in self.entries() {
-            pos_by_change.insert(&entry.change_id(), entry.position());
-        }
-        Box::new(ChangeIdIndexImpl {
-            index: self.index.clone(),
-            pos_by_change: pos_by_change.build(),
-        })
-    }
-
     fn is_empty(&self) -> bool {
         self.entries().next().is_none()
     }

lib/src/index.rs

@@ -72,6 +72,11 @@ pub trait Index: Send + Sync {
     /// Parents before children
     fn topo_order(&self, input: &mut dyn Iterator<Item = &CommitId>) -> Vec<CommitId>;
 
+    fn change_id_index(
+        &self,
+        heads: &mut dyn Iterator<Item = &CommitId>,
+    ) -> Box<dyn ChangeIdIndex + '_>;
+
     fn evaluate_revset<'index>(
         &'index self,
         expression: &ResolvedExpression,

lib/src/repo.rs

@@ -49,7 +49,6 @@ use crate::operation::Operation;
 use crate::refs::{
     diff_named_ref_targets, diff_named_remote_refs, merge_ref_targets, merge_remote_refs,
 };
-use crate::revset::RevsetExpression;
 use crate::rewrite::{DescendantRebaser, RebaseOptions};
 use crate::settings::{RepoSettings, UserSettings};
 use crate::signing::{SignInitError, Signer};
@@ -1402,14 +1401,16 @@ impl Repo for MutableRepo {
     }
 
     fn resolve_change_id_prefix(&self, prefix: &HexPrefix) -> PrefixResolution<Vec<CommitId>> {
-        let revset = RevsetExpression::all().evaluate_programmatic(self).unwrap();
-        let change_id_index = revset.change_id_index();
+        let change_id_index = self
+            .index()
+            .change_id_index(&mut self.view().heads().iter());
         change_id_index.resolve_prefix(prefix)
     }
 
     fn shortest_unique_change_id_prefix_len(&self, target_id: &ChangeId) -> usize {
-        let revset = RevsetExpression::all().evaluate_programmatic(self).unwrap();
-        let change_id_index = revset.change_id_index();
+        let change_id_index = self
+            .index()
+            .change_id_index(&mut self.view().heads().iter());
         change_id_index.shortest_unique_prefix_len(target_id)
     }
 }

lib/src/revset.rs

@@ -35,7 +35,6 @@ use crate::backend::{BackendError, BackendResult, ChangeId, CommitId};
 use crate::commit::Commit;
 use crate::git;
 use crate::hex_util::to_forward_hex;
-use crate::index::ChangeIdIndex;
 use crate::object_id::{HexPrefix, PrefixResolution};
 use crate::op_store::WorkspaceId;
 use crate::repo::Repo;
@@ -2414,8 +2413,6 @@ pub trait Revset<'index>: fmt::Debug {
 
     fn iter_graph(&self) -> Box<dyn Iterator<Item = (CommitId, Vec<RevsetGraphEdge>)> + '_>;
 
-    fn change_id_index(&self) -> Box<dyn ChangeIdIndex + 'index>;
-
     fn is_empty(&self) -> bool;
 
     fn count(&self) -> usize;

lib/tests/test_index.rs

@@ -26,7 +26,6 @@ use jj_lib::default_index::{
 use jj_lib::index::Index as _;
 use jj_lib::object_id::{HexPrefix, ObjectId as _, PrefixResolution};
 use jj_lib::repo::{MutableRepo, ReadonlyRepo, Repo};
-use jj_lib::revset::RevsetExpression;
 use jj_lib::settings::UserSettings;
 use testutils::test_backend::TestBackend;
 use testutils::{
@@ -674,11 +673,9 @@ fn test_change_id_index() {
     let commit_5 = commit_with_change_id("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb");
 
     let index_for_heads = |commits: &[&Commit]| {
-        RevsetExpression::commits(commits.iter().map(|commit| commit.id().clone()).collect())
-            .ancestors()
-            .evaluate_programmatic(tx.repo())
-            .unwrap()
-            .change_id_index()
+        tx.repo()
+            .index()
+            .change_id_index(&mut commits.iter().map(|commit| commit.id()))
     };
     let change_id_index = index_for_heads(&[&commit_1, &commit_2, &commit_3, &commit_4, &commit_5]);
     let prefix_len =