D7717: rust-discovery: restoring add_missing cheap early return
gracinet (Georges Racinet)
phabricator at mercurial-scm.org
Tue Dec 24 13:50:09 UTC 2019
gracinet created this revision.
Herald added subscribers: mercurial-devel, mjpieters, kevincox, durin42.
Herald added a reviewer: hg-reviewers.
REVISION SUMMARY
In case the iterator of missing revisions argument turns out
to be empty, we need to refrain from computing the undecided
set. This way, we'll benefit from subsequent add_common_revisions
until there is an actually meaningful add_missing_revisions.
To do this with the typestate pattern, that means the early
return has to happen before potential transition into an
WithUndecided
REPOSITORY
rHG Mercurial
BRANCH
default
REVISION DETAIL
https://phab.mercurial-scm.org/D7717
AFFECTED FILES
rust/hg-core/src/discovery.rs
CHANGE DETAILS
diff --git a/rust/hg-core/src/discovery.rs b/rust/hg-core/src/discovery.rs
--- a/rust/hg-core/src/discovery.rs
+++ b/rust/hg-core/src/discovery.rs
@@ -256,6 +256,11 @@
&mut self,
missing: impl IntoIterator<Item = Revision>,
) -> Result<(), GraphError> {
+ let missing: VecDeque<Revision> = missing.into_iter().collect();
+ if missing.is_empty() {
+ return Ok(());
+ }
+
self.mutate_undecided(
|oc| oc.compute_undecided(),
|wu| wu.add_missing_revisions(missing),
@@ -431,20 +436,14 @@
///
/// # Performance note
///
- /// Except in the most trivial case, the first call of this method has
- /// the side effect of computing `self.undecided` set for the first time,
- /// and the related caches it might need for efficiency of its internal
- /// computation. This is typically faster if more information is
- /// available in `self.common`. Therefore, for good performance, the
- /// caller should avoid calling this too early.
+ /// `tovisit` is provided as a `VecDeque` already so that a potential
+ /// caller from `PartialDiscovery` can avoid the undecided set in case
+ /// it turns out to be empty (hence restrain
+ /// from performing the transition to `WithUndecided`)
pub fn add_missing_revisions(
&mut self,
- missing: impl IntoIterator<Item = Revision>,
+ mut tovisit: VecDeque<Revision>,
) -> Result<(), GraphError> {
- let mut tovisit: VecDeque<Revision> = missing.into_iter().collect();
- if tovisit.is_empty() {
- return Ok(());
- }
self.ensure_children_cache()?;
let children = self.children_cache.as_ref().unwrap();
let mut seen: HashSet<Revision> = HashSet::new();
@@ -773,7 +772,7 @@
// of `undecided`, let's check that, force the mutation and
// ask for them
assert_disco_is_only_commons(&disco);
- disco.add_missing_revisions(Vec::new())?;
+ disco.mutate_undecided(|oc| oc.compute_undecided(), |_| Ok(()))?;
assert_disco_is_b(&disco);
assert_eq!(sorted_undecided(&disco), vec![5, 8, 10, 13]);
assert_eq!(disco.stats().undecided, Some(4));
To: gracinet, #hg-reviewers
Cc: durin42, kevincox, mjpieters, mercurial-devel
More information about the Mercurial-devel
mailing list