Cache Git configuration used in Settings for the lifetime of the process (#1172)

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()`.

Whilst in this space we can fix a couple of bugs:

1. We were not calling `git config --type=path` for `isPath = true`
variables when resolving settings, and instead consulting the cache
first. We always want to go to Git for these types of values.

2. We were not using the existing cache for unscoped entries!

In my limited testing this results in a 2-3x speedup in calls to `get &&
store`!

Author: mjcheetham

src/shared/Core/Settings.cs

@@ -12,6 +12,7 @@ namespace GitCredentialManager
 {
     /// <summary>
     /// Component that represents settings for Git Credential Manager as found from the environment and Git configuration.
+    /// Setting values from Git configuration may be cached for performance reasons.
     /// </summary>
     public interface ISettings : IDisposable
     {
@@ -26,7 +27,6 @@ public interface ISettings : IDisposable
         /// <returns>True if a setting value was found, false otherwise.</returns>
         bool TryGetSetting(string envarName, string section, string property, out string value);
 
-
         /// <summary>
         /// Try and get the value of a specified setting as specified in the environment and Git configuration,
         /// with the environment taking precedence over Git. If the value is pulled from the Git configuration,
@@ -291,6 +291,8 @@ public class Settings : ISettings
         private readonly IEnvironment _environment;
         private readonly IGit _git;
 
+        private Dictionary<string,string> _configEntries;
+
         public Settings(IEnvironment environment, IGit git)
         {
             EnsureArgument.NotNull(environment, nameof(environment));
@@ -334,6 +336,30 @@ public IEnumerable<string> GetSettingValues(string envarName, string section, st
             {
                 IGitConfiguration config = _git.GetConfiguration();
 
+                //
+                // Enumerate all configuration entries for all sections and property names and make a
+                // local copy of them here to avoid needing to call `TryGetValue` on the IGitConfiguration
+                // object multiple times in a loop below.
+                //
+                // This is a performance optimisation to avoid calling `TryGet` on the IGitConfiguration
+                // object multiple times in a loop below, or each time this entire method is called.
+                // The assumption is that the configuration entries will not change during a single invocation
+                // of Git Credential Manager, which is reasonable given process lifetime is typically less
+                // than a few seconds. For some entries (type=path), we still need to ask Git in order to
+                // expand the path correctly.
+                //
+                if (_configEntries is null)
+                {
+                    _configEntries = new Dictionary<string, string>(GitConfigurationKeyComparer.Instance);
+                    config.Enumerate(entry =>
+                    {
+                        _configEntries[entry.Key] = entry.Value;
+
+                        // Continue the enumeration
+                        return true;
+                    });
+                }
+
                 if (RemoteUri != null)
                 {
                     /*
@@ -379,26 +405,14 @@ public IEnumerable<string> GetSettingValues(string envarName, string section, st
                      *
                      */
 
-                    // Enumerate all configuration entries with the correct section and property name
-                    // and make a local copy of them here to avoid needing to call `TryGetValue` on the
-                    // IGitConfiguration object multiple times in a loop below.
-                    var configEntries = new Dictionary<string, string>(GitConfigurationKeyComparer.Instance);
-                    config.Enumerate(section, property, entry =>
-                    {
-                        configEntries[entry.Key] = entry.Value;
-
-                        // Continue the enumeration
-                        return true;
-                    });
-
                     foreach (string scope in RemoteUri.GetGitConfigurationScopes())
                     {
                         string queryName = $"{section}.{scope}.{property}";
                         // Look for a scoped entry that includes the scheme "protocol://example.com" first as
                         // this is more specific. If `isPath` is true, then re-get the value from the
                         // `GitConfiguration` with `isPath` specified.
-                        if (configEntries.TryGetValue(queryName, out value) &&
-                            (!isPath || config.TryGet(queryName, isPath, out value)))
+                        if ((isPath && config.TryGet(queryName, true, out value)) ||
+                            _configEntries.TryGetValue(queryName, out value))
                         {
                             yield return value;
                         }
@@ -408,8 +422,8 @@ public IEnumerable<string> GetSettingValues(string envarName, string section, st
                         // `isPath` specified.
                         string scopeWithoutScheme = scope.TrimUntilIndexOf(Uri.SchemeDelimiter);
                         string queryWithSchemeName = $"{section}.{scopeWithoutScheme}.{property}";
-                        if (configEntries.TryGetValue(queryWithSchemeName, out value) &&
-                            (!isPath || config.TryGet(queryWithSchemeName, isPath, out value)))
+                        if ((isPath && config.TryGet(queryWithSchemeName, true, out value)) ||
+                            _configEntries.TryGetValue(queryWithSchemeName, out value))
                         {
                             yield return value;
                         }
@@ -429,7 +443,9 @@ public IEnumerable<string> GetSettingValues(string envarName, string section, st
                  *        property = value
                  *
                  */
-                if (config.TryGet($"{section}.{property}", isPath, out value))
+                string name = $"{section}.{property}";
+                if ((isPath && config.TryGet(name, true, out value)) ||
+                    _configEntries.TryGetValue(name, out value))
                 {
                     yield return value;
                 }