Fixed performance bug in AncestorNodeStream by changing how parents were fetched

Summary: Fixed a bug in which AncestorNodeStream was excessively slow if it were used in cache_warmup. Changing how a node hash was mapped to its parents made this a lot faster.

Reviewed By: farnz

Differential Revision: D8754990

fbshipit-source-id: bbd0cd97b516b7d3a5f9e048c7bdea0f24d6f7f3
This commit is contained in:
Matthew Dippel 2018-07-12 10:21:29 -07:00 committed by Facebook Github Bot
parent c6fd393842
commit 42e809a2fc
2 changed files with 13 additions and 24 deletions

View File

@ -23,12 +23,12 @@ use std::sync::Arc;
use blobrepo::BlobRepo;
use bookmarks::Bookmark;
use futures::{Future, IntoFuture, Stream};
use futures::future::{loop_fn, Loop};
use futures_ext::{BoxFuture, FutureExt};
use mercurial_types::{Changeset, HgChangesetId, MPath, RepoPath};
use mercurial_types::manifest::{Entry, Type};
use mercurial_types::manifest_utils::recursive_entry_stream;
use metaconfig::CacheWarmupParams;
use revset::AncestorsNodeStream;
use slog::Logger;
mod errors {
@ -93,23 +93,13 @@ fn changesets_warmup(
logger: Logger,
) -> BoxFuture<(), Error> {
info!(logger, "about to start warming up changesets cache");
let mut count = 0;
// TODO(stash): ancestor revset is surprisingly slow, we need to investigate it - T28905795
// For now use a simpler option.
loop_fn(start_rev, move |rev| {
count += 1;
if count >= cs_limit {
Ok(Loop::Break(())).into_future().boxify()
} else {
repo.get_changeset_parents(&rev)
.map(|parents| match parents.get(0) {
Some(p1) => Loop::Continue(*p1),
None => Loop::Break(()),
})
AncestorsNodeStream::new(&repo, start_rev.into_nodehash())
.take(cs_limit as u64)
.collect()
.map(|_| ())
.boxify()
}
}).boxify()
}
fn do_cache_warmup(
repo: Arc<BlobRepo>,

View File

@ -20,7 +20,7 @@ use futures::stream::{iter_ok, Stream};
use UniqueHeap;
use blobrepo::BlobRepo;
use mercurial_types::{Changeset, HgNodeHash};
use mercurial_types::HgNodeHash;
use mercurial_types::nodehash::HgChangesetId;
use mononoke_types::Generation;
@ -50,20 +50,19 @@ fn make_pending(
iter_ok::<_, Error>(hashes)
.map(move |hash| {
new_repo_changesets
.get_changeset_by_changesetid(&HgChangesetId::new(hash))
.map(|cs| cs.parents().clone())
.get_changeset_parents(&HgChangesetId::new(hash))
.map_err(|err| err.context(ErrorKind::ParentsFetchFailed).into())
})
.buffered(size)
.map(|parents| iter_ok::<_, Error>(parents.into_iter()))
.flatten()
.and_then(move |node_hash| {
.and_then(move |node_cs| {
new_repo_gennums
.get_generation_number(&HgChangesetId::new(node_hash))
.get_generation_number(&node_cs)
.and_then(move |genopt| {
genopt.ok_or_else(|| err_msg(format!("{} not found", node_hash)))
genopt.ok_or_else(|| err_msg(format!("{} not found", node_cs)))
})
.map(move |gen_id| (node_hash, gen_id))
.map(move |gen_id| (*node_cs.as_nodehash(), gen_id))
.map_err(|err| err.context(ErrorKind::GenerationFetchFailed).into())
}),
)