Merge pull request 'Add visibleParentResolver and optimize Git file status lookup' (#42) from performance into main

scripts/graph-parents.test.mjs

@@ -0,0 +1,51 @@
+import assert from 'node:assert/strict';
+import { readFileSync } from 'node:fs';
+import test from 'node:test';
+import ts from 'typescript';
+const source = readFileSync(new URL('../src/lib/graphParents.ts', import.meta.url), 'utf8');
+const compiled = ts.transpileModule(source, { compilerOptions: { module: ts.ModuleKind.ESNext, target: ts.ScriptTarget.ES2022 } }).outputText;
+const { visibleParentResolver } = await import(`data:text/javascript;base64,${Buffer.from(compiled).toString('base64')}`);
+function oldResolver(hash, visible, commits, seen = new Set()) {
+  if (visible.has(hash)) return [hash];
+  if (seen.has(hash)) return [];
+  seen.add(hash);
+  const commit = commits.get(hash);
+  return commit ? [...new Set(commit.parents.flatMap(parent => oldResolver(parent, visible, commits, new Set(seen))))] : [];
+}
+test('preserves parent order and deduplicates converging paths', () => {
+  const items = [{hash:'tip',parents:['a','b']},{hash:'a',parents:['x','y']},{hash:'b',parents:['y','z']}];
+  const resolve = visibleParentResolver(items,new Set(['x','y','z']));
+  assert.deepEqual(resolve('tip'),['x','y','z']);
+  assert.deepEqual(resolve('missing'),[]);
+  assert.deepEqual(resolve('x'),['x']);
+});
+test('matches previous traversal across deterministic merge DAGs and visibility filters', () => {
+  let seed=42;
+  const random=()=>((seed=(Math.imul(seed,1664525)+1013904223)>>>0)/2**32);
+  for(let run=0;run<80;run++) {
+    const items=Array.from({length:40},(_,i)=>({hash:String(i),parents:i===39?[]:[String(i+1),...(random()<.5?[String(i+1+Math.floor(random()*(39-i)))]:[])]}));
+    const visible=new Set(items.filter(()=>random()<.35).map(x=>x.hash));
+    const resolve=visibleParentResolver(items,visible);
+    const map=new Map(items.map(x=>[x.hash,x]));
+    for(const item of items) assert.deepEqual(resolve(item.hash),oldResolver(item.hash,visible,map));
+  }
+});
+test('handles 20000 hidden ancestors without overflowing the call stack', () => {
+  const items=Array.from({length:20000},(_,i)=>({hash:String(i),parents:[String(i+1)]}));
+  assert.deepEqual(visibleParentResolver(items,new Set(['20000']))('0'),['20000']);
+});
+test('shared merge ancestry is expanded only once', () => {
+  let reads=0;
+  const items=Array.from({length:30},(_,i)=>({hash:String(i),get parents(){reads++;return i===29?['root']:[String(i+1),String(Math.min(i+2,29))];}}));
+  const resolve=visibleParentResolver(items,new Set(['root']));
+  assert.deepEqual(resolve('0'),['root']);
+  const initial=reads;
+  assert.deepEqual(resolve('1'),['root']);
+  assert.equal(reads,initial);
+  assert.ok(reads<300);
+});
+const items=Array.from({length:24},(_,i)=>({hash:String(i),parents:i===23?['root']:[String(i+1),String(Math.min(i+2,23))]}));
+const visible=new Set(['root']);const map=new Map(items.map(x=>[x.hash,x]));
+const before=performance.now();oldResolver('0',visible,map);const oldMs=performance.now()-before;
+const after=performance.now();visibleParentResolver(items,visible)('0');const newMs=performance.now()-after;
+console.log(`Synthetic shared-ancestry benchmark (24 commits): old ${oldMs.toFixed(2)} ms, new ${newMs.toFixed(2)} ms`);

src-tauri/src/git.rs

@@ -5756,6 +5756,7 @@ fn repository_files_with_status(
     repo: &Path,
     status: &GitStatus,
 ) -> Result<Vec<GitRepositoryFile>, String> {
+    let status_index = file_status_index(&status.files);
     let mut files = BTreeMap::<String, GitRepositoryFile>::new();
 
     let tracked_output = run_git(repo, ["ls-files", "-z", "-t", "--cached", "--deleted"])?;
@@ -5772,7 +5773,7 @@ fn repository_files_with_status(
             continue;
         }
         let path = String::from_utf8_lossy(path_bytes).into_owned();
-        let status = status_for_file(&status.files, &path);
+        let status = status_index.get(path.as_str()).copied().flatten();
         files.insert(
             path.clone(),
             GitRepositoryFile {
@@ -5785,7 +5786,7 @@ fn repository_files_with_status(
 
     let untracked_output = run_git(repo, ["ls-files", "-z", "--others", "--exclude-standard"])?;
     for path in parse_nul_paths(&untracked_output) {
-        let status = status_for_file(&status.files, &path).or(Some(FileStatusKind::Untracked));
+        let status = status_index.get(path.as_str()).copied().flatten().or(Some(FileStatusKind::Untracked));
         files.insert(
             path.clone(),
             GitRepositoryFile {
@@ -6151,16 +6152,23 @@ fn parse_nul_paths(output: &[u8]) -> Vec<String> {
         .collect()
 }
 
-fn status_for_file(statuses: &[GitFileStatus], path: &str) -> Option<FileStatusKind> {
-    let status = find_status(statuses, path)?;
-
-    if matches!(status.staged, Some(FileStatusKind::Conflicted))
-        || matches!(status.unstaged, Some(FileStatusKind::Conflicted))
-    {
-        return Some(FileStatusKind::Conflicted);
+fn file_status_index(statuses: &[GitFileStatus]) -> std::collections::HashMap<&str, Option<FileStatusKind>> {
+    let mut index = std::collections::HashMap::with_capacity(statuses.len());
+    for status in statuses {
+        let kind = if matches!(status.staged, Some(FileStatusKind::Conflicted))
+            || matches!(status.unstaged, Some(FileStatusKind::Conflicted))
+        {
+            Some(FileStatusKind::Conflicted)
+        } else {
+            status.unstaged.or(status.staged)
+        };
+        // Match find_status: the first record wins, including rename aliases.
+        index.entry(status.path.as_str()).or_insert(kind);
+        if let Some(old_path) = status.old_path.as_deref() {
+            index.entry(old_path).or_insert(kind);
+        }
     }
-
-    status.unstaged.or(status.staged)
+    index
 }
 
 fn has_unresolved_conflicts(status: &GitStatus) -> bool {
@@ -10115,6 +10123,22 @@ mod tests {
         assert_eq!(docs.replace("\r\n", "\n"), "docs2\n");
     }
 
+    #[test]
+    fn file_status_index_preserves_rename_conflict_and_first_match_semantics() {
+        let statuses = vec![
+            GitFileStatus { path: "new".into(), old_path: Some("old".into()), staged: Some(FileStatusKind::Renamed), unstaged: Some(FileStatusKind::Modified) },
+            GitFileStatus { path: "old".into(), old_path: None, staged: Some(FileStatusKind::Added), unstaged: None },
+            GitFileStatus { path: "conflict".into(), old_path: None, staged: Some(FileStatusKind::Conflicted), unstaged: Some(FileStatusKind::Modified) },
+            GitFileStatus { path: "empty".into(), old_path: None, staged: None, unstaged: None },
+        ];
+        let index = file_status_index(&statuses);
+        assert_eq!(index.get("new"), Some(&Some(FileStatusKind::Modified)));
+        assert_eq!(index.get("old"), Some(&Some(FileStatusKind::Modified)));
+        assert_eq!(index.get("conflict"), Some(&Some(FileStatusKind::Conflicted)));
+        assert_eq!(index.get("empty"), Some(&None));
+        assert_eq!(index.get("missing"), None);
+    }
+
     #[test]
     fn repository_files_include_tracked_deleted_and_untracked_entries() {
         let repo = init_temp_repo("repository_files");

src/App.svelte

@@ -912,18 +912,17 @@
       const path = activeRepoPath;
       backgroundFetchInFlight = true;
       try {
-        let refsFetched = false;
+        let fetchedStatus: GitStatus | null = null;
         try {
-          await fetchRemote(path);
-          refsFetched = true;
+          fetchedStatus = await fetchRemote(path);
         } catch {
           // Manual Fetch/Pull surfaces remote errors; background work stays silent.
         }
 
         const notesFetched = await backgroundFetchCommitNotes(path);
         if (sameRepoPath(path, activeRepoPath)) {
-          if (refsFetched) {
-            applyStatus(await getStatus(path));
+          if (fetchedStatus) {
+            applyStatus(fetchedStatus);
             await refreshRefsAndCommitGraph(path);
           } else if (notesFetched) {
             await refreshCommitHistory(path);
@@ -944,17 +943,16 @@
 
     backgroundFetchInFlight = true;
     try {
-      let refsFetched = false;
+      let fetchedStatus: GitStatus | null = null;
       try {
-        await fetchRemote(path);
-        refsFetched = true;
+        fetchedStatus = await fetchRemote(path);
       } catch {
         // Manual Fetch/Pull surfaces remote errors; background work stays silent.
       }
 
       const notesFetched = await backgroundFetchCommitNotes(path);
-      if (refsFetched) {
-        const nextStatus = await getStatus(path);
+      if (fetchedStatus) {
+        const nextStatus = fetchedStatus;
         if (sameRepoPath(path, activeRepoPath)) {
           applyStatus(nextStatus);
           await refreshRefsAndCommitGraph(path);
@@ -995,6 +993,14 @@
       lastOpened: openTab?.lastOpened ?? cached?.lastOpened ?? 0,
     };
 
+    // Do not invalidate the dashboard or synchronously serialize the entire
+    // status cache when a background poll found no display changes.
+    const unchanged = (previous: RepoTab | undefined) => previous
+      && previous.branch === row.branch && previous.ahead === row.ahead
+      && previous.behind === row.behind && previous.changed === row.changed
+      && previous.lastOpened === row.lastOpened;
+    if (unchanged(cached) && (!openTab || unchanged(openTab))) return;
+
     if (openTab) {
       repoTabs = repoTabs.map((tab) => sameRepoPath(tab.path, path) ? row : tab);
     }
@@ -1019,10 +1025,7 @@
         backgroundRepoStatusIndex += 1;
 
         try {
-          if (fetchFirst) {
-            await fetchRemote(path);
-          }
-          const nextStatus = await getStatus(path);
+          const nextStatus = fetchFirst ? await fetchRemote(path) : await getStatus(path);
           updateRepoManagementStatus(path, nextStatus);
         } catch {
           // ignore this repo — same rationale as the active-repo background fetch above

src/lib/components/HistoryPanel.svelte

@@ -1,5 +1,6 @@
 <script lang="ts">
   import { Cherry, ChevronDown, ChevronRight, CloudOff, EllipsisVertical, GitBranch, GitMerge, LoaderCircle, RotateCcw, StickyNote, Tag, X } from "@lucide/svelte";
+  import { visibleParentResolver } from "../graphParents";
   import type { FileStatusKind, GitCommit, GitCommitFile } from "../types";
 
   interface GraphSegment {
@@ -338,25 +339,6 @@
     return branchesAreVisible(row?.branchLabels ?? []);
   }
 
-  function nearestVisibleGraphParents(
-    hash: string,
-    visibleHashes: Set<string>,
-    commitByHash: Map<string, GitCommit>,
-    seen: Set<string>,
-  ): string[] {
-    if (visibleHashes.has(hash)) return [hash];
-    if (seen.has(hash)) return [];
-    seen.add(hash);
-
-    const commit = commitByHash.get(hash);
-    if (!commit) return [];
-    return uniqueStrings(
-      commit.parents.flatMap((parentHash) => (
-        nearestVisibleGraphParents(parentHash, visibleHashes, commitByHash, new Set(seen))
-      )),
-    );
-  }
-
   function branchMembershipByHash(items: GitCommit[]): Map<string, string[]> {
     const commitByHash = new Map(items.map((commit) => [commit.hash, commit]));
     const membership = new Map<string, Set<string>>();
@@ -395,12 +377,12 @@
   function visibleCommitEntriesForGraph(items: GitCommit[], branchMembership: Map<string, string[]>): VisibleCommitEntry[] {
     const visibleItems = items.filter((commit) => branchesAreVisible(branchMembership.get(commit.hash) ?? []));
     const visibleHashes = new Set(visibleItems.map((commit) => commit.hash));
-    const commitByHash = new Map(items.map((commit) => [commit.hash, commit]));
+    const resolveParents = visibleParentResolver(items, visibleHashes);
 
     return visibleItems.map((commit) => {
       const parents = uniqueStrings(
         commit.parents.flatMap((parentHash) => (
-          nearestVisibleGraphParents(parentHash, visibleHashes, commitByHash, new Set())
+          resolveParents(parentHash)
         )),
       );
       return { commit, graphCommit: { ...commit, parents } };

src/lib/graphParents.ts

@@ -0,0 +1,35 @@
+interface ParentCommit { hash: string; parents: string[] }
+
+// Resolve hidden ancestry once per graph, without recursion or copying a visited
+// set for every path through a merge. Preserve Git's first-parent ordering.
+export function visibleParentResolver(items: ParentCommit[], visibleHashes: Set<string>) {
+  const commits = new Map(items.map(commit => [commit.hash, commit]));
+  const cache = new Map<string, string[]>();
+  return (hash: string): string[] => {
+    const visiting = new Set<string>();
+    const stack: { hash: string; expanded: boolean }[] = [{ hash, expanded: false }];
+    while (stack.length) {
+      const frame = stack.pop()!;
+      if (cache.has(frame.hash)) continue;
+      if (visibleHashes.has(frame.hash)) {
+        cache.set(frame.hash, [frame.hash]);
+        continue;
+      }
+      const commit = commits.get(frame.hash);
+      if (!commit) { cache.set(frame.hash, []); continue; }
+      if (frame.expanded) {
+        cache.set(frame.hash, [...new Set(commit.parents.flatMap(parent => cache.get(parent) ?? []))]);
+        visiting.delete(frame.hash);
+      } else {
+        if (visiting.has(frame.hash)) continue;
+        visiting.add(frame.hash);
+        stack.push({ hash: frame.hash, expanded: true });
+        for (let index = commit.parents.length - 1; index >= 0; index--) {
+          const parent = commit.parents[index];
+          if (!visiting.has(parent)) stack.push({ hash: parent, expanded: false });
+        }
+      }
+    }
+    return cache.get(hash) ?? [];
+  };
+}