Skip to content

Commit

Permalink
Memoize lfs check which was very expensive. (#53)
Browse files Browse the repository at this point in the history
The LFS check should only be performed when necessary since it hits the
file system and we should memorize the result so we don't keep repinging
it.
  • Loading branch information
EliSchleifer authored Nov 11, 2024
1 parent c9cf33e commit 4185c74
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 22 deletions.
52 changes: 35 additions & 17 deletions src/git.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
use chrono::{DateTime, FixedOffset};
use git2::{AttrCheckFlags, AttrValue, Delta, DiffOptions, Repository};
use lazy_static::lazy_static;
use std::collections::HashMap;
use std::path::{Path, PathBuf};
use std::process::Command;
use std::process::Output as ProcessOutput;

use std::sync::Mutex;
#[derive(Debug, Clone)]
pub struct Hunk {
pub path: PathBuf,
Expand Down Expand Up @@ -52,17 +53,34 @@ impl Output {
}
}
}
lazy_static! {
static ref LFS_CACHE: Mutex<HashMap<String, bool>> = Mutex::new(HashMap::new());
}

fn is_lfs(repo: &Repository, path: &Path) -> bool {
let path_str = path.to_string_lossy().to_string();

// Check the cache first
if let Some(&cached_result) = LFS_CACHE.lock().unwrap().get(&path_str) {
return cached_result;
}

// "filter" is the primary LFS attribute, see gitattributes(5)
// FILE_THEN_INDEX checks working tree then index; mimics git itself
// https://github.com/libgit2/libgit2/blob/v1.5.0/include/git2/attr.h#L104-L116
if let Ok(filter_bytes) = repo.get_attr_bytes(path, "filter", AttrCheckFlags::FILE_THEN_INDEX) {
let result = if let Ok(filter_bytes) =
repo.get_attr_bytes(path, "filter", AttrCheckFlags::FILE_THEN_INDEX)
{
let filter = AttrValue::from_bytes(filter_bytes);
filter.eq(&AttrValue::from_string(Some("lfs")))
} else {
false
}
};

// Store the result in the cache
LFS_CACHE.lock().unwrap().insert(path_str, result);

result
}

pub fn modified_since(upstream: &str, repo_path: Option<&Path>) -> anyhow::Result<FileChanges> {
Expand Down Expand Up @@ -106,7 +124,7 @@ pub fn modified_since(upstream: &str, repo_path: Option<&Path>) -> anyhow::Resul
let mut ret = FileChanges::default();
let mut maybe_current_hunk: Option<Hunk> = None;
diff.foreach(
&mut |delta, _| {
&mut |delta: git2::DiffDelta<'_>, _| {
if let Some(path) = delta.new_file().path() {
if !is_lfs(&repo, path) {
match delta.status() {
Expand All @@ -132,13 +150,13 @@ pub fn modified_since(upstream: &str, repo_path: Option<&Path>) -> anyhow::Resul
None,
Some(&mut |delta, _, line| {
if let Some(path) = delta.new_file().path() {
if !is_lfs(&repo, path) {
match delta.status() {
Delta::Added
| Delta::Copied
| Delta::Untracked
| Delta::Modified
| Delta::Renamed => {
match delta.status() {
Delta::Added
| Delta::Copied
| Delta::Untracked
| Delta::Modified
| Delta::Renamed => {
if !is_lfs(&repo, path) {
if let Some(new_lineno) = line.new_lineno() {
if line.old_lineno().is_none() {
maybe_current_hunk = maybe_current_hunk
Expand All @@ -161,13 +179,13 @@ pub fn modified_since(upstream: &str, repo_path: Option<&Path>) -> anyhow::Resul
}
}
}
Delta::Unmodified
| Delta::Deleted
| Delta::Ignored
| Delta::Typechange
| Delta::Unreadable
| Delta::Conflicted => (),
}
Delta::Unmodified
| Delta::Deleted
| Delta::Ignored
| Delta::Typechange
| Delta::Unreadable
| Delta::Conflicted => (),
}
}
true
Expand Down
14 changes: 9 additions & 5 deletions src/rules/if_change_then_change.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
use crate::run::Run;
use anyhow::Context;
use log::{debug, trace};
use log::{debug, trace, warn};
use rayon::iter::{IntoParallelRefIterator, ParallelIterator};
use regex::Regex;
use std::collections::{HashMap, HashSet};
use std::fs::File;
use std::io::{BufRead, BufReader};
use std::path::Path;
use std::path::PathBuf;

use rayon::iter::{IntoParallelRefIterator, ParallelIterator};
use regex::Regex;

use crate::diagnostic;
use crate::git;

Expand Down Expand Up @@ -54,7 +53,12 @@ pub fn find_ictc_blocks(path: &PathBuf) -> anyhow::Result<Vec<IctcBlock>> {

trace!("scanning contents of {}", path.display());

let in_file = File::open(path).with_context(|| format!("failed to open: {:#?}", path))?;
let in_file = File::open(path).with_context(|| {
let error_message = format!("failed to open: {:#?}", path);
warn!("{}", error_message);
error_message
})?;

let in_buf = BufReader::new(in_file);

let mut block: Option<IctcBlock> = None;
Expand Down

0 comments on commit 4185c74

Please sign in to comment.