Skip to content

Commit 4f37e54

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 625d254 commit 4f37e54

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
@@ -336,7 +337,7 @@ public class GitProcessConfiguration : IGitConfiguration
336337

337338
private readonly ITrace _trace;
338339
private readonly GitProcess _git;
339-
private readonly ConfigCache _cache;
340+
private readonly Dictionary<GitConfigurationType, ConfigCache> _cache;
340341
private readonly bool _useCache;
341342

342343
internal GitProcessConfiguration(ITrace trace, GitProcess git) : this(trace, git, useCache: true)
@@ -351,15 +352,44 @@ internal GitProcessConfiguration(ITrace trace, GitProcess git, bool useCache)
351352
_trace = trace;
352353
_git = git;
353354
_useCache = useCache;
354-
_cache = useCache ? new ConfigCache() : null;
355+
_cache = useCache ? new Dictionary<GitConfigurationType, ConfigCache>() : null;
355356
}
356357

357-
private void EnsureCacheLoaded()
358+
private void EnsureCacheLoaded(GitConfigurationType type)
358359
{
359-
if (!_useCache || _cache.IsLoaded)
360+
ConfigCache cache;
361+
if (!_useCache || (_cache.TryGetValue(type, out cache) && cache.IsLoaded))
362+
{
363+
return;
364+
}
365+
366+
if (cache == null)
367+
{
368+
cache = new ConfigCache();
369+
_cache[type] = cache;
370+
}
371+
372+
string typeArg;
373+
374+
switch (type)
375+
{
376+
case GitConfigurationType.Raw:
377+
typeArg = "--no-type";
378+
break;
379+
380+
case GitConfigurationType.Path:
381+
typeArg = "--type=path";
382+
break;
383+
384+
case GitConfigurationType.Bool:
385+
typeArg = "--type=bool";
386+
break;
387+
388+
default:
360389
return;
390+
}
361391

362-
using (ChildProcess git = _git.CreateProcess("config list --show-origin -z"))
392+
using (ChildProcess git = _git.CreateProcess($"config list --show-origin -z {typeArg}"))
363393
{
364394
git.Start(Trace2ProcessClass.Git);
365395
// To avoid deadlocks, always read the output stream first and then wait
@@ -369,7 +399,7 @@ private void EnsureCacheLoaded()
369399
switch (git.ExitCode)
370400
{
371401
case 0: // OK
372-
_cache.Load(data, _trace);
402+
cache.Load(data, _trace);
373403
break;
374404
default:
375405
_trace.WriteLine($"Failed to load config cache (exit={git.ExitCode})");
@@ -383,18 +413,24 @@ private void InvalidateCache()
383413
{
384414
if (_useCache)
385415
{
386-
_cache.Clear();
416+
foreach (ConfigCache cache in _cache.Values)
417+
{
418+
cache.Clear();
419+
}
387420
}
388421
}
389422

390423
public void Enumerate(GitConfigurationLevel level, GitConfigurationEnumerationCallback cb)
391424
{
392425
if (_useCache)
393426
{
394-
EnsureCacheLoaded();
395-
if (_cache.IsLoaded)
427+
EnsureCacheLoaded(GitConfigurationType.Raw);
428+
429+
ConfigCache cache = _cache[GitConfigurationType.Raw];
430+
431+
if (cache.IsLoaded)
396432
{
397-
_cache.Enumerate(level, cb);
433+
cache.Enumerate(level, cb);
398434
return;
399435
}
400436
}
@@ -468,17 +504,19 @@ public void Enumerate(GitConfigurationLevel level, GitConfigurationEnumerationCa
468504

469505
public bool TryGet(GitConfigurationLevel level, GitConfigurationType type, string name, out string value)
470506
{
471-
// Use cache for raw types only - typed queries need Git's canonicalization
472-
if (_useCache && type == GitConfigurationType.Raw)
507+
if (_useCache)
473508
{
474-
EnsureCacheLoaded();
475-
if (_cache.IsLoaded && _cache.TryGet(name, level, out value))
509+
EnsureCacheLoaded(type);
510+
511+
ConfigCache cache = _cache[type];
512+
if (cache.IsLoaded)
476513
{
477-
return true;
514+
// Cache is loaded, use it for the result (whether found or not)
515+
return cache.TryGet(name, level, out value);
478516
}
479517
}
480518

481-
// Fall back to individual git config command for typed queries or cache miss
519+
// Fall back to individual git config command if cache not available
482520
string levelArg = GetLevelFilterArg(level);
483521
string typeArg = GetCanonicalizeTypeArg(type);
484522
using (ChildProcess git = _git.CreateProcess($"config --null {levelArg} {typeArg} {QuoteCmdArg(name)}"))
@@ -583,13 +621,14 @@ public void Unset(GitConfigurationLevel level, string name)
583621

584622
public IEnumerable<string> GetAll(GitConfigurationLevel level, GitConfigurationType type, string name)
585623
{
586-
// Use cache for raw types only - typed queries need Git's canonicalization
587-
if (_useCache && type == GitConfigurationType.Raw)
624+
if (_useCache)
588625
{
589-
EnsureCacheLoaded();
590-
if (_cache.IsLoaded)
626+
EnsureCacheLoaded(type);
627+
628+
ConfigCache cache = _cache[type];
629+
if (cache.IsLoaded)
591630
{
592-
var cachedValues = _cache.GetAll(name, level);
631+
var cachedValues = cache.GetAll(name, level);
593632
foreach (var val in cachedValues)
594633
{
595634
yield return val;

0 commit comments

Comments
 (0)