Skip to content

memoize worktree→family-key resolution off the trace-ingestion path#1621

Open
svarlamov wants to merge 1 commit into
mainfrom
perf/memoize-family-key
Open

memoize worktree→family-key resolution off the trace-ingestion path#1621
svarlamov wants to merge 1 commit into
mainfrom
perf/memoize-family-key

Conversation

@svarlamov

@svarlamov svarlamov commented Jun 23, 2026

Copy link
Copy Markdown
Member

Summary

Removes repeated filesystem syscalls from the daemon's trace-ingestion path by memoizing the worktree → family-key derivation. Independent of (and not stacked on) the pull-rebase PRs; branches off main.

Background

Deriving a family key from a worktree does real I/O:

  • common_dir_for_worktree — walks parent dirs stat-ing for .git, plus a read_to_string of the .git file for linked worktrees/submodules
  • .canonicalize()stat/readlink per path component

This ran uncached on the ingestion path:

  • maybe_append_pending_root_from_trace_payload fires per mutating (family-sequencer-participating) command, doing the full derivation before its dedup guard.
  • finalize_root re-derives it again when the family key isn't already set.

A daemon session touches only a handful of worktrees, but each recurring one re-paid these syscalls on every qualifying frame.

Change

  • New canonical_family_key_for_worktree in repo_state.rs: a process-global memoized map (worktree path → canonical family-key string) behind a Mutex.
  • Routes all four call sites through it (2 in the daemon coordinator, 2 in the trace normalizer).
  • Only successful resolutions are cached. A path that isn't yet a repository (before clone/init completes) returns None and is re-resolved next call — so a repo that appears later is still picked up. This is the key correctness invariant.
  • Coarse size cap (clear-and-rebuild at 4096 entries) bounds growth in a very long-lived daemon.

Correctness

The memoized value is byte-identical to the previous uncached derivation. The mapping is a stable function of the on-disk repo layout for a live worktree.

Tests

  • New unit tests: equality-with-uncached + stability across calls; no-cache-on-miss invariant (a path that becomes a repo after an initial miss resolves on the next call).
  • Existing suites pass: daemon_mode (55), cross_repo (24), worktree (1035), clone (12). cargo fmt + clippy -D warnings clean.

Note

Pre-existing optimization surfaced during perf review of the pull-rebase work; orthogonal to those PRs, hence a standalone branch off main.

🤖 Generated with Claude Code


Open in Devin Review

Deriving a family key from a worktree runs common_dir_for_worktree (stat walk
up the tree for .git, plus a .git-file read for linked worktrees) followed by
canonicalize (stat/readlink per path component). This ran uncached on the
trace-ingestion path — maybe_append_pending_root_from_trace_payload fires per
mutating command, and finalize_root re-derives it again — so the same handful
of worktrees re-paid these syscalls on every frame that reached them.

Add canonical_family_key_for_worktree in repo_state: a process-global
memoized map (worktree path → canonical family-key string) behind a Mutex,
and route all four call sites (the two daemon coordinator sites and the two
trace normalizer sites) through it. The mapping is a stable function of the
on-disk repo layout for a live worktree, so caching is safe. Only successful
resolutions are cached — a path that is not yet a repository (before
clone/init completes) returns None and is re-resolved next call, so a repo
that appears later is still picked up. A coarse size cap (clear-and-rebuild at
4096 entries) bounds growth in a very long-lived daemon; real sessions touch
far fewer.

Behavior is unchanged: the value equals the previous uncached derivation. New
unit tests cover equality-with-uncached, stability across calls, and the
no-cache-on-miss invariant. Existing daemon_mode (55), cross_repo (24),
worktree (1035), and clone (12) suites pass.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant