Repository navigation
fix(topic): a scoped placement reads lazy stores without fetching them - #12
Merged
Merged
Conversation
A placement that asks no lazy remote for the topic's branch (`start`, `join`, `leave`) answered from the lazy store's refs, but read them as an online placement does, which fetches the store first. So every lazy repository whose store was on disk was fetched, one after another, on each start and join: more round trips than asking each remote would have cost. It now reads what the command already fetched, or else what the store last fetched, and asks no remote. A store not on disk is not read at all, so a placement fetching on a miss no longer fetches one only to answer this. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q7zEtVsJRpPdHYgsL1ycA4
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes a regression in v0.8.2 from #11: on a workspace whose lazy repositories have stores on disk,
topic start,joinandleavefetch every one of those stores, one after another.Cause
#11's second commit made
Network::Scopedanswer a lazy repository's topic branch from its store's refs rather than "no". It read them throughGitStores::refs, which in an online placement fetches the store first (load_refs→Stores::update). So the scoped placement, meant to ask no lazy remote, rangit fetch --pruneon each lazy store it found. That costs more than the ref advertisement per remote it replaced.perf_028andperf_030didn't catch it because their lazy repository has no store yet.What changes
remote_topicin a scoped placement reads what this command already fetched for the store, or else what the store last fetched (stored_refs, split out ofload_refs), and asks no remote. perf(topic): join and leave ask no other lazy remote; finish fetches a store once #11's fix still holds: a branch the shared store already has is found.stored && self.refs(url)short-circuited. Keeping that matters for a placement that fetches on a miss: reading an absent store marks it missed, and the next round fetches it.docs/topics.mdsays the store is read without being fetched again.Measurements
On a workspace of about 50 lazy entries with stores on disk, all behind one SSH remote:
topic joinof one repositorytopic leaveof itTests
perf_033_scoped_placement_fetches_no_lazy_store: a lazy repository joined once (so its store exists), thenstart,joinandleaveof another checkout must make no round trip to it. It fails onmain(topic startfetched the lazy store: 1).cargo testandcargo clippy --all-targets -- -D warningspass, apart from five tests that fail onmaintoo when the checkout sits inside another git repository (cli::error_010,config::error_001,lazy::normal_004,require::edge_006,workspace::error_004). They expect no enclosing repository.🤖 Generated with Claude Code
https://claude.ai/code/session_01Q7zEtVsJRpPdHYgsL1ycA4