ok/jj
1
0
Fork 0
forked from mirrors/jj

index: make segment-level lookup return neighbor commit ids instead of positions

Both readonly and mutable segments know the commit ids to return, and the
caller only needs the ids. Since segment_commit_id(local_pos) scans the graph
entries, doing that would increase the chance of cache miss.
This commit is contained in:
Yuya Nishihara 2023-12-25 21:01:55 +09:00
parent 0e7834feb9
commit ee8d5e279a
4 changed files with 41 additions and 50 deletions

View file

@ -41,12 +41,12 @@ pub(super) trait IndexSegment: Send + Sync {
fn segment_commit_id_to_pos(&self, commit_id: &CommitId) -> Option<IndexPosition>;
/// Suppose the given `commit_id` exists, returns the positions of the
/// previous and next commit ids in lexicographical order.
fn segment_commit_id_to_neighbor_positions(
/// Suppose the given `commit_id` exists, returns the previous and next
/// commit ids in lexicographical order.
fn segment_resolve_neighbor_commit_ids(
&self,
commit_id: &CommitId,
) -> (Option<IndexPosition>, Option<IndexPosition>);
) -> (Option<CommitId>, Option<CommitId>);
fn segment_resolve_prefix(&self, prefix: &HexPrefix) -> PrefixResolution<CommitId>;
@ -168,16 +168,7 @@ impl<'a> CompositeIndex<'a> {
commit_id: &CommitId,
) -> (Option<CommitId>, Option<CommitId>) {
self.ancestor_index_segments()
.map(|segment| {
let num_parent_commits = segment.segment_num_parent_commits();
let to_local_pos = |pos: IndexPosition| pos.0 - num_parent_commits;
let (prev_pos, next_pos) =
segment.segment_commit_id_to_neighbor_positions(commit_id);
(
prev_pos.map(|p| segment.segment_commit_id(to_local_pos(p))),
next_pos.map(|p| segment.segment_commit_id(to_local_pos(p))),
)
})
.map(|segment| segment.segment_resolve_neighbor_commit_ids(commit_id))
.reduce(|(acc_prev_id, acc_next_id), (prev_id, next_id)| {
(
acc_prev_id.into_iter().chain(prev_id).max(),

View file

@ -423,58 +423,58 @@ mod tests {
// Local lookup in readonly index, commit_id exists.
assert_eq!(
initial_file.segment_commit_id_to_neighbor_positions(&id_0),
(None, Some(IndexPosition(1))),
initial_file.segment_resolve_neighbor_commit_ids(&id_0),
(None, Some(id_1.clone())),
);
assert_eq!(
initial_file.segment_commit_id_to_neighbor_positions(&id_1),
(Some(IndexPosition(0)), Some(IndexPosition(2))),
initial_file.segment_resolve_neighbor_commit_ids(&id_1),
(Some(id_0.clone()), Some(id_2.clone())),
);
assert_eq!(
initial_file.segment_commit_id_to_neighbor_positions(&id_2),
(Some(IndexPosition(1)), None),
initial_file.segment_resolve_neighbor_commit_ids(&id_2),
(Some(id_1.clone()), None),
);
// Local lookup in readonly index, commit_id does not exist.
assert_eq!(
initial_file.segment_commit_id_to_neighbor_positions(&CommitId::from_hex("000000")),
(None, Some(IndexPosition(0))),
initial_file.segment_resolve_neighbor_commit_ids(&CommitId::from_hex("000000")),
(None, Some(id_0.clone())),
);
assert_eq!(
initial_file.segment_commit_id_to_neighbor_positions(&CommitId::from_hex("000002")),
(Some(IndexPosition(0)), Some(IndexPosition(1))),
initial_file.segment_resolve_neighbor_commit_ids(&CommitId::from_hex("000002")),
(Some(id_0.clone()), Some(id_1.clone())),
);
assert_eq!(
initial_file.segment_commit_id_to_neighbor_positions(&CommitId::from_hex("ffffff")),
(Some(IndexPosition(2)), None),
initial_file.segment_resolve_neighbor_commit_ids(&CommitId::from_hex("ffffff")),
(Some(id_2.clone()), None),
);
// Local lookup in mutable index, commit_id exists. id_5 < id_3 < id_4
assert_eq!(
mutable_segment.segment_commit_id_to_neighbor_positions(&id_5),
(None, Some(IndexPosition(3))),
mutable_segment.segment_resolve_neighbor_commit_ids(&id_5),
(None, Some(id_3.clone())),
);
assert_eq!(
mutable_segment.segment_commit_id_to_neighbor_positions(&id_3),
(Some(IndexPosition(5)), Some(IndexPosition(4))),
mutable_segment.segment_resolve_neighbor_commit_ids(&id_3),
(Some(id_5.clone()), Some(id_4.clone())),
);
assert_eq!(
mutable_segment.segment_commit_id_to_neighbor_positions(&id_4),
(Some(IndexPosition(3)), None),
mutable_segment.segment_resolve_neighbor_commit_ids(&id_4),
(Some(id_3.clone()), None),
);
// Local lookup in mutable index, commit_id does not exist. id_5 < id_3 < id_4
assert_eq!(
mutable_segment.segment_commit_id_to_neighbor_positions(&CommitId::from_hex("033332")),
(None, Some(IndexPosition(5))),
mutable_segment.segment_resolve_neighbor_commit_ids(&CommitId::from_hex("033332")),
(None, Some(id_5.clone())),
);
assert_eq!(
mutable_segment.segment_commit_id_to_neighbor_positions(&CommitId::from_hex("033334")),
(Some(IndexPosition(5)), Some(IndexPosition(3))),
mutable_segment.segment_resolve_neighbor_commit_ids(&CommitId::from_hex("033334")),
(Some(id_5.clone()), Some(id_3.clone())),
);
assert_eq!(
mutable_segment.segment_commit_id_to_neighbor_positions(&CommitId::from_hex("ffffff")),
(Some(IndexPosition(4)), None),
mutable_segment.segment_resolve_neighbor_commit_ids(&CommitId::from_hex("ffffff")),
(Some(id_4.clone()), None),
);
// Global lookup, commit_id exists. id_0 < id_1 < id_5 < id_3 < id_2 < id_4

View file

@ -319,21 +319,21 @@ impl IndexSegment for MutableIndexSegment {
self.lookup.get(commit_id).cloned()
}
fn segment_commit_id_to_neighbor_positions(
fn segment_resolve_neighbor_commit_ids(
&self,
commit_id: &CommitId,
) -> (Option<IndexPosition>, Option<IndexPosition>) {
let prev_pos = self
) -> (Option<CommitId>, Option<CommitId>) {
let prev_id = self
.lookup
.range((Bound::Unbounded, Bound::Excluded(commit_id)))
.next_back()
.map(|(_, &pos)| pos);
let next_pos = self
.map(|(id, _)| id.clone());
let next_id = self
.lookup
.range((Bound::Excluded(commit_id), Bound::Unbounded))
.next()
.map(|(_, &pos)| pos);
(prev_pos, next_pos)
.map(|(id, _)| id.clone());
(prev_id, next_id)
}
fn segment_resolve_prefix(&self, prefix: &HexPrefix) -> PrefixResolution<CommitId> {

View file

@ -385,10 +385,10 @@ impl IndexSegment for ReadonlyIndexSegment {
(&entry.commit_id() == commit_id).then(|| entry.pos())
}
fn segment_commit_id_to_neighbor_positions(
fn segment_resolve_neighbor_commit_ids(
&self,
commit_id: &CommitId,
) -> (Option<IndexPosition>, Option<IndexPosition>) {
) -> (Option<CommitId>, Option<CommitId>) {
if let Some(lookup_pos) = self.commit_id_byte_prefix_to_lookup_pos(commit_id) {
let entry_commit_id = self.lookup_entry(lookup_pos).commit_id();
let (prev_lookup_pos, next_lookup_pos) = match entry_commit_id.cmp(commit_id) {
@ -402,9 +402,9 @@ impl IndexSegment for ReadonlyIndexSegment {
}
Ordering::Greater => (lookup_pos.checked_sub(1), Some(lookup_pos)),
};
let prev_pos = prev_lookup_pos.map(|p| self.lookup_entry(p).pos());
let next_pos = next_lookup_pos.map(|p| self.lookup_entry(p).pos());
(prev_pos, next_pos)
let prev_id = prev_lookup_pos.map(|p| self.lookup_entry(p).commit_id());
let next_id = next_lookup_pos.map(|p| self.lookup_entry(p).commit_id());
(prev_id, next_id)
} else {
(None, None)
}