Skip to content

Commit c7687ad

Browse files
feat: Cache config entries by type for typed queries
Context: The config cache only stored raw (untyped) values, so Bool and Path queries always fell back to spawning individual git processes. Since Git's --type flag canonicalizes values (e.g., expanding ~/... for paths, normalizing yes/on/1 to true for bools), serving these from the raw cache would return incorrect values. Justification: Instead of bypassing the cache for typed queries, we maintain a separate cache per GitConfigurationType. Each cache is loaded with the appropriate --type flag passed to 'git config list', so Git performs canonicalization during the bulk load. This preserves the correctness guarantee while extending the performance benefit to all query types. The cache result is now authoritative when loaded: if a key is not found in the cache, we return 'not found' directly rather than falling back to an individual git process call. This avoids a redundant process spawn when the key genuinely doesn't exist. Implementation: Changed _cache from a single ConfigCache to a Dictionary keyed by GitConfigurationType. EnsureCacheLoaded() now accepts a type parameter and passes --no-type, --type=bool, or --type=path to the git config list command. InvalidateCache() clears all type-specific caches on any write operation. Renamed TypedQuery_DoesNotUseCache test to TypedQuery_CanonicalizesValues since typed queries now use their own type-specific cache rather than bypassing the cache entirely. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent eb0d445 commit c7687ad

File tree

2 files changed

+63
-23
lines changed

2 files changed

+63
-23
lines changed

src/shared/Core.Tests/GitConfigurationTests.cs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -634,7 +634,7 @@ public void GitConfiguration_CacheLevelFilter_ReturnsOnlyLocalValues()
634634
}
635635

636636
[Fact]
637-
public void GitConfiguration_TypedQuery_DoesNotUseCache()
637+
public void GitConfiguration_TypedQuery_CanonicalizesValues()
638638
{
639639
string repoPath = CreateRepository(out string workDirPath);
640640
ExecGit(repoPath, workDirPath, "config --local test.path ~/example").AssertSuccess();
@@ -647,7 +647,8 @@ public void GitConfiguration_TypedQuery_DoesNotUseCache()
647647
var git = new GitProcess(trace, trace2, processManager, gitPath, repoPath);
648648
IGitConfiguration config = git.GetConfiguration();
649649

650-
// Path type should not use cache (needs Git's canonicalization)
650+
// Path type queries use a separate cache loaded with --type=path,
651+
// so Git canonicalizes the values during cache load.
651652
bool result = config.TryGet(GitConfigurationLevel.Local, GitConfigurationType.Path,
652653
"test.path", out string value);
653654
Assert.True(result);

src/shared/Core/GitConfiguration.cs

Lines changed: 60 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
using System;
22
using System.Collections.Generic;
33
using System.Diagnostics;
4+
using System.Linq;
45
using System.Text;
56

67
namespace GitCredentialManager
@@ -345,7 +346,7 @@ public class GitProcessConfiguration : IGitConfiguration
345346

346347
private readonly ITrace _trace;
347348
private readonly GitProcess _git;
348-
private readonly ConfigCache _cache;
349+
private readonly Dictionary<GitConfigurationType, ConfigCache> _cache;
349350
private readonly bool _useCache;
350351

351352
internal GitProcessConfiguration(ITrace trace, GitProcess git) : this(trace, git, useCache: true)
@@ -360,15 +361,44 @@ internal GitProcessConfiguration(ITrace trace, GitProcess git, bool useCache)
360361
_trace = trace;
361362
_git = git;
362363
_useCache = useCache;
363-
_cache = useCache ? new ConfigCache() : null;
364+
_cache = useCache ? new Dictionary<GitConfigurationType, ConfigCache>() : null;
364365
}
365366

366-
private void EnsureCacheLoaded()
367+
private void EnsureCacheLoaded(GitConfigurationType type)
367368
{
368-
if (!_useCache || _cache.IsLoaded)
369+
ConfigCache cache;
370+
if (!_useCache || (_cache.TryGetValue(type, out cache) && cache.IsLoaded))
371+
{
372+
return;
373+
}
374+
375+
if (cache == null)
376+
{
377+
cache = new ConfigCache();
378+
_cache[type] = cache;
379+
}
380+
381+
string typeArg;
382+
383+
switch (type)
384+
{
385+
case GitConfigurationType.Raw:
386+
typeArg = "--no-type";
387+
break;
388+
389+
case GitConfigurationType.Path:
390+
typeArg = "--type=path";
391+
break;
392+
393+
case GitConfigurationType.Bool:
394+
typeArg = "--type=bool";
395+
break;
396+
397+
default:
369398
return;
399+
}
370400

371-
using (ChildProcess git = _git.CreateProcess("config list --show-scope --show-origin -z"))
401+
using (ChildProcess git = _git.CreateProcess($"config list --show-scope --show-origin -z {typeArg}"))
372402
{
373403
git.Start(Trace2ProcessClass.Git);
374404
// To avoid deadlocks, always read the output stream first and then wait
@@ -378,7 +408,7 @@ private void EnsureCacheLoaded()
378408
switch (git.ExitCode)
379409
{
380410
case 0: // OK
381-
_cache.Load(data, _trace);
411+
cache.Load(data, _trace);
382412
break;
383413
default:
384414
_trace.WriteLine($"Failed to load config cache (exit={git.ExitCode}), will use individual git config commands");
@@ -392,18 +422,24 @@ private void InvalidateCache()
392422
{
393423
if (_useCache)
394424
{
395-
_cache.Clear();
425+
foreach (ConfigCache cache in _cache.Values)
426+
{
427+
cache.Clear();
428+
}
396429
}
397430
}
398431

399432
public void Enumerate(GitConfigurationLevel level, GitConfigurationEnumerationCallback cb)
400433
{
401434
if (_useCache)
402435
{
403-
EnsureCacheLoaded();
404-
if (_cache.IsLoaded)
436+
EnsureCacheLoaded(GitConfigurationType.Raw);
437+
438+
ConfigCache cache = _cache[GitConfigurationType.Raw];
439+
440+
if (cache.IsLoaded)
405441
{
406-
_cache.Enumerate(level, cb);
442+
cache.Enumerate(level, cb);
407443
return;
408444
}
409445
}
@@ -477,17 +513,19 @@ public void Enumerate(GitConfigurationLevel level, GitConfigurationEnumerationCa
477513

478514
public bool TryGet(GitConfigurationLevel level, GitConfigurationType type, string name, out string value)
479515
{
480-
// Use cache for raw types only - typed queries need Git's canonicalization
481-
if (_useCache && type == GitConfigurationType.Raw)
516+
if (_useCache)
482517
{
483-
EnsureCacheLoaded();
484-
if (_cache.IsLoaded && _cache.TryGet(name, level, out value))
518+
EnsureCacheLoaded(type);
519+
520+
ConfigCache cache = _cache[type];
521+
if (cache.IsLoaded)
485522
{
486-
return true;
523+
// Cache is loaded, use it for the result (whether found or not)
524+
return cache.TryGet(name, level, out value);
487525
}
488526
}
489527

490-
// Fall back to individual git config command for typed queries or cache miss
528+
// Fall back to individual git config command if cache not available
491529
string levelArg = GetLevelFilterArg(level);
492530
string typeArg = GetCanonicalizeTypeArg(type);
493531
using (ChildProcess git = _git.CreateProcess($"config --null {levelArg} {typeArg} {QuoteCmdArg(name)}"))
@@ -592,13 +630,14 @@ public void Unset(GitConfigurationLevel level, string name)
592630

593631
public IEnumerable<string> GetAll(GitConfigurationLevel level, GitConfigurationType type, string name)
594632
{
595-
// Use cache for raw types only - typed queries need Git's canonicalization
596-
if (_useCache && type == GitConfigurationType.Raw)
633+
if (_useCache)
597634
{
598-
EnsureCacheLoaded();
599-
if (_cache.IsLoaded)
635+
EnsureCacheLoaded(type);
636+
637+
ConfigCache cache = _cache[type];
638+
if (cache.IsLoaded)
600639
{
601-
var cachedValues = _cache.GetAll(name, level);
640+
var cachedValues = cache.GetAll(name, level);
602641
foreach (var val in cachedValues)
603642
{
604643
yield return val;

0 commit comments

Comments
 (0)