Skip to content

Commit d9b39f5

Browse files
committed
cleanup: Avoid N git commands for building UI buffer
Per #17 (comment) this seems to be one reason why Limmat can be unresponsive when running on big repos, especially with crippled Git. I don't think this will make the issue go away but it does seem worth avoiding zillions of git commands anyway, this probably has some tangible benefit even if the system is architected to isolate the performance impact of those commands.
1 parent cf37993 commit d9b39f5

File tree

2 files changed

+70
-33
lines changed

2 files changed

+70
-33
lines changed

src/git.rs

Lines changed: 1 addition & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use core::fmt;
22
use core::fmt::{Debug, Display};
33
use std::ffi::{OsStr, OsString};
44
use std::ops::Deref;
5-
use std::os::unix::ffi::{OsStrExt as _, OsStringExt as _};
5+
use std::os::unix::ffi::OsStrExt as _;
66
use std::path::{Path, PathBuf};
77
use std::pin::pin;
88
use std::process::Command as SyncCommand;
@@ -189,7 +189,6 @@ impl From<Commit> for CommitHash {
189189

190190
pub enum LogStyle {
191191
WithGraph,
192-
#[expect(dead_code)]
193192
NoGraph,
194193
}
195194

@@ -316,27 +315,6 @@ pub trait Worktree: Debug {
316315
Ok(stdout)
317316
}
318317

319-
async fn log_n1<S, T>(&self, rev_spec: S, format_spec: T) -> anyhow::Result<OsString>
320-
where
321-
S: AsRef<OsStr>,
322-
T: AsRef<OsStr>,
323-
{
324-
let mut format_arg = OsString::from("--format=");
325-
format_arg.push(format_spec.as_ref());
326-
let stdout = self
327-
.git(["log", "-n1"])
328-
.args([&format_arg, rev_spec.as_ref()])
329-
.execute()
330-
.await
331-
.context(format!(
332-
"getting -n1 log for {:?} with format {:?}",
333-
rev_spec.as_ref(),
334-
format_spec.as_ref(),
335-
))?
336-
.stdout;
337-
Ok(OsString::from_vec(stdout))
338-
}
339-
340318
// Watch for events that could change the meaning of a revspec. When that happens, send an event
341319
// on the channel with the new resolved spec.
342320
fn watch_refs<'a>(

src/ui.rs

Lines changed: 69 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,10 @@ use std::{collections::HashMap, ffi::OsStr, io::Write, mem, sync::Arc};
33
use ansi_control_codes::control_sequences::{CUP, ED};
44
use anyhow::{self, bail, Context as _};
55
use colored::Colorize;
6+
use futures::future::try_join;
67
use lazy_static::lazy_static;
8+
#[allow(unused_imports)]
9+
use log::debug;
710
use regex::Regex;
811

912
use crate::{
@@ -207,6 +210,60 @@ impl GraphBuffer {
207210
}
208211
}
209212

213+
// Helper for OutputBuffer - a way to grab log info for a bunch of commits with
214+
// a single git command. It's important that we don't do N git commands, that
215+
// can really slow things down when the range is large.
216+
217+
struct CommitInfoBuffer {
218+
raw_buf: String, // Output straight from Git.
219+
}
220+
221+
impl CommitInfoBuffer {
222+
// log_format must not contain %x00 as that's used internally for splitting
223+
// up the raw buffer.
224+
pub async fn new(
225+
repo: &Arc<impl Worktree>,
226+
range_spec: impl AsRef<OsStr>,
227+
log_format: &str,
228+
) -> anyhow::Result<Self> {
229+
if log_format.contains("%x00") {
230+
bail!("NUL bytes not allowed in log format");
231+
}
232+
let raw_buf = repo
233+
.log(
234+
range_spec.as_ref(),
235+
format!("%H {log_format}%x00"),
236+
LogStyle::NoGraph,
237+
)
238+
.await?;
239+
// Hack: OsStr doesn't have a proper API, so just squash to utf-8, sorry
240+
// users.
241+
Ok(Self {
242+
raw_buf: String::from_utf8_lossy(&raw_buf).to_string(),
243+
})
244+
}
245+
246+
pub fn info(&self) -> anyhow::Result<HashMap<CommitHash, &str>> {
247+
let b = self.raw_buf.trim();
248+
let b = b.strip_suffix('\0').unwrap_or(b);
249+
if b.is_empty() {
250+
return Ok(HashMap::new());
251+
}
252+
b.split('\0')
253+
.map(|raw_chunk| {
254+
let [hash, chunk] = raw_chunk
255+
.splitn(2, ' ')
256+
.collect::<Vec<_>>()
257+
.try_into()
258+
.map_err(|v: Vec<_>| {
259+
anyhow::anyhow!("expected chunk split len 2, got {}", v.len())
260+
})?;
261+
Ok((CommitHash::new(hash.trim()), chunk))
262+
})
263+
.collect()
264+
}
265+
}
266+
210267
// Represents the buffer showing the current status of all the commits being tested.
211268
struct OutputBuffer {
212269
// Pre-rendered lines containing static information (graph, commit log info etc).
@@ -254,22 +311,24 @@ impl OutputBuffer {
254311
// buffer pairwise. If it has more lines then we will need to stretch
255312
// out the graph vertically to make space first.
256313

314+
let (graph_buf, info_buf) = try_join(
315+
GraphBuffer::new(repo, range_spec.as_ref()),
316+
CommitInfoBuffer::new(repo, range_spec.as_ref(), log_format),
317+
)
318+
.await?;
319+
320+
let commit_info = info_buf.info()?;
257321
let mut lines = Vec::new();
258322
let mut status_commits = HashMap::new();
259-
let graph_buf = GraphBuffer::new(repo, range_spec).await?;
260323
for (hash, mut chunk) in graph_buf.chunks()? {
261-
let log_n1_os = repo
262-
.log_n1(&hash, log_format)
263-
.await
264-
.context(format!("couldn't get commit data for {:?}", hash))?;
265-
// Hack: because OsStr doesn't have a proper API, luckily we can
266-
// just squash to utf-8, sorry users.
267-
let log_n1 = log_n1_os.to_string_lossy();
324+
let log_info = commit_info
325+
.get(&hash)
326+
.with_context(|| format!("missing commit info for {:?}", hash))?;
268327

269328
// We're gonna add our own newlines in so we don't need the one that
270329
// Git printed.
271-
let log_n1 = log_n1.strip_suffix('\n').unwrap_or(&log_n1);
272-
let mut info_lines: Vec<&str> = log_n1.split('\n').collect();
330+
let log_info = log_info.strip_suffix('\n').unwrap_or(log_info);
331+
let mut info_lines: Vec<&str> = log_info.split('\n').collect();
273332

274333
// Here's where we'll inject the live status
275334
status_commits.insert(lines.len() + info_lines.len(), hash);

0 commit comments

Comments
 (0)