git: Load buffer git diff bases with a single batched git process (#59357)
# Objective Whenever the git repository state is updated on disk (e.g., via staging, committing, branch switching, or stashing), `reload_buffer_diff_bases` is scheduled to reload the diff for all active buffers. This causes 2 git processes to be spawned for each open file which can become noticeable when many files are open https://github.com/zed-industries/zed/blob/5e32405669d0470baec345073dafffb552a18bae/crates/project/src/git_store.rs#L5179 ## Solution This PR introduces `load_revisions` which uses a single `git cat-file --batch` command to compute the diff for all files in the same git process. This prevents the need to sequentially schedule 2 git subprocesses per open buffer. I also changed `load_index_text` and `load_commited_text` to rely on `load_revisions` which simplifies the code. ## Testing I added a unittest and manually verified that Zed now only runs a single `git cat-file --batch` command instead of 2 `git show` processes per open buffer. On macOS I viewed the currently running git processes using: ```shell sudo eslogger exec | jq --unbuffered -r ' select(.event.exec?.target?.executable?.path? | strings | contains("git")) | (.event.exec?.args? // []) | join(" ") ' ``` ## Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content adheres to Zed's UI standards ([UX/UI](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) and [icon](https://github.com/zed-industries/zed/blob/main/crates/icons/README.md) guidelines) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable /cc @Veykril Release Notes: - Reduced number of git processes for calculating diff of open buffers when the repo state changes on disk
This commit is contained in:
1 parent
d564495dde
commit
bc29bcfe72
3 files changed
+223
-86
No files matched your search
@@ -162,28 +162,6 @@ impl FakeGitRepository {
|
||||
}
|
||||
|
||||
impl GitRepository for FakeGitRepository {
|
||||
fn load_index_text(&self, path: RepoPath) -> BoxFuture<'_, Option<String>> {
|
||||
let fut = self.with_state_async(false, move |state| {
|
||||
state
|
||||
.index_contents
|
||||
.get(&path)
|
||||
.context("not present in index")
|
||||
.cloned()
|
||||
});
|
||||
self.executor.spawn(async move { fut.await.ok() }).boxed()
|
||||
}
|
||||
|
||||
fn load_committed_text(&self, path: RepoPath) -> BoxFuture<'_, Option<String>> {
|
||||
let fut = self.with_state_async(false, move |state| {
|
||||
state
|
||||
.head_contents
|
||||
.get(&path)
|
||||
.context("not present in HEAD")
|
||||
.cloned()
|
||||
});
|
||||
self.executor.spawn(async move { fut.await.ok() }).boxed()
|
||||
}
|
||||
|
||||
fn load_commit_template(&self) -> BoxFuture<'_, Result<Option<GitCommitTemplate>>> {
|
||||
async { Ok(None) }.boxed()
|
||||
}
|
||||
@@ -264,6 +242,24 @@ impl GitRepository for FakeGitRepository {
|
||||
})
|
||||
}
|
||||
|
||||
fn load_revisions(&self, revisions: Vec<String>) -> BoxFuture<'_, Result<Vec<Option<String>>>> {
|
||||
let fut = self.with_state_async(false, move |state| {
|
||||
Ok(revisions
|
||||
.into_iter()
|
||||
.map(|rev| {
|
||||
let (prefix, path) = rev.split_once(':')?;
|
||||
let repo_path = RepoPath::new(path).ok()?;
|
||||
match prefix {
|
||||
"" => state.index_contents.get(&repo_path).cloned(),
|
||||
"HEAD" => state.head_contents.get(&repo_path).cloned(),
|
||||
_ => None,
|
||||
}
|
||||
})
|
||||
.collect())
|
||||
});
|
||||
self.executor.spawn(fut).boxed()
|
||||
}
|
||||
|
||||
fn show(&self, commit: String) -> BoxFuture<'_, Result<CommitDetails>> {
|
||||
self.with_state_async(false, move |state| {
|
||||
let sha = state.refs.get(&commit).cloned().unwrap_or(commit);
|
||||
|
||||
+169
-43
@@ -801,12 +801,18 @@ pub trait GitRepository: Send + Sync {
|
||||
/// Returns the contents of an entry in the repository's index, or None if there is no entry for the given path.
|
||||
///
|
||||
/// Also returns `None` for symlinks.
|
||||
fn load_index_text(&self, path: RepoPath) -> BoxFuture<'_, Option<String>>;
|
||||
fn load_index_text(&self, path: RepoPath) -> BoxFuture<'_, Option<String>> {
|
||||
let future = self.load_revisions(vec![format!(":{}", path.as_unix_str())]);
|
||||
async move { future.await.ok()?.pop()? }.boxed()
|
||||
}
|
||||
|
||||
/// Returns the contents of an entry in the repository's HEAD, or None if HEAD does not exist or has no entry for the given path.
|
||||
///
|
||||
/// Also returns `None` for symlinks.
|
||||
fn load_committed_text(&self, path: RepoPath) -> BoxFuture<'_, Option<String>>;
|
||||
fn load_committed_text(&self, path: RepoPath) -> BoxFuture<'_, Option<String>> {
|
||||
let future = self.load_revisions(vec![format!("HEAD:{}", path.as_unix_str())]);
|
||||
async move { future.await.ok()?.pop()? }.boxed()
|
||||
}
|
||||
fn load_blob_content(&self, oid: Oid) -> BoxFuture<'_, Result<String>>;
|
||||
|
||||
fn set_index_text(
|
||||
@@ -830,6 +836,8 @@ pub trait GitRepository: Send + Sync {
|
||||
/// Resolve a list of refs to SHAs.
|
||||
fn revparse_batch(&self, revs: Vec<String>) -> BoxFuture<'_, Result<Vec<Option<String>>>>;
|
||||
|
||||
fn load_revisions(&self, revisions: Vec<String>) -> BoxFuture<'_, Result<Vec<Option<String>>>>;
|
||||
|
||||
fn head_sha(&self) -> BoxFuture<'_, Option<String>> {
|
||||
async move {
|
||||
self.revparse_batch(vec!["HEAD".into()])
|
||||
@@ -1585,47 +1593,6 @@ impl GitRepository for RealGitRepository {
|
||||
.boxed()
|
||||
}
|
||||
|
||||
fn load_index_text(&self, path: RepoPath) -> BoxFuture<'_, Option<String>> {
|
||||
let git_binary = self.git_binary();
|
||||
let path_str = format!(":{}", path.as_unix_str());
|
||||
self.executor
|
||||
.spawn(async move {
|
||||
let git = git_binary;
|
||||
let output = git
|
||||
.build_command(&["show", &path_str])
|
||||
.stdout(Stdio::piped())
|
||||
.stderr(Stdio::piped())
|
||||
.output()
|
||||
.await
|
||||
.log_err()?;
|
||||
if !output.status.success() {
|
||||
return None;
|
||||
}
|
||||
String::from_utf8(output.stdout).ok()
|
||||
})
|
||||
.boxed()
|
||||
}
|
||||
|
||||
fn load_committed_text(&self, path: RepoPath) -> BoxFuture<'_, Option<String>> {
|
||||
let git = self.git_binary();
|
||||
let path_str = format!("HEAD:{}", path.as_unix_str());
|
||||
self.executor
|
||||
.spawn(async move {
|
||||
let output = git
|
||||
.build_command(&["show", &path_str])
|
||||
.stdout(Stdio::piped())
|
||||
.stderr(Stdio::piped())
|
||||
.output()
|
||||
.await
|
||||
.log_err()?;
|
||||
if !output.status.success() {
|
||||
return None;
|
||||
}
|
||||
String::from_utf8(output.stdout).ok()
|
||||
})
|
||||
.boxed()
|
||||
}
|
||||
|
||||
fn load_blob_content(&self, oid: Oid) -> BoxFuture<'_, Result<String>> {
|
||||
let git_binary = self.git_binary();
|
||||
let oid_str = oid.to_string();
|
||||
@@ -1801,6 +1768,68 @@ impl GitRepository for RealGitRepository {
|
||||
.boxed()
|
||||
}
|
||||
|
||||
fn load_revisions(&self, revisions: Vec<String>) -> BoxFuture<'_, Result<Vec<Option<String>>>> {
|
||||
let git = self.git_binary();
|
||||
self.executor
|
||||
.spawn(async move {
|
||||
if revisions.is_empty() {
|
||||
return Ok(Vec::new());
|
||||
}
|
||||
|
||||
let mut process = git
|
||||
.build_command(&["cat-file", "--batch"])
|
||||
.stdin(Stdio::piped())
|
||||
.stdout(Stdio::piped())
|
||||
.stderr(Stdio::piped())
|
||||
.kill_on_drop(true)
|
||||
.spawn()?;
|
||||
|
||||
let mut stdin = BufWriter::new(process.stdin.take().context("no stdin")?);
|
||||
let mut stdout = BufReader::new(process.stdout.take().context("no stdout")?);
|
||||
let mut newline = [0u8; 1];
|
||||
|
||||
let mut header_bytes = Vec::new();
|
||||
let mut results = Vec::with_capacity(revisions.len());
|
||||
for rev in &revisions {
|
||||
stdin.write_all(rev.as_bytes()).await?;
|
||||
stdin.write_all(b"\n").await?;
|
||||
stdin.flush().await?;
|
||||
|
||||
header_bytes.clear();
|
||||
stdout.read_until(b'\n', &mut header_bytes).await?;
|
||||
let header_line = String::from_utf8_lossy(&header_bytes);
|
||||
|
||||
let parts: Vec<&str> = header_line.trim().split(' ').collect();
|
||||
match parts[..] {
|
||||
[.., "missing"] => {
|
||||
results.push(None);
|
||||
}
|
||||
[_, object_type, size_str] => {
|
||||
let size: usize = size_str
|
||||
.parse()
|
||||
.with_context(|| format!("invalid object size: {size_str}"))?;
|
||||
|
||||
let mut content = vec![0u8; size];
|
||||
stdout.read_exact(&mut content).await?;
|
||||
stdout.read_exact(&mut newline).await?;
|
||||
|
||||
if object_type == "blob" {
|
||||
results.push(String::from_utf8(content).ok());
|
||||
} else {
|
||||
results.push(None);
|
||||
}
|
||||
}
|
||||
_ => bail!("invalid cat-file header: {header_line}"),
|
||||
}
|
||||
}
|
||||
|
||||
drop(stdin);
|
||||
process.output().await?;
|
||||
Ok(results)
|
||||
})
|
||||
.boxed()
|
||||
}
|
||||
|
||||
fn merge_message(&self) -> BoxFuture<'_, Option<String>> {
|
||||
let path = self.path().join("MERGE_MSG");
|
||||
self.executor
|
||||
@@ -4709,6 +4738,103 @@ mod tests {
|
||||
// );
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_load_revisions(cx: &mut TestAppContext) {
|
||||
disable_git_global_config();
|
||||
cx.executor().allow_parking();
|
||||
|
||||
let repo_dir = tempfile::tempdir().unwrap();
|
||||
git_init_repo(repo_dir.path());
|
||||
|
||||
let file1_path = repo_dir.path().join("file1");
|
||||
let file2_path = repo_dir.path().join("file2");
|
||||
let space_file_path = repo_dir.path().join("file with spaces");
|
||||
|
||||
smol::fs::write(&file1_path, "file1 committed contents")
|
||||
.await
|
||||
.unwrap();
|
||||
smol::fs::write(&file2_path, "file2 committed contents")
|
||||
.await
|
||||
.unwrap();
|
||||
smol::fs::write(&space_file_path, "space file committed contents")
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let repo = RealGitRepository::new(
|
||||
&repo_dir.path().join(".git"),
|
||||
None,
|
||||
Some("git".into()),
|
||||
cx.executor(),
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
// Stage files and commit
|
||||
repo.stage_paths(
|
||||
vec![
|
||||
repo_path("file1"),
|
||||
repo_path("file2"),
|
||||
repo_path("file with spaces"),
|
||||
],
|
||||
Arc::new(HashMap::default()),
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
repo.commit(
|
||||
"Initial commit".into(),
|
||||
None,
|
||||
CommitOptions::default(),
|
||||
AskPassDelegate::new(&mut cx.to_async(), |_, _, _| {}),
|
||||
Arc::new(test_commit_envs()),
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
// Now modify files in index but not yet committed
|
||||
smol::fs::write(&file1_path, "file1 index contents")
|
||||
.await
|
||||
.unwrap();
|
||||
repo.stage_paths(vec![repo_path("file1")], Arc::new(HashMap::default()))
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
// Write working tree contents (not indexed, not committed)
|
||||
smol::fs::write(&file1_path, "file1 worktree contents")
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
// Now test load_revisions
|
||||
let results = repo
|
||||
.load_revisions(
|
||||
[
|
||||
"HEAD:file1",
|
||||
":file1",
|
||||
"HEAD:file2",
|
||||
":file2",
|
||||
"HEAD:nonexistent",
|
||||
"HEAD:file with spaces",
|
||||
"HEAD:nonexistent file with spaces",
|
||||
]
|
||||
.into_iter()
|
||||
.map(String::from)
|
||||
.collect(),
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(
|
||||
results,
|
||||
vec![
|
||||
Some("file1 committed contents".into()),
|
||||
Some("file1 index contents".into()),
|
||||
Some("file2 committed contents".into()),
|
||||
Some("file2 committed contents".into()), // untouched in index, should match HEAD
|
||||
None,
|
||||
Some("space file committed contents".into()),
|
||||
None,
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_checkpoint_empty_repo(cx: &mut TestAppContext) {
|
||||
disable_git_global_config();
|
||||
|
||||
@@ -5670,26 +5670,36 @@ impl Repository {
|
||||
|
||||
let buffer_diff_base_changes = cx
|
||||
.background_spawn(async move {
|
||||
let mut changes = Vec::new();
|
||||
for (
|
||||
buffer,
|
||||
repo_path,
|
||||
is_symlink,
|
||||
current_index_text,
|
||||
current_head_text,
|
||||
) in &repo_diff_state_updates
|
||||
let mut revisions = Vec::new();
|
||||
for (_, repo_path, is_symlink, current_index_text, current_head_text) in
|
||||
&repo_diff_state_updates
|
||||
{
|
||||
let index_text = if current_index_text.is_some() && !*is_symlink {
|
||||
backend.load_index_text(repo_path.clone())
|
||||
} else {
|
||||
future::ready(None).boxed()
|
||||
};
|
||||
let head_text = if current_head_text.is_some() && !*is_symlink {
|
||||
backend.load_committed_text(repo_path.clone())
|
||||
} else {
|
||||
future::ready(None).boxed()
|
||||
};
|
||||
let (index_text, head_text) = future::join(index_text, head_text).await;
|
||||
if current_index_text.is_some() && !*is_symlink {
|
||||
revisions.push(format!(":{}", repo_path.as_unix_str()));
|
||||
}
|
||||
if current_head_text.is_some() && !*is_symlink {
|
||||
revisions.push(format!("HEAD:{}", repo_path.as_unix_str()));
|
||||
}
|
||||
}
|
||||
|
||||
let mut loaded_revisions = backend
|
||||
.load_revisions(revisions)
|
||||
.await
|
||||
.log_err()
|
||||
.into_iter()
|
||||
.flatten();
|
||||
|
||||
let mut changes = Vec::new();
|
||||
for (buffer, _, is_symlink, current_index_text, current_head_text) in
|
||||
&repo_diff_state_updates
|
||||
{
|
||||
let index_text = (current_index_text.is_some() && !*is_symlink)
|
||||
.then(|| loaded_revisions.next().flatten())
|
||||
.flatten();
|
||||
|
||||
let head_text = (current_head_text.is_some() && !*is_symlink)
|
||||
.then(|| loaded_revisions.next().flatten())
|
||||
.flatten();
|
||||
|
||||
let change =
|
||||
match (current_index_text.as_ref(), current_head_text.as_ref()) {
|
||||
@@ -8996,8 +9006,13 @@ impl Repository {
|
||||
let rx = self.send_job("load_committed_text", None, move |state, _| async move {
|
||||
match state {
|
||||
RepositoryState::Local(LocalRepositoryState { backend, .. }) => {
|
||||
let committed_text = backend.load_committed_text(repo_path.clone()).await;
|
||||
let staged_text = backend.load_index_text(repo_path).await;
|
||||
let revisions = vec![
|
||||
format!("HEAD:{}", repo_path.as_unix_str()),
|
||||
format!(":{}", repo_path.as_unix_str()),
|
||||
];
|
||||
let mut loaded_revisions = backend.load_revisions(revisions).await?.into_iter();
|
||||
let committed_text = loaded_revisions.next().flatten();
|
||||
let staged_text = loaded_revisions.next().flatten();
|
||||
let diff_bases_change = if committed_text == staged_text {
|
||||
DiffBasesChange::SetBoth(committed_text)
|
||||
} else {
|
||||
|
||||
Reference in new issue
Block a user