max / makenotwork

server: cache flattened notes trees in AppState The badge added in f42510dd re-walked the notes tree on every log page view. NotesCache exists for exactly that and now lives in AppCaches, projected into the Git state slice, so the commit log and the file log share one cache across every request. Its key is repo path plus namespace plus ref tip. The tip means a stale entry is impossible and there is no invalidation to write: a note added to a namespace moves the ref, which moves the key, so the next read is a miss rather than a wrong answer. The repo path is there because a fork shares its notes tree with its parent, and the tip alone would collide across the two. The cache stays a value held by AppState rather than a global inside the notes module. That module is meant to lift into its own crate once the P3 merge strategies exist, and a library owning process-wide state would not survive the move.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: Max Johnson <me@maxj.phd> · 2026-08-08 19:26 UTC

Signed with PGP, not checked

Commit: df407bfdfc43d883cc6a3872a5c2a0aac41149b9

Parent: f42510d

3 files changed, +58 insertions, -24 deletions

M server/src/lib.rs +8

			@@ -185,6 +185,11 @@
185	185		pub sync_notify: Arc<DashMap<(SyncAppId, UserId), tokio::sync::broadcast::Sender<i64>>>,
186	186		/// Concurrent SSE connection count per user (for rate limiting).
187	187		pub sse_connections: Arc<DashMap<UserId, std::sync::atomic::AtomicUsize>>,
	188	+	/// Flattened `refs/notes/*` trees, so a log page does not re-walk the notes
	189	+	/// tree on every request. Keyed by repo path + namespace + ref tip, which
	190	+	/// means a moved ref is a miss rather than a stale hit and there is no
	191	+	/// invalidation to get wrong.
	192	+	pub notes_cache: Arc<git::notes::NotesCache>,
188	193		}
189	194
190	195		/// Concurrency limiters held by [`AppState`], guarding memory-/process-heavy
			@@ -338,6 +343,7 @@
338	343		pub struct Git {
339	344		pub syntax: Option<Arc<git::SyntaxHighlighter>>,
340	345		pub smart_http_semaphore: Arc<tokio::sync::Semaphore>,
	346	+	pub notes_cache: Arc<git::notes::NotesCache>,
341	347		}
342	348
343	349		impl FromRef<AppState> for Git {
			@@ -345,6 +351,7 @@
345	351		Self {
346	352		syntax: s.syntax.clone(),
347	353		smart_http_semaphore: s.limiters.git_smart_http_semaphore.clone(),
	354	+	notes_cache: Arc::clone(&s.caches.notes_cache),
348	355		}
349	356		}
350	357		}
			@@ -463,6 +470,7 @@
463	470		domain_cache: parts.domain_cache,
464	471		sync_notify: Arc::new(DashMap::new()),
465	472		sse_connections: Arc::new(DashMap::new()),
	473	+	notes_cache: Arc::new(git::notes::NotesCache::default()),
466	474		},
467	475		limiters: AppLimiters {
468	476		scan_semaphore: Arc::new(tokio::sync::Semaphore::new(

M server/src/routes/git/browsing.rs +6 -2

			@@ -312,6 +312,7 @@
312	312		pub(super) async fn commit_log(
313	313		State(db): State<PgPool>,
314	314		State(config): State<Config>,
	315	+	State(git_state): State<Git>,
315	316		session: Session,
316	317		MaybeUserVerified(maybe_user): MaybeUserVerified,
317	318		Path((owner, repo_name, git_ref)): Path<(String, String, String)>,
			@@ -331,6 +332,7 @@
331	332		let offset = (page - 1).saturating_mul(limit);
332	333
333	334		let git_ref_c = git_ref.clone();
	335	+	let notes_cache = git_state.notes_cache;
334	336		let (refs, commits, has_more, annotated) = resolved
335	337		.with_repo(move \|gix_repo\| {
336	338		let refs = git::list_refs(gix_repo);
			@@ -341,7 +343,7 @@
341	343		let has_more = commits.len() > limit;
342	344		commits.truncate(limit);
343	345		let page_oids: Vec<String> = commits.iter().map(\|c\| c.oid.clone()).collect();
344		-	let annotated = notes_view::annotation_counts(gix_repo, &page_oids);
	346	+	let annotated = notes_view::annotation_counts(gix_repo, &notes_cache, &page_oids);
345	347		Ok((refs, commits, has_more, annotated))
346	348		})
347	349		.await?;
			@@ -564,6 +566,7 @@
564	566		pub(super) async fn file_log(
565	567		State(db): State<PgPool>,
566	568		State(config): State<Config>,
	569	+	State(git_state): State<Git>,
567	570		session: Session,
568	571		MaybeUserVerified(maybe_user): MaybeUserVerified,
569	572		Path((owner, repo_name, git_ref, path)): Path<(String, String, String, String)>,
			@@ -584,6 +587,7 @@
584	587
585	588		let git_ref_c = git_ref.clone();
586	589		let path_c = path.clone();
	590	+	let notes_cache = git_state.notes_cache;
587	591		let (refs, commits, has_more, annotated) = resolved
588	592		.with_repo(move \|gix_repo\| {
589	593		let refs = git::list_refs(gix_repo);
			@@ -599,7 +603,7 @@
599	603		let has_more = commits.len() > limit;
600	604		commits.truncate(limit);
601	605		let page_oids: Vec<String> = commits.iter().map(\|c\| c.oid.clone()).collect();
602		-	let annotated = notes_view::annotation_counts(gix_repo, &page_oids);
	606	+	let annotated = notes_view::annotation_counts(gix_repo, &notes_cache, &page_oids);
603	607		Ok((refs, commits, has_more, annotated))
604	608		})
605	609		.await?;

M server/src/routes/git/notes_view.rs +44 -22

			@@ -100,13 +100,18 @@
100	100		/// How many namespaces annotate each of `targets`, keyed by the hex id the
101	101		/// caller passed in.
102	102		///
103		-	/// This is the log-page path, so it is one [`notes::notes_for_many`] call per
104		-	/// namespace per page and never one lookup per row: `notes_for_many` flattens
105		-	/// the notes tree once and answers every row from that. A repository with no
106		-	/// notes costs one ref scan and stops there.
	103	+	/// This is the log-page path: every row on the page is answered from one
	104	+	/// flattened notes tree per namespace, never a lookup per row. The flattening
	105	+	/// goes through `cache`, so a repository whose notes ref has not moved pays for
	106	+	/// the walk once rather than once per page view. A repository with no notes
	107	+	/// costs one ref scan and stops there.
107	108		///
108	109		/// Targets absent from the result carry no note; the templates ask with `get`.
109		-	pub fn annotation_counts(repo: &gix::Repository, targets: &[String]) -> HashMap<String, usize> {
	110	+	pub fn annotation_counts(
	111	+	repo: &gix::Repository,
	112	+	cache: &notes::NotesCache,
	113	+	targets: &[String],
	114	+	) -> HashMap<String, usize> {
110	115		let mut counts = HashMap::new();
111	116		if targets.is_empty() {
112	117		return counts;
			@@ -122,27 +127,30 @@
122	127		}
123	128		};
124	129
	130	+	// The cache key needs the repository as well as the ref tip: a fork shares
	131	+	// its notes tree with its parent, so the tip alone collides across repos.
	132	+	let repo_key = repo.path().to_string_lossy().into_owned();
	133	+
125	134		// Keep the caller's spelling of each id: the templates look the count up by
126	135		// the same string they printed, and re-deriving it from the Oid would make
127	136		// that lookup depend on two hex encoders agreeing.
128		-	let mut by_oid: HashMap<Oid, &String> = HashMap::with_capacity(targets.len());
129		-	for hex in targets {
130		-	if let Ok(oid) = Oid::from_hex(hex.as_bytes()) {
131		-	by_oid.insert(oid, hex);
132		-	}
133		-	}
134		-	let oids: Vec<Oid> = by_oid.keys().copied().collect();
	137	+	let parsed: Vec<(Oid, &String)> = targets
	138	+	.iter()
	139	+	.filter_map(\|hex\| Oid::from_hex(hex.as_bytes()).ok().map(\|oid\| (oid, hex)))
	140	+	.collect();
135	141
136	142		for ns in namespaces {
137		-	match notes::notes_for_many(&engine, ns.tip, &oids) {
138		-	Ok(found) => {
139		-	for target in found.keys() {
140		-	if let Some(hex) = by_oid.get(target) {
141		-	counts.entry((hex).clone()).or_insert(0) += 1;
142		-	}
143		-	}
	143	+	let flattened = match cache.flattened(&engine, &repo_key, &ns) {
	144	+	Ok(map) => map,
	145	+	Err(e) => {
	146	+	tracing::warn!(namespace = %ns.name, error = %e, "batch note lookup failed");
	147	+	continue;
	148	+	}
	149	+	};
	150	+	for (oid, hex) in &parsed {
	151	+	if flattened.contains_key(oid) {
	152	+	counts.entry((hex).clone()).or_insert(0) += 1;
144	153		}
145		-	Err(e) => tracing::warn!(namespace = %ns.name, error = %e, "batch note lookup failed"),
146	154		}
147	155		}
148	156		counts
			@@ -231,16 +239,30 @@
231	239		// Three ids, so this takes `notes_for_many`'s flattening branch — the
232	240		// one the log page actually uses.
233	241		let page = vec![TARGET.to_string(), "0".repeat(40), "1".repeat(40)];
234		-	let counts = annotation_counts(&repo, &page);
	242	+	let counts = annotation_counts(&repo, &notes::NotesCache::default(), &page);
235	243
236	244		assert_eq!(counts.get(TARGET).copied(), Some(2));
237	245		assert_eq!(counts.len(), 1, "unannotated commits carry no entry");
238	246		}
239	247
	248	+	#[test]
	249	+	fn a_second_page_view_is_answered_from_the_cache() {
	250	+	let (_tmp, repo) = annotated_repo();
	251	+	let cache = notes::NotesCache::default();
	252	+	let page = vec![TARGET.to_string()];
	253	+
	254	+	let first = annotation_counts(&repo, &cache, &page);
	255	+	assert_eq!(cache.len(), 2, "one flattened tree per namespace");
	256	+
	257	+	let second = annotation_counts(&repo, &cache, &page);
	258	+	assert_eq!(first, second);
	259	+	assert_eq!(cache.len(), 2, "the second view walked nothing new");
	260	+	}
	261	+
240	262		#[test]
241	263		fn a_page_of_nothing_asks_the_repository_nothing() {
242	264		let (_tmp, repo) = annotated_repo();
243		-	assert!(annotation_counts(&repo, &[]).is_empty());
	265	+	assert!(annotation_counts(&repo, &notes::NotesCache::default(), &[]).is_empty());
244	266		}
245	267
246	268		#[test]