dag: rename GroupId to Group

Summary:
Address review comment in D18640899. This makes the word `id` universally
refer to a same concept within the crate.

Reviewed By: sfilipco

Differential Revision: D18820723

fbshipit-source-id: 6803192db7e1304a72100568f8f29b90f25c7779
This commit is contained in:
Jun Wu 2019-12-17 19:25:48 -08:00 committed by Facebook Github Bot
parent d3543665eb
commit a1d710fa58
8 changed files with 59 additions and 59 deletions

View File

@ -11,7 +11,7 @@ use anyhow::Error;
use cpython::*;
use cpython_failure::{FallibleExt, ResultPyErrExt};
use dag::{
id::{GroupId, Id},
id::{Group, Id},
idmap::IdMap,
segment::Dag,
spanset::{SpanSet, SpanSetIter},
@ -181,7 +181,7 @@ py_class!(class dagindex |py| {
def build(&self, masternodes: Vec<PyBytes>, othernodes: Vec<PyBytes>, parentfunc: PyObject) -> PyResult<PyObject> {
let map = self.map(py).borrow();
// All nodes known and nothing needs to be built?
if masternodes.iter().all(|n| is_ok_some(map.find_id_by_slice_with_max_group(n.data(py), GroupId::MASTER)))
if masternodes.iter().all(|n| is_ok_some(map.find_id_by_slice_with_max_group(n.data(py), Group::MASTER)))
&& othernodes.iter().all(|n| is_ok_some(map.find_id_by_slice(n.data(py)))) {
return Ok(py.None());
}
@ -189,7 +189,7 @@ py_class!(class dagindex |py| {
let get_parents = translate_get_parents(py, parentfunc);
let mut map = self.map(py).borrow_mut();
let mut map = map.prepare_filesystem_sync().map_pyerr::<exc::IOError>(py)?;
for (nodes, group) in [(masternodes, GroupId::MASTER), (othernodes, GroupId::NON_MASTER)].iter() {
for (nodes, group) in [(masternodes, Group::MASTER), (othernodes, Group::NON_MASTER)].iter() {
for node in nodes {
let node = node.data(py);
map.assign_head(&node, &get_parents, *group).map_pyerr::<exc::RuntimeError>(py)?;
@ -201,7 +201,7 @@ py_class!(class dagindex |py| {
let mut dag = self.dag(py).borrow_mut();
use std::ops::DerefMut;
let mut syncable = dag.prepare_filesystem_sync().map_pyerr::<exc::IOError>(py)?;
for &group in GroupId::ALL.iter() {
for &group in Group::ALL.iter() {
let id = map.next_free_id(group).map_pyerr::<exc::IOError>(py)?;
if id > group.min_id() {
syncable.build_segments_persistent(id - 1, &get_parents).map_pyerr::<exc::IOError>(py)?;

View File

@ -6,7 +6,7 @@
*/
use anyhow::Result;
use dag::{idmap::IdMap, segment::Dag, spanset::SpanSet, GroupId, Id};
use dag::{idmap::IdMap, segment::Dag, spanset::SpanSet, Group, Id};
use minibench::{bench, elapsed};
use tempfile::tempdir;
@ -30,7 +30,7 @@ fn main() {
let id_map_dir = tempdir().unwrap();
let mut id_map = IdMap::open(id_map_dir.path()).unwrap();
id_map
.assign_head(&head_name, &parents_by_name, GroupId::MASTER)
.assign_head(&head_name, &parents_by_name, Group::MASTER)
.unwrap();
let head_id = id_map.find_id_by_slice(&head_name).unwrap().unwrap();

View File

@ -6,7 +6,7 @@
*/
use anyhow::Result;
use dag::{idmap::IdMap, segment::Dag, GroupId, Id};
use dag::{idmap::IdMap, segment::Dag, Group, Id};
use minibench::{
bench, elapsed,
measure::{self, Measure},
@ -33,7 +33,7 @@ fn main() {
let id_map_dir = tempdir().unwrap();
let mut id_map = IdMap::open(id_map_dir.path()).unwrap();
id_map
.assign_head(&head_name, &parents_by_name, GroupId::MASTER)
.assign_head(&head_name, &parents_by_name, Group::MASTER)
.unwrap();
let head_id = id_map.find_id_by_slice(&head_name).unwrap().unwrap();

View File

@ -24,9 +24,9 @@ pub struct Id(pub u64);
///
/// `(Group, Id)` are also topologically sorted.
#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub struct GroupId(pub(crate) usize);
pub struct Group(pub(crate) usize);
impl GroupId {
impl Group {
/// The "master" group. `ancestors(master)`.
/// - Expected to have most of the commits in a repo.
/// - Expected to be free from fragmentation. In other words,
@ -59,11 +59,11 @@ impl GroupId {
}
impl Id {
/// The [`GroupId`] of an Id.
pub fn group_id(self) -> GroupId {
let group = (self.0 >> (64 - GroupId::BITS)) as usize;
debug_assert!(group < GroupId::MAX);
GroupId(group)
/// The [`Group`] of an Id.
pub fn group(self) -> Group {
let group = (self.0 >> (64 - Group::BITS)) as usize;
debug_assert!(group < Group::MAX);
Group(group)
}
/// Similar to `self..=other`.
@ -99,8 +99,8 @@ impl Id {
impl fmt::Display for Id {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
let group = self.group_id();
if group == GroupId::NON_MASTER {
let group = self.group();
if group == Group::NON_MASTER {
write!(f, "N")?;
}
write!(f, "{}", self.0 - group.min_id().0)
@ -113,7 +113,7 @@ impl fmt::Debug for Id {
}
}
impl fmt::Display for GroupId {
impl fmt::Display for Group {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "{}", self.0)
}

View File

@ -9,7 +9,7 @@
//!
//! See [`IdMap`] for the main structure.
use crate::id::{GroupId, Id};
use crate::id::{Group, Id};
use anyhow::{bail, ensure, format_err, Result};
use byteorder::{BigEndian, ReadBytesExt, WriteBytesExt};
use fs2::FileExt;
@ -25,7 +25,7 @@ use std::sync::atomic::{self, AtomicU64};
pub struct IdMap {
log: log::Log,
path: PathBuf,
cached_next_free_ids: [AtomicU64; GroupId::MAX],
cached_next_free_ids: [AtomicU64; Group::MAX],
}
/// Guard to make sure [`IdMap`] on-disk writes are race-free.
@ -143,10 +143,10 @@ impl IdMap {
pub fn find_id_by_slice_with_max_group(
&self,
slice: &[u8],
max_group: GroupId,
max_group: Group,
) -> Result<Option<Id>> {
Ok(self.find_id_by_slice(slice)?.and_then(|id| {
if id.group_id() <= max_group {
if id.group() <= max_group {
Some(id)
} else {
None
@ -158,7 +158,7 @@ impl IdMap {
///
/// Errors if the new entry conflicts with existing entries.
pub fn insert(&mut self, id: Id, slice: &[u8]) -> Result<()> {
let group = id.group_id();
let group = id.group();
if id < self.next_free_id(group)? {
let existing_slice = self.find_slice_by_id(id)?;
if let Some(existing_slice) = existing_slice {
@ -184,7 +184,7 @@ impl IdMap {
// non-master groups.
if existing_id == id {
return Ok(());
} else if existing_id.group_id() <= group {
} else if existing_id.group() <= group {
bail!(
"logic error: new entry {} = {:?} conflicts with an existing entry {} = {:?}",
id,
@ -207,7 +207,7 @@ impl IdMap {
}
/// Return the next unused id in the given group.
pub fn next_free_id(&self, group: GroupId) -> Result<Id> {
pub fn next_free_id(&self, group: Group) -> Result<Id> {
let cached = self.cached_next_free_ids[group.0].load(atomic::Ordering::SeqCst);
let id = if cached == 0 {
let id = Self::get_next_free_id(&self.log, group)?;
@ -221,7 +221,7 @@ impl IdMap {
// Find an unused id that is bigger than existing ids.
// Used internally. It should match `next_free_id`.
fn get_next_free_id(log: &log::Log, group: GroupId) -> Result<Id> {
fn get_next_free_id(log: &log::Log, group: Group) -> Result<Id> {
// Checks should have been done at callsite.
let lower_bound_id = group.min_id();
let upper_bound_id = group.max_id();
@ -256,7 +256,7 @@ impl IdMap {
/// New `id`s inserted by this function will have the specified `group`.
/// Existing `id`s that are ancestors of `head` will get re-assigned
/// if they have a higher `group`.
pub fn assign_head<F>(&mut self, head: &[u8], parents_by_name: &F, group: GroupId) -> Result<Id>
pub fn assign_head<F>(&mut self, head: &[u8], parents_by_name: &F, group: Group) -> Result<Id>
where
F: Fn(&[u8]) -> Result<Vec<Box<[u8]>>>,
{
@ -456,18 +456,18 @@ mod tests {
let dir = tempdir().unwrap();
let mut map = IdMap::open(dir.path()).unwrap();
let mut map = map.prepare_filesystem_sync().unwrap();
assert_eq!(map.next_free_id(GroupId::MASTER).unwrap().0, 0);
assert_eq!(map.next_free_id(Group::MASTER).unwrap().0, 0);
map.insert(Id(1), b"abc").unwrap();
assert_eq!(map.next_free_id(GroupId::MASTER).unwrap().0, 2);
assert_eq!(map.next_free_id(Group::MASTER).unwrap().0, 2);
map.insert(Id(2), b"def").unwrap();
assert_eq!(map.next_free_id(GroupId::MASTER).unwrap().0, 3);
assert_eq!(map.next_free_id(Group::MASTER).unwrap().0, 3);
map.insert(Id(10), b"ghi").unwrap();
assert_eq!(map.next_free_id(GroupId::MASTER).unwrap().0, 11);
assert_eq!(map.next_free_id(Group::MASTER).unwrap().0, 11);
map.insert(Id(11), b"ghi").unwrap_err(); // ghi maps to 10
map.insert(Id(10), b"ghi2").unwrap_err(); // 10 maps to ghi
// Test another group.
let id = map.next_free_id(GroupId::NON_MASTER).unwrap();
let id = map.next_free_id(Group::NON_MASTER).unwrap();
map.insert(id, b"jkl").unwrap();
map.insert(id, b"jkl").unwrap();
map.insert(id, b"jkl2").unwrap_err(); // id maps to jkl
@ -475,7 +475,7 @@ mod tests {
map.insert(id + 2, b"jkl2").unwrap_err(); // jkl2 maps to id + 1
map.insert(Id(15), b"jkl2").unwrap(); // reassign jkl2 to master group - ok.
map.insert(id + 3, b"abc").unwrap_err(); // reassign abc to non-master group - error.
assert_eq!(map.next_free_id(GroupId::NON_MASTER).unwrap(), id + 2);
assert_eq!(map.next_free_id(Group::NON_MASTER).unwrap(), id + 2);
for _ in 0..=1 {
assert_eq!(map.find_slice_by_id(Id(1)).unwrap().unwrap(), b"abc");

View File

@ -17,7 +17,7 @@ pub mod protocol;
pub mod segment;
pub mod spanset;
pub use id::{GroupId, Id};
pub use id::{Group, Id};
pub use idmap::IdMap;
pub use segment::Dag;

View File

@ -14,7 +14,7 @@
//! have in-memory-only changes. [`SyncableDag`] is the only way to update
//! the filesystem state, and does not support queires.
use crate::id::{GroupId, Id};
use crate::id::{Group, Id};
use crate::spanset::Span;
use crate::spanset::SpanSet;
use anyhow::{bail, ensure, format_err, Result};
@ -188,7 +188,7 @@ impl Dag {
/// Return the next unused id for segments of the specified level.
///
/// Useful for building segments incrementally.
pub fn next_free_id(&self, level: Level, group: GroupId) -> Result<Id> {
pub fn next_free_id(&self, level: Level, group: Group) -> Result<Id> {
let lower_bound = group.min_id().to_prefixed_bytearray(level);
let upper_bound = group.max_id().to_prefixed_bytearray(level);
let range = &lower_bound[..]..=&upper_bound[..];
@ -296,7 +296,7 @@ impl Dag {
{
let mut count = 0;
count += self.build_flat_segments(high, get_parents, 0)?;
if self.next_free_id(0, high.group_id())? <= high {
if self.next_free_id(0, high.group())? <= high {
bail!("internal error: flat segments are not built as expected");
}
count += self.build_all_high_level_segments(false)?;
@ -323,12 +323,12 @@ impl Dag {
where
F: Fn(Id) -> Result<Vec<Id>>,
{
let group = high.group_id();
let group = high.group();
let low = self.next_free_id(0, group)?;
let mut current_low = None;
let mut current_parents = Vec::new();
let mut insert_count = 0;
let mut head_ids: HashSet<Id> = if group == GroupId::MASTER && low > Id(0) {
let mut head_ids: HashSet<Id> = if group == Group::MASTER && low > Id(0) {
self.heads(Id(0)..=(low - 1))?.iter().collect()
} else {
Default::default()
@ -338,7 +338,7 @@ impl Dag {
if parents.is_empty() {
flags |= SegmentFlags::HAS_ROOT
}
if group == GroupId::MASTER {
if group == Group::MASTER {
head_ids = &head_ids - &parents.iter().cloned().collect();
if head_ids.is_empty() {
flags |= SegmentFlags::ONLY_HEAD;
@ -377,7 +377,7 @@ impl Dag {
/// Find segments that covers `id..` range at the given level, within a same group.
fn next_segments(&self, id: Id, level: Level) -> Result<Vec<Segment>> {
let lower_bound = Self::serialize_head_level_lookup_key(id, level);
let upper_bound = Self::serialize_head_level_lookup_key(id.group_id().max_id(), level);
let upper_bound = Self::serialize_head_level_lookup_key(id.group().max_id(), level);
let mut result = Vec::new();
for entry in self
.log
@ -432,7 +432,7 @@ impl Dag {
let size = self.new_seg_size;
let mut insert_count = 0;
for &group in GroupId::ALL.iter() {
for &group in Group::ALL.iter() {
// `get_parents` is on the previous level of segments.
let get_parents = |head: Id| -> Result<Vec<Id>> {
if let Some(seg) = self.find_segment_by_head_and_level(head, level - 1)? {
@ -570,7 +570,7 @@ impl Dag {
/// Return a [`SpanSet`] that covers all ids stored in this [`Dag`].
pub fn all(&self) -> Result<SpanSet> {
let mut result = SpanSet::empty();
for &group in GroupId::ALL.iter().rev() {
for &group in Group::ALL.iter().rev() {
let next = self.next_free_id(0, group)?;
if next > group.min_id() {
result.push(group.min_id()..=(next - 1));
@ -581,7 +581,7 @@ impl Dag {
/// Return a [`SpanSet`] that covers all ids stored in the master group.
pub(crate) fn master_group(&self) -> Result<SpanSet> {
let group = GroupId::MASTER;
let group = Group::MASTER;
let next = self.next_free_id(0, group)?;
if next > group.min_id() {
Ok((group.min_id()..=(next - 1)).into())
@ -1213,7 +1213,7 @@ impl Dag {
}
}
}
let next_master = self.next_free_id(0, GroupId::MASTER)?;
let next_master = self.next_free_id(0, Group::MASTER)?;
if next_master.0 > 0 {
let master = next_master - 1;
let slice = full_idmap.slice(master)?;
@ -1502,25 +1502,25 @@ mod tests {
fn test_segment_basic_lookups() {
let dir = tempdir().unwrap();
let mut dag = Dag::open(dir.path()).unwrap();
assert_eq!(dag.next_free_id(0, GroupId::MASTER).unwrap().0, 0);
assert_eq!(dag.next_free_id(1, GroupId::MASTER).unwrap().0, 0);
assert_eq!(dag.next_free_id(0, Group::MASTER).unwrap().0, 0);
assert_eq!(dag.next_free_id(1, Group::MASTER).unwrap().0, 0);
let flags = SegmentFlags::empty();
dag.insert(flags, 0, Id(0), Id(50), &vec![]).unwrap();
assert_eq!(dag.next_free_id(0, GroupId::MASTER).unwrap().0, 51);
assert_eq!(dag.next_free_id(0, Group::MASTER).unwrap().0, 51);
dag.insert(flags, 0, Id(51), Id(100), &vec![Id(50)])
.unwrap();
assert_eq!(dag.next_free_id(0, GroupId::MASTER).unwrap().0, 101);
assert_eq!(dag.next_free_id(0, Group::MASTER).unwrap().0, 101);
dag.insert(flags, 0, Id(101), Id(150), &vec![Id(100)])
.unwrap();
assert_eq!(dag.next_free_id(0, GroupId::MASTER).unwrap().0, 151);
assert_eq!(dag.next_free_id(1, GroupId::MASTER).unwrap().0, 0);
assert_eq!(dag.next_free_id(0, Group::MASTER).unwrap().0, 151);
assert_eq!(dag.next_free_id(1, Group::MASTER).unwrap().0, 0);
dag.insert(flags, 1, Id(0), Id(100), &vec![]).unwrap();
assert_eq!(dag.next_free_id(1, GroupId::MASTER).unwrap().0, 101);
assert_eq!(dag.next_free_id(1, Group::MASTER).unwrap().0, 101);
dag.insert(flags, 1, Id(101), Id(150), &vec![Id(100)])
.unwrap();
assert_eq!(dag.next_free_id(1, GroupId::MASTER).unwrap().0, 151);
assert_eq!(dag.next_free_id(1, Group::MASTER).unwrap().0, 151);
// Helper functions to make the below lines shorter.
let low_by_head = |head, level| match dag.find_segment_by_head_and_level(Id(head), level) {
@ -1568,7 +1568,7 @@ mod tests {
fn test_sync_reload() {
let dir = tempdir().unwrap();
let mut dag = Dag::open(dir.path()).unwrap();
assert_eq!(dag.next_free_id(0, GroupId::MASTER).unwrap().0, 0);
assert_eq!(dag.next_free_id(0, Group::MASTER).unwrap().0, 0);
let mut syncable = dag.prepare_filesystem_sync().unwrap();
syncable

View File

@ -5,7 +5,7 @@
* GNU General Public License version 2.
*/
use crate::id::{GroupId, Id};
use crate::id::{Group, Id};
use crate::idmap::IdMap;
use crate::protocol::{Process, RequestLocationToSlice, RequestSliceToLocation};
use crate::segment::Dag;
@ -345,7 +345,7 @@ Lv3: R0-12[] N0-N8[1, 5]"#
// 'm' has 2 ids: 8 (master) and 5 (non-master).
assert_eq!(built.id_map.find_id_by_slice(b"m").unwrap().unwrap(), Id(8));
assert_eq!(built.id_map.find_slice_by_id(Id(8)).unwrap().unwrap(), b"m");
let id = GroupId::NON_MASTER.min_id() + 5;
let id = Group::NON_MASTER.min_id() + 5;
assert_eq!(built.id_map.find_slice_by_id(id).unwrap().unwrap(), b"m");
}
@ -740,7 +740,7 @@ impl IdMap {
/// Replace names in an ASCII DAG using the ids assigned.
fn replace(&self, text: &str) -> String {
let mut result = text.to_string();
for &group in GroupId::ALL.iter() {
for &group in Group::ALL.iter() {
for id in group.min_id().to(self.next_free_id(group).unwrap()) {
if let Ok(Some(name)) = self.find_slice_by_id(id) {
let name = String::from_utf8(name.to_vec()).unwrap();
@ -794,9 +794,9 @@ fn build_segments(text: &str, heads: &str, segment_size: usize) -> BuildSegmentR
.map(|head| {
// Assign to non-master if the name starts with a lowercase character.
let group = if head.chars().nth(0).unwrap().is_lowercase() {
GroupId::NON_MASTER
Group::NON_MASTER
} else {
GroupId::MASTER
Group::MASTER
};
let head = head.as_bytes();
id_map.assign_head(head, &parents_by_name, group).unwrap();