( )⚙ D11398 rust: Add Repo::dirstate_map and use it in `rhg status`

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D11398

rust: Add Repo::dirstate_map and use it in `rhg status`
ClosedPublic

Authored by SimonSapin on Sep 10 2021, 11:11 AM.

Details

Reviewers
Alphare
Group Reviewers
hg-reviewers
Commits
rHG81aedf1fc897: rust: Add Repo::dirstate_map and use it in `rhg status`
Summary

This moves low-level dirstate wrangling out of the status command and into
a more reusable location.

The open dirstate map is lazily initialized and kept on the Repo object,
for reuse by sub-sequent calls.

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

SimonSapin created this revision.Sep 10 2021, 11:11 AM
Alphare accepted this revision.Sep 13 2021, 6:38 AM
Alphare added a subscriber: Alphare.

Cool ! That should speed up some operations, especially with more complex commands.

rust/hg-core/src/repo.rs
270

Should we set the parents to DirstateParents::NULL here?

311

I had no idea you could re-assign to a dropped variable, but I guess it makes sense.

This revision is now accepted and ready to land.Sep 13 2021, 6:38 AM
SimonSapin updated this revision to Diff 30208.Sep 13 2021, 6:41 AM
SimonSapin added inline comments.Sep 13 2021, 6:44 AM
rust/hg-core/src/repo.rs
270

Indeed! Done. (Though it’s not critical, it just saves the dirstate_parents getter from opening on trying to open the .hg/dirstate file again.)

Revision Contents
Changeset List

Diff 30208

rust/hg-core/src/dirstate.rs

use bytes_cast::{unaligned, BytesCast}; use bytes_cast::{unaligned, BytesCast};
use std::convert::TryFrom; use std::convert::TryFrom;
pub mod dirs_multiset; pub mod dirs_multiset;
pub mod dirstate_map; pub mod dirstate_map;
pub mod parsers; pub mod parsers;
pub mod status; pub mod status;
#[derive(Debug, PartialEq, Clone, BytesCast)] #[derive(Debug, PartialEq, Copy, Clone, BytesCast)]
#[repr(C)] #[repr(C)]
pub struct DirstateParents { pub struct DirstateParents {
pub p1: Node, pub p1: Node,
pub p2: Node, pub p2: Node,
} }
impl DirstateParents { impl DirstateParents {
pub const NULL: Self = Self { pub const NULL: Self = Self {

rust/hg-core/src/repo.rs

use crate::config::{Config, ConfigError, ConfigParseError}; use crate::config::{Config, ConfigError, ConfigParseError};
use crate::dirstate::DirstateParents;
use crate::dirstate_tree::dirstate_map::DirstateMap;
use crate::dirstate_tree::owning::OwningDirstateMap;
use crate::errors::HgError; use crate::errors::HgError;
use crate::errors::HgResultExt;
use crate::exit_codes; use crate::exit_codes;
use crate::requirements; use crate::requirements;
use crate::utils::files::get_path_from_bytes; use crate::utils::files::get_path_from_bytes;
use crate::utils::SliceExt; use crate::utils::SliceExt;
use crate::vfs::{is_dir, is_file, Vfs}; use crate::vfs::{is_dir, is_file, Vfs};
use crate::DirstateError;
use std::cell::{Cell, Ref, RefCell, RefMut};
use std::collections::HashSet; use std::collections::HashSet;
use std::path::{Path, PathBuf}; use std::path::{Path, PathBuf};
/// A repository on disk /// A repository on disk
pub struct Repo { pub struct Repo {
working_directory: PathBuf, working_directory: PathBuf,
dot_hg: PathBuf, dot_hg: PathBuf,
store: PathBuf, store: PathBuf,
requirements: HashSet<String>, requirements: HashSet<String>,
config: Config, config: Config,
// None means not known/initialized yet
dirstate_parents: Cell<Option<DirstateParents>>,
dirstate_map: RefCell<Option<OwningDirstateMap>>,
} }
#[derive(Debug, derive_more::From)] #[derive(Debug, derive_more::From)]
pub enum RepoError { pub enum RepoError {
NotFound { NotFound {
at: PathBuf, at: PathBuf,
}, },
#[from] #[from]
}; };
let repo = Self { let repo = Self {
requirements: reqs, requirements: reqs,
working_directory, working_directory,
store: store_path, store: store_path,
dot_hg, dot_hg,
config: repo_config, config: repo_config,
dirstate_parents: Cell::new(None),
dirstate_map: RefCell::new(None),
}; };
requirements::check(&repo)?; requirements::check(&repo)?;
Ok(repo) Ok(repo)
} }
pub fn working_directory_path(&self) -> &Path { pub fn working_directory_path(&self) -> &Path {
} }
} }
pub fn has_dirstate_v2(&self) -> bool { pub fn has_dirstate_v2(&self) -> bool {
self.requirements self.requirements
.contains(requirements::DIRSTATE_V2_REQUIREMENT) .contains(requirements::DIRSTATE_V2_REQUIREMENT)
} }
pub fn dirstate_parents( fn dirstate_file_contents(&self) -> Result<Vec<u8>, HgError> {
&self, Ok(self
) -> Result<crate::dirstate::DirstateParents, HgError> { .hg_vfs()
let dirstate = self.hg_vfs().mmap_open("dirstate")?; .read("dirstate")
if dirstate.is_empty() { .io_not_found_as_none()?
return Ok(crate::dirstate::DirstateParents::NULL); .unwrap_or(Vec::new()))
} }
let parents = if self.has_dirstate_v2() {
pub fn dirstate_parents(&self) -> Result<DirstateParents, HgError> {
if let Some(parents) = self.dirstate_parents.get() {
return Ok(parents);
}
let dirstate = self.dirstate_file_contents()?;
let parents = if dirstate.is_empty() {
DirstateParents::NULL
} else if self.has_dirstate_v2() {
crate::dirstate_tree::on_disk::read_docket(&dirstate)?.parents() crate::dirstate_tree::on_disk::read_docket(&dirstate)?.parents()
} else { } else {
crate::dirstate::parsers::parse_dirstate_parents(&dirstate)? crate::dirstate::parsers::parse_dirstate_parents(&dirstate)?
.clone() .clone()
}; };
self.dirstate_parents.set(Some(parents));
Ok(parents) Ok(parents)
} }
fn new_dirstate_map(&self) -> Result<OwningDirstateMap, DirstateError> {
let dirstate_file_contents = self.dirstate_file_contents()?;
if dirstate_file_contents.is_empty() {
self.dirstate_parents.set(Some(DirstateParents::NULL));
AlphareUnsubmitted
Not Done

Should we set the parents to DirstateParents::NULL here?

Alphare: Should we set the parents to `DirstateParents::NULL` here?
SimonSapinAuthorUnsubmitted
Done

Indeed! Done. (Though it’s not critical, it just saves the dirstate_parents getter from opening on trying to open the .hg/dirstate file again.)

SimonSapin: Indeed! Done. (Though it’s not critical, it just saves the `dirstate_parents` getter from…
Ok(OwningDirstateMap::new_empty(Vec::new()))
} else if self.has_dirstate_v2() {
let docket = crate::dirstate_tree::on_disk::read_docket(
&dirstate_file_contents,
)?;
self.dirstate_parents.set(Some(docket.parents()));
let data_size = docket.data_size();
let metadata = docket.tree_metadata();
let mut map = if let Some(data_mmap) = self
.hg_vfs()
.mmap_open(docket.data_filename())
.io_not_found_as_none()?
{
OwningDirstateMap::new_empty(MmapWrapper(data_mmap))
} else {
OwningDirstateMap::new_empty(Vec::new())
};
let (on_disk, placeholder) = map.get_mut_pair();
*placeholder = DirstateMap::new_v2(on_disk, data_size, metadata)?;
Ok(map)
} else {
let mut map = OwningDirstateMap::new_empty(dirstate_file_contents);
let (on_disk, placeholder) = map.get_mut_pair();
let (inner, parents) = DirstateMap::new_v1(on_disk)?;
self.dirstate_parents
.set(Some(parents.unwrap_or(DirstateParents::NULL)));
*placeholder = inner;
Ok(map)
}
}
pub fn dirstate_map(
&self,
) -> Result<Ref<OwningDirstateMap>, DirstateError> {
let mut borrowed = self.dirstate_map.borrow();
if borrowed.is_none() {
drop(borrowed);
// Only use `borrow_mut` if it is really needed to avoid panic in
// case there is another outstanding borrow but mutation is not
// needed.
*self.dirstate_map.borrow_mut() = Some(self.new_dirstate_map()?);
AlphareUnsubmitted
Not Done

I had no idea you could re-assign to a dropped variable, but I guess it makes sense.

Alphare: I had no idea you could re-assign to a dropped variable, but I guess it makes sense.
borrowed = self.dirstate_map.borrow()
}
Ok(Ref::map(borrowed, |option| option.as_ref().unwrap()))
} }
pub fn dirstate_map_mut(
&self,
) -> Result<RefMut<OwningDirstateMap>, DirstateError> {
let mut borrowed = self.dirstate_map.borrow_mut();
if borrowed.is_none() {
*borrowed = Some(self.new_dirstate_map()?);
}
Ok(RefMut::map(borrowed, |option| option.as_mut().unwrap()))
}
}
// TODO: remove this when https://github.com/RazrFalcon/memmap2-rs/pull/22 is on crates.io
struct MmapWrapper(memmap2::Mmap);
impl std::ops::Deref for MmapWrapper {
type Target = [u8];
fn deref(&self) -> &[u8] {
self.0.deref()
}
}
unsafe impl stable_deref_trait::StableDeref for MmapWrapper {}

rust/rhg/src/commands/status.rs

// status.rs // status.rs
// //
// Copyright 2020, Georges Racinet <georges.racinets@octobus.net> // Copyright 2020, Georges Racinet <georges.racinets@octobus.net>
// //
// This software may be used and distributed according to the terms of the // This software may be used and distributed according to the terms of the
// GNU General Public License version 2 or any later version. // GNU General Public License version 2 or any later version.
use crate::error::CommandError; use crate::error::CommandError;
use crate::ui::Ui; use crate::ui::Ui;
use clap::{Arg, SubCommand}; use clap::{Arg, SubCommand};
use hg; use hg;
use hg::dirstate_tree::dirstate_map::DirstateMap; use hg::dirstate_tree::dispatch::DirstateMapMethods;
use hg::dirstate_tree::on_disk;
use hg::errors::HgResultExt;
use hg::errors::IoResultExt; use hg::errors::IoResultExt;
use hg::matchers::AlwaysMatcher; use hg::matchers::AlwaysMatcher;
use hg::operations::cat; use hg::operations::cat;
use hg::repo::Repo; use hg::repo::Repo;
use hg::revlog::node::Node; use hg::revlog::node::Node;
use hg::utils::hg_path::{hg_path_to_os_string, HgPath}; use hg::utils::hg_path::{hg_path_to_os_string, HgPath};
use hg::StatusError; use hg::StatusError;
use hg::{HgPathCow, StatusOptions}; use hg::{HgPathCow, StatusOptions};
if requested.is_empty() { if requested.is_empty() {
DEFAULT_DISPLAY_STATES DEFAULT_DISPLAY_STATES
} else { } else {
requested requested
} }
}; };
let repo = invocation.repo?; let repo = invocation.repo?;
let dirstate_data_mmap; let mut dmap = repo.dirstate_map_mut()?;
let (mut dmap, parents) = if repo.has_dirstate_v2() {
let docket_data =
repo.hg_vfs().read("dirstate").io_not_found_as_none()?;
let parents;
let dirstate_data;
let data_size;
let docket;
let tree_metadata;
if let Some(docket_data) = &docket_data {
docket = on_disk::read_docket(docket_data)?;
tree_metadata = docket.tree_metadata();
parents = Some(docket.parents());
data_size = docket.data_size();
dirstate_data_mmap = repo
.hg_vfs()
.mmap_open(docket.data_filename())
.io_not_found_as_none()?;
dirstate_data = dirstate_data_mmap.as_deref().unwrap_or(b"");
} else {
parents = None;
tree_metadata = b"";
data_size = 0;
dirstate_data = b"";
}
let dmap =
DirstateMap::new_v2(dirstate_data, data_size, tree_metadata)?;
(dmap, parents)
} else {
dirstate_data_mmap =
repo.hg_vfs().mmap_open("dirstate").io_not_found_as_none()?;
let dirstate_data = dirstate_data_mmap.as_deref().unwrap_or(b"");
DirstateMap::new_v1(dirstate_data)?
};
let options = StatusOptions { let options = StatusOptions {
// TODO should be provided by the dirstate parsing and // TODO should be provided by the dirstate parsing and
// hence be stored on dmap. Using a value that assumes we aren't // hence be stored on dmap. Using a value that assumes we aren't
// below the time resolution granularity of the FS and the // below the time resolution granularity of the FS and the
// dirstate. // dirstate.
last_normal_time: 0, last_normal_time: 0,
// we're currently supporting file systems with exec flags only // we're currently supporting file systems with exec flags only
// anyway // anyway
check_exec: true, check_exec: true,
list_clean: display_states.clean, list_clean: display_states.clean,
list_unknown: display_states.unknown, list_unknown: display_states.unknown,
list_ignored: display_states.ignored, list_ignored: display_states.ignored,
collect_traversed_dirs: false, collect_traversed_dirs: false,
}; };
let ignore_file = repo.working_directory_vfs().join(".hgignore"); // TODO hardcoded let ignore_file = repo.working_directory_vfs().join(".hgignore"); // TODO hardcoded
let (mut ds_status, pattern_warnings) = hg::dirstate_tree::status::status( let (mut ds_status, pattern_warnings) = dmap.status(
&mut dmap,
&AlwaysMatcher, &AlwaysMatcher,
repo.working_directory_path().to_owned(), repo.working_directory_path().to_owned(),
vec![ignore_file], vec![ignore_file],
options, options,
)?; )?;
if !pattern_warnings.is_empty() { if !pattern_warnings.is_empty() {
warn!("Pattern warnings: {:?}", &pattern_warnings); warn!("Pattern warnings: {:?}", &pattern_warnings);
} }
if !ds_status.bad.is_empty() { if !ds_status.bad.is_empty() {
warn!("Bad matches {:?}", &(ds_status.bad)) warn!("Bad matches {:?}", &(ds_status.bad))
} }
if !ds_status.unsure.is_empty() { if !ds_status.unsure.is_empty() {
info!( info!(
"Files to be rechecked by retrieval from filelog: {:?}", "Files to be rechecked by retrieval from filelog: {:?}",
&ds_status.unsure &ds_status.unsure
); );
} }
if !ds_status.unsure.is_empty() if !ds_status.unsure.is_empty()
&& (display_states.modified || display_states.clean) && (display_states.modified || display_states.clean)
{ {
let p1: Node = parents let p1: Node = repo.dirstate_parents()?.p1.into();
.expect(
"Dirstate with no parents should not list any file to
be rechecked for modifications",
)
.p1
.into();
let p1_hex = format!("{:x}", p1); let p1_hex = format!("{:x}", p1);
for to_check in ds_status.unsure { for to_check in ds_status.unsure {
if cat_file_is_modified(repo, &to_check, &p1_hex)? { if cat_file_is_modified(repo, &to_check, &p1_hex)? {
if display_states.modified { if display_states.modified {
ds_status.modified.push(to_check); ds_status.modified.push(to_check);
} }
} else { } else {
if display_states.clean { if display_states.clean {