Skip to content

Commit

Permalink
Reduce amount of max candidates, add --debug flag (#298)
Browse files Browse the repository at this point in the history
  • Loading branch information
Byron committed Apr 5, 2022
1 parent 324a839 commit c8c13e3
Show file tree
Hide file tree
Showing 7 changed files with 66 additions and 13 deletions.
31 changes: 29 additions & 2 deletions git-repository/src/commit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,22 @@ pub mod describe {
use git_odb::FindExt;
use std::borrow::Cow;

/// The result of [try_resolve()][Platform::try_resolve()].
pub struct Resolution<'repo> {
/// The outcome of the describe operation.
pub outcome: git_revision::describe::Outcome<'static>,
/// The id to describe.
pub id: crate::Id<'repo>,
}

impl<'repo> Resolution<'repo> {
/// Turn this instance into something displayable
pub fn format(self) -> Result<git_revision::describe::Format<'static>, Error> {
let prefix = self.id.shorten()?;
Ok(self.outcome.into_format(prefix.hex_len()))
}
}

/// The error returned by [try_format()][Platform::try_format()].
#[derive(Debug, thiserror::Error)]
#[allow(missing_docs)]
Expand Down Expand Up @@ -126,6 +142,14 @@ pub mod describe {
///
/// Note that there will always be `Some(format)`
pub fn try_format(self) -> Result<Option<git_revision::describe::Format<'static>>, Error> {
Ok(self.try_resolve()?.map(|r| r.format()).transpose()?)
}

/// Try to find a name for the configured commit id using all prior configuration, returning `Some(Outcome)`
/// if one was found.
///
/// The outcome provides additional information, but leaves the caller with the burden
pub fn try_resolve(self) -> Result<Option<crate::commit::describe::Resolution<'repo>>, Error> {
// TODO: dirty suffix with respective dirty-detection
let outcome = git_revision::describe(
&self.id,
Expand All @@ -137,8 +161,11 @@ pub mod describe {
..Default::default()
},
)?;
let prefix = self.id.attach(self.repo).shorten()?;
Ok(outcome.map(|o| o.into_format(prefix.hex_len())))

Ok(outcome.map(|outcome| crate::commit::describe::Resolution {
outcome,
id: self.id.attach(self.repo),
}))
}

/// Like [`try_format()`][Platform::try_format()], but turns `id_as_fallback()` on to always produce a format.
Expand Down
22 changes: 15 additions & 7 deletions git-revision/src/describe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ pub struct Outcome<'name> {
pub depth: u32,
/// The mapping between object ids and their names initially provided by the describe call.
pub name_by_oid: hash_hasher::HashedMap<git_hash::ObjectId, Cow<'name, BStr>>,
/// The amount of commits we traversed.
pub commits_seen: u32,
}

impl<'a> Outcome<'a> {
Expand Down Expand Up @@ -115,7 +117,7 @@ pub struct Options<'name> {
impl<'name> Default for Options<'name> {
fn default() -> Self {
Options {
max_candidates: MAX_CANDIDATES,
max_candidates: 28, // the same number as git uses, otherwise we perform worse by default on big repos
name_by_oid: Default::default(),
fallback_to_oid: false,
first_parent: false,
Expand Down Expand Up @@ -182,6 +184,7 @@ pub(crate) mod function {
id: commit.to_owned(),
depth: 0,
name_by_oid,
commits_seen: 0,
}));
}
max_candidates = max_candidates.min(MAX_CANDIDATES);
Expand All @@ -192,19 +195,19 @@ pub(crate) mod function {

let mut queue = VecDeque::from_iter(Some(commit.to_owned()));
let mut candidates = Vec::new();
let mut seen_commits = 0;
let mut commits_seen = 0;
let mut gave_up_on_commit = None;
let mut seen = hash_hasher::HashedMap::default();
seen.insert(commit.to_owned(), 0u32);

while let Some(commit) = queue.pop_front() {
seen_commits += 1;
commits_seen += 1;
if let Some(name) = name_by_oid.get(&commit) {
if candidates.len() < max_candidates {
let identity_bit = 1 << candidates.len();
candidates.push(Candidate {
name: name.clone(),
commits_in_its_future: seen_commits - 1,
commits_in_its_future: commits_seen - 1,
identity_bit,
order: candidates.len(),
});
Expand Down Expand Up @@ -266,6 +269,7 @@ pub(crate) mod function {
name: None,
name_by_oid,
depth: 0,
commits_seen,
}))
} else {
Ok(None)
Expand All @@ -280,9 +284,10 @@ pub(crate) mod function {

if let Some(commit_id) = gave_up_on_commit {
queue.push_front(commit_id);
commits_seen -= 1;
}

finish_depth_computation(
commits_seen += finish_depth_computation(
queue,
find,
candidates.first_mut().expect("at least one candidate"),
Expand All @@ -298,6 +303,7 @@ pub(crate) mod function {
id: commit.to_owned(),
depth: c.commits_in_its_future,
name_by_oid,
commits_seen,
}))
}

Expand Down Expand Up @@ -368,12 +374,14 @@ pub(crate) mod function {
mut parent_buf: Vec<u8>,
mut parents: Vec<(git_hash::ObjectId, Flags)>,
first_parent: bool,
) -> Result<(), Error<E>>
) -> Result<u32, Error<E>>
where
Find: for<'b> FnMut(&oid, &'b mut Vec<u8>) -> Result<CommitRefIter<'b>, E>,
E: std::error::Error + Send + Sync + 'static,
{
let mut commits_seen = 0;
while let Some(commit) = queue.pop_front() {
commits_seen += 1;
let flags = seen[&commit];
if (flags & best_candidate.identity_bit) == best_candidate.identity_bit {
if queue
Expand All @@ -398,7 +406,7 @@ pub(crate) mod function {
first_parent,
)?;
}
Ok(())
Ok(commits_seen)
}

#[derive(Debug)]
Expand Down
2 changes: 1 addition & 1 deletion git-revision/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
//!
//! One can also describe revisions using a different algorithm.
#![forbid(unsafe_code, rust_2018_idioms)]
#[deny(missing_docs)]
#![deny(missing_docs)]

/// Access to collections optimized for keys that are already a hash.
pub use hash_hasher;
Expand Down
2 changes: 2 additions & 0 deletions git-revision/tests/describe/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ fn exact_match_with_dirty_and_long() {
id: hex_to_id("b920bbb055e1efb9080592a409d3975738b6efb3"),
depth: 0,
name_by_oid: Default::default(),
commits_seen: 0,
}
.into_format(7);
assert!(format.is_exact_match());
Expand Down Expand Up @@ -38,6 +39,7 @@ fn show_abbrev_hash_if_no_name_is_known() {
id: hex_to_id("b920bbb055e1efb9080592a409d3975738b6efb3"),
depth: 0,
name_by_oid: Default::default(),
commits_seen: 0,
}
.into_format(7);
assert!(
Expand Down
12 changes: 10 additions & 2 deletions gitoxide-core/src/repository/commit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,13 @@ pub fn describe(
repo: impl Into<PathBuf>,
rev_spec: Option<&str>,
mut out: impl std::io::Write,
mut err: impl std::io::Write,
describe::Options {
all_tags,
all_refs,
first_parent,
always,
statistics,
long_format,
}: describe::Options,
) -> Result<()> {
Expand All @@ -27,14 +29,19 @@ pub fn describe(
} else {
Default::default()
};
let mut describe_id = commit
let resolution = commit
.describe()
.names(select_ref)
.traverse_first_parent(first_parent)
.id_as_fallback(always)
.try_format()?
.try_resolve()?
.with_context(|| format!("Did not find a single candidate ref for naming id '{}'", commit.id))?;

if statistics {
writeln!(err, "traversed {} commits", resolution.outcome.commits_seen)?;
}

let mut describe_id = resolution.format()?;
describe_id.long(long_format);

writeln!(out, "{}", describe_id)?;
Expand All @@ -49,5 +56,6 @@ pub mod describe {
pub first_parent: bool,
pub always: bool,
pub long_format: bool,
pub statistics: bool,
}
}
6 changes: 5 additions & 1 deletion src/plumbing/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use std::{

use anyhow::Result;
use clap::Parser;

use gitoxide_core as core;
use gitoxide_core::pack::verify;

Expand Down Expand Up @@ -163,23 +164,26 @@ pub fn main() -> Result<()> {
first_parent,
always,
long,
statistics,
rev_spec,
} => prepare_and_run(
"repository-commit-describe",
verbose,
progress,
progress_keep_open,
None,
move |_progress, out, _err| {
move |_progress, out, err| {
core::repository::commit::describe(
repository,
rev_spec.as_deref(),
out,
err,
core::repository::commit::describe::Options {
all_tags,
all_refs,
long_format: long,
first_parent,
statistics,
always,
},
)
Expand Down
4 changes: 4 additions & 0 deletions src/plumbing/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -411,6 +411,10 @@ pub mod repo {
#[clap(long, short = 'l')]
long: bool,

/// Print information on stderr to inform about performance statistics
#[clap(long, short = 's')]
statistics: bool,

#[clap(long)]
/// If there was no way to describe the commit, fallback to using the abbreviated input revision.
always: bool,
Expand Down

0 comments on commit c8c13e3

Please sign in to comment.