From 2d17f9d6a286b4126fc49f3d0dc50737af253ab1 Mon Sep 17 00:00:00 2001 From: Tyrie Vella Date: Fri, 12 Jun 2026 14:54:42 -0700 Subject: [PATCH] Fix bad merge: prefetch cache args and noop-cache update MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The merge of #2002 (prefetch-offload-to-mount) on top of #2004 (expand-prefetch-cache) left three issues: 1. PrefetchVerb.cs hydration-fallback path passed the removed lastPrefetchArgs variable instead of prefetchCache + prefetchCacheSize. 2. InProcessMount.cs HandlePrefetchBlobsRequest passed the removed lastPrefetchArgs variable. The mount-side handler is a one-shot download with no persistent noop cache, so it correctly receives null + 0. 3. When prefetch succeeds via mount offload, the verb-side noop cache was never updated — SavePrefetchArgs only runs inside BlobPrefetcher.PrefetchWithStats, which is skipped on the offload path. This caused NoopPrefetch to re-download on the second run instead of printing 'Nothing new to prefetch.' Fix: add BlobPrefetcher.UpdateNoopCache() static method (extracted from SavePrefetchArgs logic) and call it from PrefetchVerb after successful mount offload. Update PrefetchBlobsMountedAfterRemount test to expect the noop message since the file was already cached by a prior test in the same fixture. Assisted-by: Claude Opus 4.6 Signed-off-by: Tyrie Vella --- GVFS/GVFS.Common/Prefetch/BlobPrefetcher.cs | 36 +++++++++++++++++++ .../PrefetchBlobsOffloadTests.cs | 11 +++--- GVFS/GVFS.Mount/InProcessMount.cs | 3 +- GVFS/GVFS/CommandLine/PrefetchVerb.cs | 12 +++++-- 4 files changed, 55 insertions(+), 7 deletions(-) diff --git a/GVFS/GVFS.Common/Prefetch/BlobPrefetcher.cs b/GVFS/GVFS.Common/Prefetch/BlobPrefetcher.cs index 14dd3d09e..de47a3762 100644 --- a/GVFS/GVFS.Common/Prefetch/BlobPrefetcher.cs +++ b/GVFS/GVFS.Common/Prefetch/BlobPrefetcher.cs @@ -688,6 +688,42 @@ private void SavePrefetchArgs(string targetCommit, bool hydrate) } } + /// + /// Updates the noop prefetch cache after a successful prefetch that was + /// handled externally (e.g. offloaded to the mount process). This mirrors + /// the logic in but is callable without a + /// BlobPrefetcher instance. + /// + public static void UpdateNoopCache( + FileBasedDictionary prefetchCache, + int maxCacheSize, + string commitId, + List files, + List folders, + bool hydrate) + { + if (prefetchCache == null || maxCacheSize <= 0) + { + return; + } + + string cacheKey = ComputeCacheKey(files, folders, hydrate); + + Dictionary allEntries = prefetchCache.GetAllKeysAndValues(); + if (allEntries.Count >= maxCacheSize && !allEntries.ContainsKey(cacheKey)) + { + using (Dictionary.Enumerator enumerator = allEntries.GetEnumerator()) + { + if (enumerator.MoveNext()) + { + prefetchCache.RemoveAndFlush(enumerator.Current.Key); + } + } + } + + prefetchCache.SetValueAndFlush(cacheKey, commitId); + } + internal static string ComputeCacheKey(List files, List folders, bool hydrate) { List sortedFiles = new List(files); diff --git a/GVFS/GVFS.FunctionalTests/Tests/EnlistmentPerFixture/PrefetchBlobsOffloadTests.cs b/GVFS/GVFS.FunctionalTests/Tests/EnlistmentPerFixture/PrefetchBlobsOffloadTests.cs index c3fcf977b..98380456f 100644 --- a/GVFS/GVFS.FunctionalTests/Tests/EnlistmentPerFixture/PrefetchBlobsOffloadTests.cs +++ b/GVFS/GVFS.FunctionalTests/Tests/EnlistmentPerFixture/PrefetchBlobsOffloadTests.cs @@ -42,12 +42,13 @@ public void PrefetchBlobsMountedReportsStats() public void PrefetchBlobsUnmountedFallsBackToDirectAuth() { // Unmount, then blob prefetch should fall back to direct auth - // and still succeed. + // and still succeed. Use a file not prefetched by earlier tests + // so the noop cache doesn't short-circuit. this.Enlistment.UnmountGVFS(); try { - string output = this.Enlistment.Prefetch($"--files {Path.Combine("GVFS", "GVFS", "Program.cs")}"); + string output = this.Enlistment.Prefetch($"--files {Path.Combine("GVFS", "GVFS.Common", "GVFSEnlistment.cs")}"); output.ShouldContain("Matched blobs:"); output.ShouldContain("Downloaded:"); } @@ -69,12 +70,14 @@ public void PrefetchBlobsMountedWithFolders() public void PrefetchBlobsMountedAfterRemount() { // After unmount + remount, blob prefetch should work via - // the mount process again. + // the mount process again. Since this file was already + // prefetched in Order(1), the noop cache correctly detects + // there's nothing new to download. this.Enlistment.UnmountGVFS(); this.Enlistment.MountGVFS(); string output = this.Enlistment.Prefetch($"--files {Path.Combine("GVFS", "GVFS", "Program.cs")}"); - output.ShouldContain("Matched blobs:"); + output.ShouldContain("Nothing new to prefetch."); } } } diff --git a/GVFS/GVFS.Mount/InProcessMount.cs b/GVFS/GVFS.Mount/InProcessMount.cs index 1e1ea3712..f8e054081 100644 --- a/GVFS/GVFS.Mount/InProcessMount.cs +++ b/GVFS/GVFS.Mount/InProcessMount.cs @@ -1123,7 +1123,8 @@ private void HandlePrefetchBlobsRequest(NamedPipeMessages.Message message, Named objectRequestor, request.Files, request.Folders, - lastPrefetchArgs, + prefetchCache: null, + maxCacheSize: 0, chunkSize: 4000, searchThreadCount: maxThreads, downloadThreadCount: downloadThreads, diff --git a/GVFS/GVFS/CommandLine/PrefetchVerb.cs b/GVFS/GVFS/CommandLine/PrefetchVerb.cs index d2a084414..ef6d3ddf9 100644 --- a/GVFS/GVFS/CommandLine/PrefetchVerb.cs +++ b/GVFS/GVFS/CommandLine/PrefetchVerb.cs @@ -231,12 +231,15 @@ protected override void Execute(GVFSEnlistment enlistment) cacheServerFromConfig, out objectRequestor, out resolvedCacheServer); - this.PrefetchBlobs(tracer, enlistment, headCommitId, filesList, foldersList, lastPrefetchArgs, objectRequestor, resolvedCacheServer); + this.PrefetchBlobs(tracer, enlistment, headCommitId, filesList, foldersList, prefetchCache, prefetchCacheSize, objectRequestor, resolvedCacheServer); } else { - // Mount handled download — now hydrate locally + // Mount handled download — hydrate locally, then update noop + // cache. Cache update is after hydration so a hydration failure + // doesn't suppress the retry on the next run. this.HydrateMatchingFiles(tracer, enlistment, filesList, foldersList); + BlobPrefetcher.UpdateNoopCache(prefetchCache, prefetchCacheSize, headCommitId, filesList, foldersList, this.HydrateFiles); } } else if (!this.TryPrefetchBlobsViaMountProcess(tracer, enlistment, filesList, foldersList, headCommitId)) @@ -251,6 +254,11 @@ protected override void Execute(GVFSEnlistment enlistment) out resolvedCacheServer); this.PrefetchBlobs(tracer, enlistment, headCommitId, filesList, foldersList, prefetchCache, prefetchCacheSize, objectRequestor, resolvedCacheServer); } + else + { + // Mount handled download — update noop cache so repeat runs are skipped + BlobPrefetcher.UpdateNoopCache(prefetchCache, prefetchCacheSize, headCommitId, filesList, foldersList, hydrate: false); + } } } catch (VerbAbortedException)