Skip to content

Commit 68b5536

Browse files
committed
settings: cache Git config values across process
When enumerating all Git configuration values in `Settings`, we currently cache a copy of all values for the lifetime of the `GetSettingValues()` method. This saves us some calls to Git, but we can do better. Pull-up the cache of Git configuration entries to the lifetime of the `Settings` object, meaning all calls to `GetSettingValues()` over the lifetime of the process can benefit from the cache! The savings here can be significant as each `ISettings` property typically results in a call to `GetSettingValues()`.
1 parent 5041199 commit 68b5536

File tree

1 file changed

+30
-16
lines changed

1 file changed

+30
-16
lines changed

src/shared/Core/Settings.cs

Lines changed: 30 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ namespace GitCredentialManager
1212
{
1313
/// <summary>
1414
/// Component that represents settings for Git Credential Manager as found from the environment and Git configuration.
15+
/// Setting values from Git configuration may be cached for performance reasons.
1516
/// </summary>
1617
public interface ISettings : IDisposable
1718
{
@@ -26,7 +27,6 @@ public interface ISettings : IDisposable
2627
/// <returns>True if a setting value was found, false otherwise.</returns>
2728
bool TryGetSetting(string envarName, string section, string property, out string value);
2829

29-
3030
/// <summary>
3131
/// Try and get the value of a specified setting as specified in the environment and Git configuration,
3232
/// with the environment taking precedence over Git. If the value is pulled from the Git configuration,
@@ -291,6 +291,8 @@ public class Settings : ISettings
291291
private readonly IEnvironment _environment;
292292
private readonly IGit _git;
293293

294+
private Dictionary<string,string> _configEntries;
295+
294296
public Settings(IEnvironment environment, IGit git)
295297
{
296298
EnsureArgument.NotNull(environment, nameof(environment));
@@ -334,6 +336,30 @@ public IEnumerable<string> GetSettingValues(string envarName, string section, st
334336
{
335337
IGitConfiguration config = _git.GetConfiguration();
336338

339+
//
340+
// Enumerate all configuration entries for all sections and property names and make a
341+
// local copy of them here to avoid needing to call `TryGetValue` on the IGitConfiguration
342+
// object multiple times in a loop below.
343+
//
344+
// This is a performance optimisation to avoid calling `TryGet` on the IGitConfiguration
345+
// object multiple times in a loop below, or each time this entire method is called.
346+
// The assumption is that the configuration entries will not change during a single invocation
347+
// of Git Credential Manager, which is reasonable given process lifetime is typically less
348+
// than a few seconds. For some entries (type=path), we still need to ask Git in order to
349+
// expand the path correctly.
350+
//
351+
if (_configEntries is null)
352+
{
353+
_configEntries = new Dictionary<string, string>(GitConfigurationKeyComparer.Instance);
354+
config.Enumerate(entry =>
355+
{
356+
_configEntries[entry.Key] = entry.Value;
357+
358+
// Continue the enumeration
359+
return true;
360+
});
361+
}
362+
337363
if (RemoteUri != null)
338364
{
339365
/*
@@ -379,26 +405,14 @@ public IEnumerable<string> GetSettingValues(string envarName, string section, st
379405
*
380406
*/
381407

382-
// Enumerate all configuration entries with the correct section and property name
383-
// and make a local copy of them here to avoid needing to call `TryGetValue` on the
384-
// IGitConfiguration object multiple times in a loop below.
385-
var configEntries = new Dictionary<string, string>(GitConfigurationKeyComparer.Instance);
386-
config.Enumerate(section, property, entry =>
387-
{
388-
configEntries[entry.Key] = entry.Value;
389-
390-
// Continue the enumeration
391-
return true;
392-
});
393-
394408
foreach (string scope in RemoteUri.GetGitConfigurationScopes())
395409
{
396410
string queryName = $"{section}.{scope}.{property}";
397411
// Look for a scoped entry that includes the scheme "protocol://example.com" first as
398412
// this is more specific. If `isPath` is true, then re-get the value from the
399413
// `GitConfiguration` with `isPath` specified.
400414
if ((isPath && config.TryGet(queryName, isPath, out value)) ||
401-
configEntries.TryGetValue(queryName, out value))
415+
_configEntries.TryGetValue(queryName, out value))
402416
{
403417
yield return value;
404418
}
@@ -409,7 +423,7 @@ public IEnumerable<string> GetSettingValues(string envarName, string section, st
409423
string scopeWithoutScheme = scope.TrimUntilIndexOf(Uri.SchemeDelimiter);
410424
string queryWithSchemeName = $"{section}.{scopeWithoutScheme}.{property}";
411425
if ((isPath && config.TryGet(queryWithSchemeName, isPath, out value)) ||
412-
configEntries.TryGetValue(queryWithSchemeName, out value))
426+
_configEntries.TryGetValue(queryWithSchemeName, out value))
413427
{
414428
yield return value;
415429
}
@@ -431,7 +445,7 @@ public IEnumerable<string> GetSettingValues(string envarName, string section, st
431445
*/
432446
string name = $"{section}.{property}";
433447
if ((isPath && config.TryGet(name, isPath, out value)) ||
434-
configEntries.TryGetValue(name, out value))
448+
_configEntries.TryGetValue(name, out value))
435449
{
436450
yield return value;
437451
}

0 commit comments

Comments
 (0)