Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

index: IndexSegment API cleanup #2743

Merged
merged 5 commits into from
Dec 25, 2023
Merged

index: IndexSegment API cleanup #2743

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7c3e775
index: inline segment_entry_by_pos()
yuja Dec 21, 2023
dee463b
index: make segment-level lookup return neighbor commit ids instead o…
yuja Dec 25, 2023
0215a4a
index: introduce newtype that represents segment-local position
yuja Dec 21, 2023
2d82025
index: remove "segment_" prefix from IndexSegment methods
yuja Dec 23, 2023
5b401a2
index: rename resolve_prefix() to resolve_commit_id_prefix()
yuja Dec 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cli/src/commands/bench.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ pub(crate) fn cmd_bench(
let workspace_command = command.workspace_helper(ui)?;
let prefix = HexPrefix::new(&args.prefix).unwrap();
let index = workspace_command.repo().index();
let routine = || index.resolve_prefix(&prefix);
let routine = || index.resolve_commit_id_prefix(&prefix);
run_bench(
ui,
&format!("resolveprefix-{}", prefix.hex()),
Expand Down
67 changes: 29 additions & 38 deletions lib/src/default_index/composite.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ use std::sync::Arc;

use itertools::Itertools;

use super::entry::{IndexEntry, IndexPosition, IndexPositionByGeneration, SmallIndexPositionsVec};
use super::entry::{
IndexEntry, IndexPosition, IndexPositionByGeneration, LocalPosition, SmallIndexPositionsVec,
};
use super::readonly::ReadonlyIndexSegment;
use super::rev_walk::RevWalk;
use crate::backend::{ChangeId, CommitId, ObjectId};
Expand All @@ -31,36 +33,34 @@ use crate::store::Store;
use crate::{backend, default_revset_engine};

pub(super) trait IndexSegment: Send + Sync {
fn segment_num_parent_commits(&self) -> u32;
fn num_parent_commits(&self) -> u32;
martinvonz marked this conversation as resolved.
Show resolved Hide resolved

fn segment_num_commits(&self) -> u32;
fn num_local_commits(&self) -> u32;

fn segment_parent_file(&self) -> Option<&Arc<ReadonlyIndexSegment>>;
fn parent_file(&self) -> Option<&Arc<ReadonlyIndexSegment>>;

fn segment_name(&self) -> Option<String>;
fn name(&self) -> Option<String>;

fn segment_commit_id_to_pos(&self, commit_id: &CommitId) -> Option<IndexPosition>;
fn 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 resolve_neighbor_commit_ids(
&self,
commit_id: &CommitId,
) -> (Option<IndexPosition>, Option<IndexPosition>);

fn segment_resolve_prefix(&self, prefix: &HexPrefix) -> PrefixResolution<CommitId>;
) -> (Option<CommitId>, Option<CommitId>);

fn segment_generation_number(&self, local_pos: u32) -> u32;
fn resolve_commit_id_prefix(&self, prefix: &HexPrefix) -> PrefixResolution<CommitId>;

fn segment_commit_id(&self, local_pos: u32) -> CommitId;
fn generation_number(&self, local_pos: LocalPosition) -> u32;

fn segment_change_id(&self, local_pos: u32) -> ChangeId;
fn commit_id(&self, local_pos: LocalPosition) -> CommitId;

fn segment_num_parents(&self, local_pos: u32) -> u32;
fn change_id(&self, local_pos: LocalPosition) -> ChangeId;

fn segment_parent_positions(&self, local_pos: u32) -> SmallIndexPositionsVec;
fn num_parents(&self, local_pos: LocalPosition) -> u32;

fn segment_entry_by_pos(&self, pos: IndexPosition, local_pos: u32) -> IndexEntry;
fn parent_positions(&self, local_pos: LocalPosition) -> SmallIndexPositionsVec;
}

/// Abstraction over owned and borrowed types that can be cheaply converted to
Expand Down Expand Up @@ -95,8 +95,8 @@ impl<'a> CompositeIndex<'a> {
pub(super) fn ancestor_files_without_local(
&self,
) -> impl Iterator<Item = &'a Arc<ReadonlyIndexSegment>> {
let parent_file = self.0.segment_parent_file();
iter::successors(parent_file, |file| file.segment_parent_file())
let parent_file = self.0.parent_file();
iter::successors(parent_file, |file| file.parent_file())
}

/// Iterates self and its ancestor index segments.
Expand All @@ -108,7 +108,7 @@ impl<'a> CompositeIndex<'a> {
}

pub fn num_commits(&self) -> u32 {
self.0.segment_num_parent_commits() + self.0.segment_num_commits()
self.0.num_parent_commits() + self.0.num_local_commits()
}

pub fn stats(&self) -> IndexStats {
Expand All @@ -133,8 +133,8 @@ impl<'a> CompositeIndex<'a> {
let mut levels = self
.ancestor_index_segments()
.map(|segment| IndexLevelStats {
num_commits: segment.segment_num_commits(),
name: segment.segment_name(),
num_commits: segment.num_local_commits(),
name: segment.name(),
})
.collect_vec();
levels.reverse();
Expand All @@ -152,15 +152,15 @@ impl<'a> CompositeIndex<'a> {
pub fn entry_by_pos(&self, pos: IndexPosition) -> IndexEntry<'a> {
self.ancestor_index_segments()
.find_map(|segment| {
u32::checked_sub(pos.0, segment.segment_num_parent_commits())
.map(|local_pos| segment.segment_entry_by_pos(pos, local_pos))
u32::checked_sub(pos.0, segment.num_parent_commits())
.map(|local_pos| IndexEntry::new(segment, pos, LocalPosition(local_pos)))
})
.unwrap()
}

pub fn commit_id_to_pos(&self, commit_id: &CommitId) -> Option<IndexPosition> {
self.ancestor_index_segments()
.find_map(|segment| segment.segment_commit_id_to_pos(commit_id))
.find_map(|segment| segment.commit_id_to_pos(commit_id))
}

/// Suppose the given `commit_id` exists, returns the previous and next
Expand All @@ -170,16 +170,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.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(),
Expand Down Expand Up @@ -333,13 +324,13 @@ impl Index for CompositeIndex<'_> {
.unwrap_or(0)
}

fn resolve_prefix(&self, prefix: &HexPrefix) -> PrefixResolution<CommitId> {
fn resolve_commit_id_prefix(&self, prefix: &HexPrefix) -> PrefixResolution<CommitId> {
self.ancestor_index_segments()
.fold(PrefixResolution::NoMatch, |acc_match, segment| {
if acc_match == PrefixResolution::AmbiguousMatch {
acc_match // avoid checking the parent file(s)
} else {
let local_match = segment.segment_resolve_prefix(prefix);
let local_match = segment.resolve_commit_id_prefix(prefix);
acc_match.plus(&local_match)
}
})
Expand Down
25 changes: 17 additions & 8 deletions lib/src/default_index/entry.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,18 @@ use smallvec::SmallVec;
use super::composite::{CompositeIndex, IndexSegment};
use crate::backend::{ChangeId, CommitId, ObjectId};

/// Global index position.
#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone, Copy, Hash)]
pub struct IndexPosition(pub(super) u32);

impl IndexPosition {
pub const MAX: Self = IndexPosition(u32::MAX);
}

/// Local position within an index segment.
#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone, Copy, Hash)]
pub(super) struct LocalPosition(pub(super) u32);

// SmallVec reuses two pointer-size fields as inline area, which meas we can
// inline up to 16 bytes (on 64-bit platform) for free.
pub(super) type SmallIndexPositionsVec = SmallVec<[IndexPosition; 4]>;
Expand All @@ -37,8 +42,8 @@ pub(super) type SmallIndexPositionsVec = SmallVec<[IndexPosition; 4]>;
pub struct IndexEntry<'a> {
source: &'a dyn IndexSegment,
pos: IndexPosition,
// Position within the source segment
local_pos: u32,
/// Position within the source segment
local_pos: LocalPosition,
}

impl Debug for IndexEntry<'_> {
Expand Down Expand Up @@ -66,7 +71,11 @@ impl Hash for IndexEntry<'_> {
}

impl<'a> IndexEntry<'a> {
pub(super) fn new(source: &'a dyn IndexSegment, pos: IndexPosition, local_pos: u32) -> Self {
pub(super) fn new(
source: &'a dyn IndexSegment,
pos: IndexPosition,
local_pos: LocalPosition,
) -> Self {
IndexEntry {
source,
pos,
Expand All @@ -79,23 +88,23 @@ impl<'a> IndexEntry<'a> {
}

pub fn generation_number(&self) -> u32 {
self.source.segment_generation_number(self.local_pos)
self.source.generation_number(self.local_pos)
}

pub fn commit_id(&self) -> CommitId {
self.source.segment_commit_id(self.local_pos)
self.source.commit_id(self.local_pos)
}

pub fn change_id(&self) -> ChangeId {
self.source.segment_change_id(self.local_pos)
self.source.change_id(self.local_pos)
}

pub fn num_parents(&self) -> u32 {
self.source.segment_num_parents(self.local_pos)
self.source.num_parents(self.local_pos)
}

pub fn parent_positions(&self) -> SmallIndexPositionsVec {
self.source.segment_parent_positions(self.local_pos)
self.source.parent_positions(self.local_pos)
}

pub fn parents(&self) -> impl ExactSizeIterator<Item = IndexEntry<'a>> {
Expand Down
66 changes: 33 additions & 33 deletions lib/src/default_index/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,44 +353,44 @@ mod tests {

// Can find commits given the full hex number
assert_eq!(
index.resolve_prefix(&HexPrefix::new(&id_0.hex()).unwrap()),
index.resolve_commit_id_prefix(&HexPrefix::new(&id_0.hex()).unwrap()),
PrefixResolution::SingleMatch(id_0)
);
assert_eq!(
index.resolve_prefix(&HexPrefix::new(&id_1.hex()).unwrap()),
index.resolve_commit_id_prefix(&HexPrefix::new(&id_1.hex()).unwrap()),
PrefixResolution::SingleMatch(id_1)
);
assert_eq!(
index.resolve_prefix(&HexPrefix::new(&id_2.hex()).unwrap()),
index.resolve_commit_id_prefix(&HexPrefix::new(&id_2.hex()).unwrap()),
PrefixResolution::SingleMatch(id_2)
);
// Test nonexistent commits
assert_eq!(
index.resolve_prefix(&HexPrefix::new("ffffff").unwrap()),
index.resolve_commit_id_prefix(&HexPrefix::new("ffffff").unwrap()),
PrefixResolution::NoMatch
);
assert_eq!(
index.resolve_prefix(&HexPrefix::new("000001").unwrap()),
index.resolve_commit_id_prefix(&HexPrefix::new("000001").unwrap()),
PrefixResolution::NoMatch
);
// Test ambiguous prefix
assert_eq!(
index.resolve_prefix(&HexPrefix::new("0").unwrap()),
index.resolve_commit_id_prefix(&HexPrefix::new("0").unwrap()),
PrefixResolution::AmbiguousMatch
);
// Test a globally unique prefix in initial part
assert_eq!(
index.resolve_prefix(&HexPrefix::new("009").unwrap()),
index.resolve_commit_id_prefix(&HexPrefix::new("009").unwrap()),
PrefixResolution::SingleMatch(CommitId::from_hex("009999"))
);
// Test a globally unique prefix in incremental part
assert_eq!(
index.resolve_prefix(&HexPrefix::new("03").unwrap()),
index.resolve_commit_id_prefix(&HexPrefix::new("03").unwrap()),
PrefixResolution::SingleMatch(CommitId::from_hex("033333"))
);
// Test a locally unique but globally ambiguous prefix
assert_eq!(
index.resolve_prefix(&HexPrefix::new("0554").unwrap()),
index.resolve_commit_id_prefix(&HexPrefix::new("0554").unwrap()),
PrefixResolution::AmbiguousMatch
);
}
Expand Down Expand Up @@ -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.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.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.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.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.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.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.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.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.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.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.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.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
Expand Down
Loading