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

index: introduce newtype that represents segment-local position

I'm thinking of changing some IndexSegment methods to return LocalPosition
instead of global IndexPosition, and using u32 there would be a source of bugs.
This commit is contained in:
Yuya Nishihara 2023-12-21 17:47:19 +09:00
parent ee8d5e279a
commit c9b9e2864e
4 changed files with 41 additions and 30 deletions

View file

@ -21,7 +21,9 @@ use std::sync::Arc;
use itertools::Itertools; use itertools::Itertools;
use super::entry::{IndexEntry, IndexPosition, IndexPositionByGeneration, SmallIndexPositionsVec}; use super::entry::{
IndexEntry, IndexPosition, IndexPositionByGeneration, LocalPosition, SmallIndexPositionsVec,
};
use super::readonly::ReadonlyIndexSegment; use super::readonly::ReadonlyIndexSegment;
use super::rev_walk::RevWalk; use super::rev_walk::RevWalk;
use crate::backend::{ChangeId, CommitId, ObjectId}; use crate::backend::{ChangeId, CommitId, ObjectId};
@ -50,15 +52,15 @@ pub(super) trait IndexSegment: Send + Sync {
fn segment_resolve_prefix(&self, prefix: &HexPrefix) -> PrefixResolution<CommitId>; fn segment_resolve_prefix(&self, prefix: &HexPrefix) -> PrefixResolution<CommitId>;
fn segment_generation_number(&self, local_pos: u32) -> u32; fn segment_generation_number(&self, local_pos: LocalPosition) -> u32;
fn segment_commit_id(&self, local_pos: u32) -> CommitId; fn segment_commit_id(&self, local_pos: LocalPosition) -> CommitId;
fn segment_change_id(&self, local_pos: u32) -> ChangeId; fn segment_change_id(&self, local_pos: LocalPosition) -> ChangeId;
fn segment_num_parents(&self, local_pos: u32) -> u32; fn segment_num_parents(&self, local_pos: LocalPosition) -> u32;
fn segment_parent_positions(&self, local_pos: u32) -> SmallIndexPositionsVec; fn segment_parent_positions(&self, local_pos: LocalPosition) -> SmallIndexPositionsVec;
} }
/// Abstraction over owned and borrowed types that can be cheaply converted to /// Abstraction over owned and borrowed types that can be cheaply converted to
@ -151,7 +153,7 @@ impl<'a> CompositeIndex<'a> {
self.ancestor_index_segments() self.ancestor_index_segments()
.find_map(|segment| { .find_map(|segment| {
u32::checked_sub(pos.0, segment.segment_num_parent_commits()) u32::checked_sub(pos.0, segment.segment_num_parent_commits())
.map(|local_pos| IndexEntry::new(segment, pos, local_pos)) .map(|local_pos| IndexEntry::new(segment, pos, LocalPosition(local_pos)))
}) })
.unwrap() .unwrap()
} }

View file

@ -22,6 +22,7 @@ use smallvec::SmallVec;
use super::composite::{CompositeIndex, IndexSegment}; use super::composite::{CompositeIndex, IndexSegment};
use crate::backend::{ChangeId, CommitId, ObjectId}; use crate::backend::{ChangeId, CommitId, ObjectId};
/// Global index position.
#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone, Copy, Hash)] #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone, Copy, Hash)]
pub struct IndexPosition(pub(super) u32); pub struct IndexPosition(pub(super) u32);
@ -29,6 +30,10 @@ impl IndexPosition {
pub const MAX: Self = IndexPosition(u32::MAX); 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 // SmallVec reuses two pointer-size fields as inline area, which meas we can
// inline up to 16 bytes (on 64-bit platform) for free. // inline up to 16 bytes (on 64-bit platform) for free.
pub(super) type SmallIndexPositionsVec = SmallVec<[IndexPosition; 4]>; pub(super) type SmallIndexPositionsVec = SmallVec<[IndexPosition; 4]>;
@ -37,8 +42,8 @@ pub(super) type SmallIndexPositionsVec = SmallVec<[IndexPosition; 4]>;
pub struct IndexEntry<'a> { pub struct IndexEntry<'a> {
source: &'a dyn IndexSegment, source: &'a dyn IndexSegment,
pos: IndexPosition, pos: IndexPosition,
// Position within the source segment /// Position within the source segment
local_pos: u32, local_pos: LocalPosition,
} }
impl Debug for IndexEntry<'_> { impl Debug for IndexEntry<'_> {
@ -66,7 +71,11 @@ impl Hash for IndexEntry<'_> {
} }
impl<'a> IndexEntry<'a> { 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 { IndexEntry {
source, source,
pos, pos,

View file

@ -30,7 +30,7 @@ use smallvec::SmallVec;
use tempfile::NamedTempFile; use tempfile::NamedTempFile;
use super::composite::{AsCompositeIndex, CompositeIndex, IndexSegment}; use super::composite::{AsCompositeIndex, CompositeIndex, IndexSegment};
use super::entry::{IndexPosition, SmallIndexPositionsVec}; use super::entry::{IndexPosition, LocalPosition, SmallIndexPositionsVec};
use super::readonly::{DefaultReadonlyIndex, ReadonlyIndexSegment}; use super::readonly::{DefaultReadonlyIndex, ReadonlyIndexSegment};
use crate::backend::{ChangeId, CommitId, ObjectId}; use crate::backend::{ChangeId, CommitId, ObjectId};
use crate::commit::Commit; use crate::commit::Commit;
@ -351,28 +351,28 @@ impl IndexSegment for MutableIndexSegment {
} }
} }
fn segment_generation_number(&self, local_pos: u32) -> u32 { fn segment_generation_number(&self, local_pos: LocalPosition) -> u32 {
self.graph[local_pos as usize].generation_number self.graph[local_pos.0 as usize].generation_number
} }
fn segment_commit_id(&self, local_pos: u32) -> CommitId { fn segment_commit_id(&self, local_pos: LocalPosition) -> CommitId {
self.graph[local_pos as usize].commit_id.clone() self.graph[local_pos.0 as usize].commit_id.clone()
} }
fn segment_change_id(&self, local_pos: u32) -> ChangeId { fn segment_change_id(&self, local_pos: LocalPosition) -> ChangeId {
self.graph[local_pos as usize].change_id.clone() self.graph[local_pos.0 as usize].change_id.clone()
} }
fn segment_num_parents(&self, local_pos: u32) -> u32 { fn segment_num_parents(&self, local_pos: LocalPosition) -> u32 {
self.graph[local_pos as usize] self.graph[local_pos.0 as usize]
.parent_positions .parent_positions
.len() .len()
.try_into() .try_into()
.unwrap() .unwrap()
} }
fn segment_parent_positions(&self, local_pos: u32) -> SmallIndexPositionsVec { fn segment_parent_positions(&self, local_pos: LocalPosition) -> SmallIndexPositionsVec {
self.graph[local_pos as usize].parent_positions.clone() self.graph[local_pos.0 as usize].parent_positions.clone()
} }
} }

View file

@ -27,7 +27,7 @@ use smallvec::SmallVec;
use thiserror::Error; use thiserror::Error;
use super::composite::{AsCompositeIndex, CompositeIndex, IndexSegment}; use super::composite::{AsCompositeIndex, CompositeIndex, IndexSegment};
use super::entry::{IndexPosition, SmallIndexPositionsVec}; use super::entry::{IndexPosition, LocalPosition, SmallIndexPositionsVec};
use super::mutable::DefaultMutableIndex; use super::mutable::DefaultMutableIndex;
use crate::backend::{ChangeId, CommitId, ObjectId}; use crate::backend::{ChangeId, CommitId, ObjectId};
use crate::default_revset_engine; use crate::default_revset_engine;
@ -310,9 +310,9 @@ impl ReadonlyIndexSegment {
self.change_id_length self.change_id_length
} }
fn graph_entry(&self, local_pos: u32) -> CommitGraphEntry { fn graph_entry(&self, local_pos: LocalPosition) -> CommitGraphEntry {
assert!(local_pos < self.num_local_commits); assert!(local_pos.0 < self.num_local_commits);
let offset = (local_pos as usize) * self.commit_graph_entry_size; let offset = (local_pos.0 as usize) * self.commit_graph_entry_size;
CommitGraphEntry { CommitGraphEntry {
data: &self.data[offset..][..self.commit_graph_entry_size], data: &self.data[offset..][..self.commit_graph_entry_size],
commit_id_length: self.commit_id_length, commit_id_length: self.commit_id_length,
@ -426,23 +426,23 @@ impl IndexSegment for ReadonlyIndexSegment {
} }
} }
fn segment_generation_number(&self, local_pos: u32) -> u32 { fn segment_generation_number(&self, local_pos: LocalPosition) -> u32 {
self.graph_entry(local_pos).generation_number() self.graph_entry(local_pos).generation_number()
} }
fn segment_commit_id(&self, local_pos: u32) -> CommitId { fn segment_commit_id(&self, local_pos: LocalPosition) -> CommitId {
self.graph_entry(local_pos).commit_id() self.graph_entry(local_pos).commit_id()
} }
fn segment_change_id(&self, local_pos: u32) -> ChangeId { fn segment_change_id(&self, local_pos: LocalPosition) -> ChangeId {
self.graph_entry(local_pos).change_id() self.graph_entry(local_pos).change_id()
} }
fn segment_num_parents(&self, local_pos: u32) -> u32 { fn segment_num_parents(&self, local_pos: LocalPosition) -> u32 {
self.graph_entry(local_pos).num_parents() self.graph_entry(local_pos).num_parents()
} }
fn segment_parent_positions(&self, local_pos: u32) -> SmallIndexPositionsVec { fn segment_parent_positions(&self, local_pos: LocalPosition) -> SmallIndexPositionsVec {
let graph_entry = self.graph_entry(local_pos); let graph_entry = self.graph_entry(local_pos);
let mut parent_entries = SmallVec::with_capacity(graph_entry.num_parents() as usize); let mut parent_entries = SmallVec::with_capacity(graph_entry.num_parents() as usize);
if graph_entry.num_parents() >= 1 { if graph_entry.num_parents() >= 1 {