Switch without a snapshot when the working copy is clean d3cd7ef95da6
A checkout snapshotted the working copy first even when the manifest proved it clean. It now loads only the base's history then, and the skip rule is the same either way: a file whose manifest graph key is the base's and whose graph is the same in the target is left alone. A manifest entry inside the racy window is checked by hashing the file instead of making the whole manifest untrusted, so a switch just after another keeps the fast path. The note on what the new state lacks is counted from the two states the checkout already has, not by loading them again. A one-file switch on the Linux tip takes 3.8 s (from 5.5 s). refs #35
- Author
- Andy McCall @andymccall <[email protected]>
- When
- · 2026-09-29 22:41 UTC
- Signature
- ✔ signed by Andy McCall
- Hash
d3cd7ef95da674d201fbef0231b2d467bb6335e2b57513b921842b2bb4931b13- Depends on
- 135212de5370 1c2bd9abdd4f 1d360c91a289 4dee277c327a 4fe7d6045860 6caacfff1d4f 75167672ebcd 7b9809e67c3d 982864068a59 a4fad5b085c7 c5ce0367eddf da868d5d2646 fce44ae9ad29
- Depended on by
- 4afe288b7946 4476cdbb0443 c2e1d09047db 0389c3e82f6c 3604179679eb 98a1620a3840 638fb21420d7 f9466021286a 1966f5db15ee 0554aaa7ddef
- In bookmarks
- agents-in-readme bare-hashes carried-not-removed carry-identical doctor-trust identical-sides keyless-tags main merge-remote-bookmark only-help-103 paths-for-resolutions-and-amend push-notes show-stat subrepo-pin-state switch-says
- In tags
- v0.1.0 v0.10.0 v0.11.0 v0.2.0 v0.3.0 v0.3.1 v0.4.0 v0.4.1 v0.4.2 v0.4.3 v0.4.4 v0.4.5 v0.4.6 v0.5.0 v0.5.1 v0.6.0 v0.7.0 v0.8.0 v0.9.0
- Edits
- 25 atoms, 0 tree operations, 5 files
Browse the tree at this change
modified crates/oxy-core/src/repo/checkout.rs +64 −28
| … | 8 unchanged lines |
||
| 9 | 9 | pub conflicted: bool, |
|
| 10 | 10 | /// Nested repositories that could not be moved, and other remarks. |
|
| 11 | 11 | pub notes: Vec<String>, |
|
| 12 | + | /// How many changes of the state left behind the new one lacks. |
|
| 13 | + | pub lacks: usize, |
|
| 12 | 14 | } |
|
| 13 | 15 | ||
| 14 | 16 | /// Text files rendered at a time by a checkout: bounds its memory. |
|
| … | 22 unchanged lines |
||
| 37 | 39 | keep_subrepos: &BTreeSet<String>, |
|
| 38 | 40 | ) -> Result<CheckoutReport> { |
|
| 39 | 41 | let _wc = self.lock_working_copy()?; |
|
| 40 | - | // A status snapshot: files the manifest proves unchanged are neither |
|
| 41 | - | // read nor rendered, and its base is history only (oxygen#35). |
|
| 42 | - | let snap = self.snapshot_status(command)?; |
|
| 43 | - | if !discard && (!snap.wc.tree_ops.is_empty() || !snap.wc.edits.is_empty()) { |
|
| 44 | - | let list: Vec<String> = snap |
|
| 45 | - | .statuses |
|
| 46 | - | .iter() |
|
| 47 | - | .take(8) |
|
| 48 | - | .map(|s| format!("{} {}", status_word(s.status), s.path)) |
|
| 49 | - | .collect(); |
|
| 50 | - | return fail(format!( |
|
| 51 | - | "the working copy has changes ({}{}); commit them, take them along with --carry, or drop them with --discard", |
|
| 52 | - | list.join(", "), |
|
| 53 | - | if snap.statuses.len() > 8 { ", …" } else { "" } |
|
| 54 | - | )); |
|
| 55 | - | } |
|
| 42 | + | let _all = crate::timing::phase("checkout"); |
|
| 43 | + | // A working copy clean by the manifest needs no snapshot; otherwise a |
|
| 44 | + | // status snapshot, which neither reads nor renders files the |
|
| 45 | + | // manifest proves unchanged, and whose base is history only |
|
| 46 | + | // (oxygen#35). |
|
| 47 | + | let (base, changed_paths) = if self.clean_by_manifest()? { |
|
| 48 | + | let (_, wc) = self.working_copy()?; |
|
| 49 | + | (self.load_state_history(&wc.deps)?, Vec::new()) |
|
| 50 | + | } else { |
|
| 51 | + | let snap = self.snapshot_status(command)?; |
|
| 52 | + | self.refuse_dirty(&snap, discard)?; |
|
| 53 | + | let paths = snap.statuses.iter().map(|s| s.path.clone()).collect(); |
|
| 54 | + | (snap.base, paths) |
|
| 55 | + | }; |
|
| 56 | + | let views = crate::timing::phase("checkout: manifest and base view"); |
|
| 56 | 57 | let old_manifest = self.read_manifest()?.map(|(_, _, m)| m).unwrap_or_default(); |
|
| 57 | - | let base = &snap.base; |
|
| 58 | - | let base_files = &snap.base_snapshot.files; |
|
| 59 | - | let changed: HashSet<&str> = snap.statuses.iter().map(|s| s.path.as_str()).collect(); |
|
| 58 | + | let bview = base.tree().view(); |
|
| 59 | + | drop(views); |
|
| 60 | + | let changed: HashSet<&str> = changed_paths.iter().map(String::as_str).collect(); |
|
| 60 | 61 | // The target's history; its file graphs are built a batch at a time |
|
| 61 | 62 | // for the files that must be written, and dropped after. |
|
| 62 | 63 | let mut target = self.load_state_history(heads)?; |
|
| 63 | 64 | let heads = target.maximal(heads); |
|
| 65 | + | let tv = crate::timing::phase("checkout: target view"); |
|
| 64 | 66 | let tview = target.tree().view(); |
|
| 67 | + | drop(tv); |
|
| 65 | 68 | let mut report = CheckoutReport::default(); |
|
| 66 | 69 | // Paths this file system cannot hold side by side (spec §7.7). |
|
| 67 | 70 | report.notes.extend( |
|
| … | 1 unchanged lines |
||
| 69 | 72 | .problems(tview.entries.keys().map(String::as_str)), |
|
| 70 | 73 | ); |
|
| 71 | 74 | report.conflicted = tview.is_conflicted(); |
|
| 75 | + | report.lacks = base.changes().filter(|(h, _)| !target.contains(h)).count(); |
|
| 72 | 76 | ||
| 73 | 77 | // Remove what the base or the working copy has and the target does |
|
| 74 | 78 | // not. A nested repository is never deleted (spec §7.8). |
|
| 75 | 79 | let mut dirs_to_prune: BTreeSet<PathBuf> = BTreeSet::new(); |
|
| 76 | - | let gone: BTreeSet<&str> = base_files |
|
| 80 | + | let gone: BTreeSet<&str> = bview |
|
| 81 | + | .entries |
|
| 77 | 82 | .iter() |
|
| 78 | 83 | .filter(|(_, f)| f.kind != NodeKind::Dir) |
|
| 79 | 84 | .map(|(p, _)| p.as_str()) |
|
| … | 1 unchanged lines |
||
| 81 | 86 | .filter(|p| !tview.entries.contains_key(*p)) |
|
| 82 | 87 | .collect(); |
|
| 83 | 88 | for path in gone { |
|
| 84 | - | if base_files |
|
| 89 | + | if bview |
|
| 90 | + | .entries |
|
| 85 | 91 | .get(path) |
|
| 86 | 92 | .is_some_and(|f| f.kind == NodeKind::Subrepo) |
|
| 87 | 93 | { |
|
| … | 14 unchanged lines |
||
| 102 | 108 | } |
|
| 103 | 109 | ||
| 104 | 110 | // The manifest after the checkout, built from what is known. |
|
| 111 | + | let plan = crate::timing::phase("checkout: plan"); |
|
| 105 | 112 | let mut entries: BTreeMap<String, FsEntry> = BTreeMap::new(); |
|
| 106 | 113 | let hex = |b: &[u8]| blake3::hash(b).to_hex().to_string(); |
|
| 107 | 114 | // Text files to render, attribute files first (the others need them). |
|
| … | 3 unchanged lines |
||
| 111 | 118 | continue; |
|
| 112 | 119 | } |
|
| 113 | 120 | let untouched = !changed.contains(path.as_str()) |
|
| 114 | - | && base_files |
|
| 121 | + | && bview |
|
| 122 | + | .entries |
|
| 115 | 123 | .get(path) |
|
| 116 | 124 | .is_some_and(|b| b.node == e.node && b.kind == e.kind && b.mode == e.mode); |
|
| 117 | 125 | if e.kind == NodeKind::File && !e.mode.opaque { |
|
| 118 | - | // Unchanged on disk (unrendered by the snapshot) and the same |
|
| 119 | - | // graph in both states: nothing to read, render or write. |
|
| 126 | + | // The manifest proves the file on disk equal to the base's |
|
| 127 | + | // rendering (its key is the base graph's, and its stat was |
|
| 128 | + | // checked just now), and the graph is the same in both |
|
| 129 | + | // states: nothing to read, render or write. |
|
| 130 | + | let key = base.graph_key(&e.node); |
|
| 120 | 131 | let same_graph = untouched |
|
| 121 | - | && snap.base_snapshot.unrendered.contains(path) |
|
| 122 | 132 | && !is_attributes(path) |
|
| 123 | - | && base.graph_key(&e.node).is_some() |
|
| 124 | - | && base.graph_key(&e.node) == target.graph_key(&e.node); |
|
| 125 | - | match old_manifest.entries.get(path).filter(|_| same_graph) { |
|
| 133 | + | && key.is_some() |
|
| 134 | + | && key == target.graph_key(&e.node); |
|
| 135 | + | let proven = |m: &&prompt::ManifestEntry| { |
|
| 136 | + | same_graph |
|
| 137 | + | && m.graph.as_deref() == key.map(|k| ObjectRef(k).to_hex()).as_deref() |
|
| 138 | + | }; |
|
| 139 | + | match old_manifest.entries.get(path).filter(proven) { |
|
| 126 | 140 | Some(m) => { |
|
| 127 | 141 | entries.insert( |
|
| 128 | 142 | path.clone(), |
|
| … | 87 unchanged lines |
||
| 216 | 230 | } |
|
| 217 | 231 | } |
|
| 218 | 232 | ||
| 233 | + | drop(plan); |
|
| 234 | + | let _render = crate::timing::phase("checkout: render and write"); |
|
| 219 | 235 | // Render and write text files a batch at a time; attribute files |
|
| 220 | 236 | // come first, since the others' line endings and locks follow them. |
|
| 221 | 237 | texts.sort_by_key(|(p, _, _)| !is_attributes(p)); |
|
| … | 76 unchanged lines |
||
| 298 | 314 | } |
|
| 299 | 315 | } |
|
| 300 | 316 | ||
| 317 | + | drop(_render); |
|
| 318 | + | let _t = crate::timing::phase("checkout: record and manifest"); |
|
| 301 | 319 | let (op_ref, op) = self.head_op()?; |
|
| 302 | 320 | let wc = self.new_working_copy(heads.clone(), ""); |
|
| 303 | 321 | let wc_hash = self.store.put_change(&wc)?; |
|
| … | 11 unchanged lines |
||
| 315 | 333 | ), |
|
| 316 | 334 | ); |
|
| 317 | 335 | Ok(report) |
|
| 336 | + | } |
|
| 337 | + | ||
| 338 | + | /// Refuse to leave uncommitted work behind unless `discard`. |
|
| 339 | + | fn refuse_dirty(&self, snap: &SnapshotResult, discard: bool) -> Result<()> { |
|
| 340 | + | if discard || (snap.wc.tree_ops.is_empty() && snap.wc.edits.is_empty()) { |
|
| 341 | + | return Ok(()); |
|
| 342 | + | } |
|
| 343 | + | let list: Vec<String> = snap |
|
| 344 | + | .statuses |
|
| 345 | + | .iter() |
|
| 346 | + | .take(8) |
|
| 347 | + | .map(|s| format!("{} {}", status_word(s.status), s.path)) |
|
| 348 | + | .collect(); |
|
| 349 | + | fail(format!( |
|
| 350 | + | "the working copy has changes ({}{}); commit them, take them along with --carry, or drop them with --discard", |
|
| 351 | + | list.join(", "), |
|
| 352 | + | if snap.statuses.len() > 8 { ", …" } else { "" } |
|
| 353 | + | )) |
|
| 318 | 354 | } |
|
| 319 | 355 | ||
| 320 | 356 | /// Whether the file at `full` already holds `bytes` with this executable |
|
| … | 91 unchanged lines |
modified crates/oxy-core/src/repo/prompt.rs +27 −11
| … | 351 unchanged lines |
||
| 352 | 352 | ||
| 353 | 353 | /// Whether the working copy is certainly clean, known from the manifest |
|
| 354 | 354 | /// alone, without loading any history (spec §12.4): the last snapshot |
|
| 355 | - | /// or commit found no uncommitted work and no conflict, every entry is |
|
| 356 | - | /// older than the racy window (so an equal size and time mean equal |
|
| 357 | - | /// content), there are no subrepos (whose state the manifest does not |
|
| 358 | - | /// hold), and no file was added, removed or changed since. `false` |
|
| 359 | - | /// means only that a snapshot must decide. |
|
| 355 | + | /// or commit found no uncommitted work and no conflict, there are no |
|
| 356 | + | /// subrepos (whose state the manifest does not hold), and no file was |
|
| 357 | + | /// added, removed or changed since. An entry inside the racy window, |
|
| 358 | + | /// where an equal size and time do not prove equal content, is checked |
|
| 359 | + | /// by hashing the file. `false` means only that a snapshot must decide. |
|
| 360 | 360 | pub fn clean_by_manifest(&self) -> Result<bool> { |
|
| 361 | 361 | let _t = crate::timing::phase("status: check the manifest"); |
|
| 362 | 362 | let Some((conflicted, uncommitted, m)) = self.read_manifest()? else { |
|
| … | 2 unchanged lines |
||
| 365 | 365 | if conflicted || uncommitted || self.root.join(".oxymodules").exists() { |
|
| 366 | 366 | return Ok(false); |
|
| 367 | 367 | } |
|
| 368 | - | let racy = m |
|
| 369 | - | .entries |
|
| 370 | - | .values() |
|
| 371 | - | .any(|e| e.mtime_ns + 2_000_000_000 >= m.written_ns); |
|
| 372 | - | if racy { |
|
| 373 | - | return Ok(false); |
|
| 368 | + | for (rel, e) in &m.entries { |
|
| 369 | + | if e.mtime_ns + 2_000_000_000 < m.written_ns { |
|
| 370 | + | continue; |
|
| 371 | + | } |
|
| 372 | + | let path = self.root.join(rel); |
|
| 373 | + | let same = match fs::symlink_metadata(&path) { |
|
| 374 | + | Ok(md) if md.file_type().is_symlink() => fs::read_link(&path).is_ok_and(|t| { |
|
| 375 | + | blake3::hash(t.to_string_lossy().as_bytes()) |
|
| 376 | + | .to_hex() |
|
| 377 | + | .as_str() |
|
| 378 | + | == e.hash |
|
| 379 | + | }), |
|
| 380 | + | Ok(md) if md.is_file() => { |
|
| 381 | + | md.len() == e.size |
|
| 382 | + | && fs::read(&path) |
|
| 383 | + | .is_ok_and(|b| blake3::hash(&b).to_hex().as_str() == e.hash) |
|
| 384 | + | } |
|
| 385 | + | _ => false, |
|
| 386 | + | }; |
|
| 387 | + | if !same { |
|
| 388 | + | return Ok(false); |
|
| 389 | + | } |
|
| 374 | 390 | } |
|
| 375 | 391 | Ok(self.quick_dirty()? == Some(false)) |
|
| 376 | 392 | } |
|
| … | 157 unchanged lines |
modified crates/oxy-core/src/repo/tests.rs +35 −1
| … | 432 unchanged lines |
||
| 433 | 433 | written: 2, |
|
| 434 | 434 | removed: 1, |
|
| 435 | 435 | conflicted: false, |
|
| 436 | - | notes: vec![] |
|
| 436 | + | notes: vec![], |
|
| 437 | + | lacks: 1, |
|
| 437 | 438 | } |
|
| 438 | 439 | ); |
|
| 439 | 440 | assert_eq!(fs::read(root.join("a.txt")).unwrap(), b"one\n"); |
|
| … | 5474 unchanged lines |
||
| 5914 | 5915 | .unwrap() |
|
| 5915 | 5916 | .statuses |
|
| 5916 | 5917 | .is_empty()); |
|
| 5918 | + | } |
|
| 5919 | + | ||
| 5920 | + | /// A file written just before the manifest (inside the racy window) is |
|
| 5921 | + | /// checked by its hash: trusted when equal, caught when edited in place |
|
| 5922 | + | /// with the same size and time. |
|
| 5923 | + | #[test] |
|
| 5924 | + | fn a_racy_manifest_entry_is_checked_by_hash() { |
|
| 5925 | + | hermetic(); |
|
| 5926 | + | let dir = tempfile::tempdir().unwrap(); |
|
| 5927 | + | let root = dir.path(); |
|
| 5928 | + | let repo = Repo::init(root).unwrap(); |
|
| 5929 | + | write(root, "a.txt", b"one\n"); |
|
| 5930 | + | repo.commit("fresh", &[]).unwrap(); |
|
| 5931 | + | assert!(repo.clean_by_manifest().unwrap(), "fresh but equal"); |
|
| 5932 | + | let t = fs::metadata(root.join("a.txt")) |
|
| 5933 | + | .unwrap() |
|
| 5934 | + | .modified() |
|
| 5935 | + | .unwrap(); |
|
| 5936 | + | write(root, "a.txt", b"two\n"); |
|
| 5937 | + | fs::File::options() |
|
| 5938 | + | .write(true) |
|
| 5939 | + | .open(root.join("a.txt")) |
|
| 5940 | + | .unwrap() |
|
| 5941 | + | .set_modified(t) |
|
| 5942 | + | .unwrap(); |
|
| 5943 | + | assert!( |
|
| 5944 | + | !repo.clean_by_manifest().unwrap(), |
|
| 5945 | + | "same size and time, other bytes" |
|
| 5946 | + | ); |
|
| 5947 | + | assert_eq!( |
|
| 5948 | + | statuses(&repo.snapshot_status("status").unwrap()), |
|
| 5949 | + | vec![("a.txt".to_string(), ChangeStatus::Modified)] |
|
| 5950 | + | ); |
|
| 5917 | 5951 | } |
modified docs/benchmarks.md +8 −0
| … | 331 unchanged lines |
||
| 332 | 332 | only for a graph that is built. A one-file switch then takes 5.4 to 5.6 s |
|
| 333 | 333 | and 0.8 GB, and the status after it 3.3 s and 600 MB (from 3.8 s). |
|
| 334 | 334 | ||
| 335 | + | A checkout of a working copy that is clean by the manifest now skips the |
|
| 336 | + | status snapshot, and a manifest entry inside the racy window is checked |
|
| 337 | + | by hashing the file rather than distrusting the whole manifest. A |
|
| 338 | + | one-file switch then takes 3.7 to 3.9 s and 705 MB. Where it goes: |
|
| 339 | + | checking the manifest (a walk of 96 000 files) 0.75 s, the base's and |
|
| 340 | + | the target's history 0.5 s each, their tree views 0.3 s each, planning |
|
| 341 | + | 0.2 s, and writing the manifest 0.6 s. |
|
| 342 | + | ||
| 335 | 343 | Before that, most of a switch was loading history: the base's and the target's |
|
| 336 | 344 | history each decode the one huge first change (1.2 s and 364 MB each). |
|
| 337 | 345 | A clone holds the whole bundle in memory. |
|
| … | 1 unchanged lines |
modified src/cli/workspace.rs +9 −19
| … | 1737 unchanged lines |
||
| 1738 | 1738 | pub fn bookmark_switch(name: &str, discard: bool, carry: bool) -> Result<i32> { |
|
| 1739 | 1739 | let mut repo = open()?; |
|
| 1740 | 1740 | let from = repo.config.bookmark.clone(); |
|
| 1741 | - | let before = repo |
|
| 1742 | - | .working_copy() |
|
| 1743 | - | .map(|(_, wc)| wc.deps) |
|
| 1744 | - | .unwrap_or_default(); |
|
| 1745 | 1741 | let sp = ui::spinner(&format!("Switching to {name}…")); |
|
| 1746 | 1742 | let (r, carried) = if carry { |
|
| 1747 | 1743 | repo.bookmark_switch_carry(name)? |
|
| … | 22 unchanged lines |
||
| 1770 | 1766 | // Say when the new state leaves out work the old one had: a state is |
|
| 1771 | 1767 | // its heads and what they depend on, which can be far less than a Git |
|
| 1772 | 1768 | // user expects. |
|
| 1773 | - | let after = repo |
|
| 1774 | - | .working_copy() |
|
| 1775 | - | .map(|(_, wc)| wc.deps) |
|
| 1776 | - | .unwrap_or_default(); |
|
| 1777 | - | if let Ok(gap) = repo.state_gap(&before, &after) { |
|
| 1778 | - | if gap.lacks > 0 { |
|
| 1779 | - | println!( |
|
| 1780 | - | " {} {}", |
|
| 1781 | - | ui::note(), |
|
| 1782 | - | ui::dim(format!( |
|
| 1783 | - | "{name} lacks {} that {from} has", |
|
| 1784 | - | plural(gap.lacks, "change") |
|
| 1785 | - | )) |
|
| 1786 | - | ); |
|
| 1787 | - | } |
|
| 1769 | + | if r.lacks > 0 { |
|
| 1770 | + | println!( |
|
| 1771 | + | " {} {}", |
|
| 1772 | + | ui::note(), |
|
| 1773 | + | ui::dim(format!( |
|
| 1774 | + | "{name} lacks {} that {from} has", |
|
| 1775 | + | plural(r.lacks, "change") |
|
| 1776 | + | )) |
|
| 1777 | + | ); |
|
| 1788 | 1778 | } |
|
| 1789 | 1779 | Ok(0) |
|
| 1790 | 1780 | } |
|
| … | 78 unchanged lines |