Skip to content

remove itertools #1294

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Feb 16, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 2 additions & 3 deletions gitoxide-core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ default = []
## Discover all git repositories within a directory. Particularly useful with [skim](https://github.com/lotabout/skim).
organize = ["dep:gix-url", "dep:jwalk"]
## Derive the amount of time invested into a git repository akin to [git-hours](https://github.com/kimmobrunfeldt/git-hours).
estimate-hours = ["dep:itertools", "dep:fs-err", "dep:crossbeam-channel", "dep:smallvec"]
estimate-hours = ["dep:fs-err", "dep:crossbeam-channel", "dep:smallvec"]
## Gather information about repositories and store it in a database for easy querying.
query = ["dep:rusqlite"]
## Run algorithms on a corpus of repositories and store their results for later comparison and intelligence gathering.
Expand All @@ -29,7 +29,7 @@ corpus = [ "dep:rusqlite", "dep:sysinfo", "organize", "dep:crossbeam-channel", "
archive = ["dep:gix-archive-for-configuration-only", "gix/worktree-archive"]

## The ability to clean a repository, similar to `git clean`.
clean = [ "gix/dirwalk" ]
clean = ["gix/dirwalk"]

#! ### Mutually Exclusive Networking
#! If both are set, _blocking-client_ will take precedence, allowing `--all-features` to be used.
Expand Down Expand Up @@ -72,7 +72,6 @@ gix-url = { version = "^0.27.0", path = "../gix-url", optional = true }
jwalk = { version = "0.8.0", optional = true }

# for 'hours'
itertools = { version = "0.12.0", optional = true }
fs-err = { version = "2.6.0", optional = true }
crossbeam-channel = { version = "0.5.6", optional = true }
smallvec = { version = "1.10.0", optional = true }
Expand Down
23 changes: 15 additions & 8 deletions gitoxide-core/src/hours/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ use std::{
};

use gix::bstr::BStr;
use itertools::Itertools;

use crate::hours::{
util::{add_lines, remove_lines},
Expand All @@ -25,17 +24,25 @@ pub fn estimate_hours(
const MAX_COMMIT_DIFFERENCE_IN_MINUTES: f32 = 2.0 * MINUTES_PER_HOUR;
const FIRST_COMMIT_ADDITION_IN_MINUTES: f32 = 2.0 * MINUTES_PER_HOUR;

let hours_for_commits = commits.iter().map(|t| &t.1).rev().tuple_windows().fold(
0_f32,
|hours, (cur, next): (&gix::actor::SignatureRef<'_>, &gix::actor::SignatureRef<'_>)| {
let hours_for_commits = {
let mut hours = 0.0;

let mut commits = commits.iter().map(|t| &t.1).rev();
let mut cur = commits.next().expect("not a single commit found");

for next in commits {
let change_in_minutes = (next.time.seconds.saturating_sub(cur.time.seconds)) as f32 / MINUTES_PER_HOUR;
if change_in_minutes < MAX_COMMIT_DIFFERENCE_IN_MINUTES {
hours + change_in_minutes / MINUTES_PER_HOUR
hours += change_in_minutes / MINUTES_PER_HOUR
} else {
hours + (FIRST_COMMIT_ADDITION_IN_MINUTES / MINUTES_PER_HOUR)
hours += FIRST_COMMIT_ADDITION_IN_MINUTES / MINUTES_PER_HOUR
}
},
);

cur = next;
}

hours
};

let author = &commits[0].1;
let (files, lines) = (!stats.is_empty())
Expand Down
30 changes: 26 additions & 4 deletions gitoxide-core/src/hours/util.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use std::sync::atomic::{AtomicUsize, Ordering};

use gix::bstr::{BStr, ByteSlice};
use itertools::Itertools;

use crate::hours::core::HOURS_PER_WORKDAY;

Expand Down Expand Up @@ -43,6 +42,29 @@ impl<'a> From<&'a WorkByEmail> for WorkByPerson {
}
}

fn join<I>(mut iter: I, sep: &str) -> String
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO you can avoid the String creation by turning this into a struct, with iter and sep stored in it.

Then impl fmt::Display on it, and you will achieve zero-allocation joining.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's fine to allocate here, it's similar to what was available before with std::slice::join().

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took it from here https://github.com/rust-itertools/itertools/blob/762643f1be2217140a972745cf4d6ed69435f722/src/lib.rs#L2295-L2324

I also think its fine as is but one could certainly think of ways to improve it. The function could also be made non generic and just accept a slice as that would have less indirections in this case but I figured the way it is now it is easier to move it to utils in case it happens to be needed somewhere else.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, if you find the time, maybe even in the other PR, could you add a note to say where it's from?
I'd like to keep track of any code that isn't specifically written for gitoxide to be sure I can comply with licensing terms.
Thanks in advance.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah sure 👍

where
I: Iterator,
<I as Iterator>::Item: std::fmt::Display,
{
use ::std::fmt::Write;

match iter.next() {
None => String::new(),
Some(first_elt) => {
// estimate lower bound of capacity needed
let (lower, _) = iter.size_hint();
let mut result = String::with_capacity(sep.len() * lower);
write!(&mut result, "{first_elt}").unwrap();
iter.for_each(|elt| {
result.push_str(sep);
write!(&mut result, "{elt}").unwrap();
});
result
}
}
}

impl WorkByPerson {
pub fn write_to(
&self,
Expand All @@ -53,9 +75,9 @@ impl WorkByPerson {
) -> std::io::Result<()> {
writeln!(
out,
"{} <{}>",
self.name.iter().join(", "),
self.email.iter().join(", ")
"{names} <{mails}>",
names = join(self.name.iter(), ", "),
mails = join(self.email.iter(), ", ")
)?;
writeln!(out, "{} commits found", self.num_commits)?;
writeln!(
Expand Down