From 4c4a84dcaa66e22eb68fcf9472abfc16604aace4 Mon Sep 17 00:00:00 2001 From: Henk Westhuis Date: Mon, 8 Oct 2018 19:18:35 +0200 Subject: [PATCH] Improved graph rendering performance by reducing locks in settingscache --- GitCommands/Settings/AppSettings.cs | 34 ++-- .../Settings/ConfigFileSettingsCache.cs | 38 ++--- GitCommands/Settings/FileSettingsCache.cs | 27 ++-- GitCommands/Settings/SettingsCache.cs | 147 +++++++----------- GitCommands/Settings/SettingsContainer.cs | 15 -- .../Columns/GraphColumnProvider.cs | 2 +- 6 files changed, 93 insertions(+), 170 deletions(-) diff --git a/GitCommands/Settings/AppSettings.cs b/GitCommands/Settings/AppSettings.cs index 3df978b1da5..cff88900160 100644 --- a/GitCommands/Settings/AppSettings.cs +++ b/GitCommands/Settings/AppSettings.cs @@ -122,22 +122,19 @@ public static bool RememberAmendCommitState public static void UsingContainer(RepoDistSettings settingsContainer, Action action) { - SettingsContainer.LockedAction(() => - { - var oldSC = SettingsContainer; - try - { - SettingsContainer = settingsContainer; - action(); - } - finally - { - SettingsContainer = oldSC; + var oldSC = SettingsContainer; + try + { + SettingsContainer = settingsContainer; + action(); + } + finally + { + SettingsContainer = oldSC; - // refresh settings if needed - SettingsContainer.GetString(string.Empty, null); - } - }); + // refresh settings if needed + SettingsContainer.GetString(string.Empty, null); + } } [CanBeNull] @@ -1342,11 +1339,8 @@ public static void SaveSettings() SaveEncodings(); try { - SettingsContainer.LockedAction(() => - { - SshPath = SshPathLocatorInstance.Find(GitBinDir); - SettingsContainer.Save(); - }); + SshPath = SshPathLocatorInstance.Find(GitBinDir); + SettingsContainer.Save(); } catch { diff --git a/GitCommands/Settings/ConfigFileSettingsCache.cs b/GitCommands/Settings/ConfigFileSettingsCache.cs index d8b5f460cf9..96f3db70815 100644 --- a/GitCommands/Settings/ConfigFileSettingsCache.cs +++ b/GitCommands/Settings/ConfigFileSettingsCache.cs @@ -73,32 +73,23 @@ protected override string GetValueImpl(string key) /// The configuration section. public void AddConfigSection(IConfigSection configSection) { - LockedAction(() => - { - EnsureSettingsAreUpToDate(); - _configFile.Value.AddConfigSection(configSection); + EnsureSettingsAreUpToDate(); + _configFile.Value.AddConfigSection(configSection); - // mark as dirty so the updated configuration is persisted - SettingsChanged(); - }); + // mark as dirty so the updated configuration is persisted + SettingsChanged(); } public IReadOnlyList GetValues(string key) { - return LockedAction(() => - { - EnsureSettingsAreUpToDate(); - return _configFile.Value.GetValues(key); - }); + EnsureSettingsAreUpToDate(); + return _configFile.Value.GetValues(key); } public IReadOnlyList GetConfigSections() { - return LockedAction(() => - { - EnsureSettingsAreUpToDate(); - return _configFile.Value.ConfigSections; - }); + EnsureSettingsAreUpToDate(); + return _configFile.Value.ConfigSections; } /// @@ -108,15 +99,12 @@ public IReadOnlyList GetConfigSections() /// If the configuration changes will be saved immediately. public void RemoveConfigSection(string configSectionName, bool performSave = false) { - LockedAction(() => + EnsureSettingsAreUpToDate(); + _configFile.Value.RemoveConfigSection(configSectionName); + if (performSave) { - EnsureSettingsAreUpToDate(); - _configFile.Value.RemoveConfigSection(configSectionName); - if (performSave) - { - _configFile.Value.Save(); - } - }); + _configFile.Value.Save(); + } } } } diff --git a/GitCommands/Settings/FileSettingsCache.cs b/GitCommands/Settings/FileSettingsCache.cs index eeca1a6e957..0bc6abdf3e9 100644 --- a/GitCommands/Settings/FileSettingsCache.cs +++ b/GitCommands/Settings/FileSettingsCache.cs @@ -62,24 +62,21 @@ protected override void Dispose(bool disposing) { if (disposing) { - LockedAction(() => + if (_saveTimer != null) { - if (_saveTimer != null) + _saveTimer.Dispose(); + _saveTimer = null; + _fileWatcher.Changed -= _fileWatcher_Changed; + _fileWatcher.Renamed -= _fileWatcher_Renamed; + _fileWatcher.Created -= _fileWatcher_Created; + + if (_autoSave) { - _saveTimer.Dispose(); - _saveTimer = null; - _fileWatcher.Changed -= _fileWatcher_Changed; - _fileWatcher.Renamed -= _fileWatcher_Renamed; - _fileWatcher.Created -= _fileWatcher_Created; - - if (_autoSave) - { - Save(); - } - - _fileWatcher.Dispose(); + Save(); } - }); + + _fileWatcher.Dispose(); + } } base.Dispose(disposing); diff --git a/GitCommands/Settings/SettingsCache.cs b/GitCommands/Settings/SettingsCache.cs index 529311fba79..749bef39000 100644 --- a/GitCommands/Settings/SettingsCache.cs +++ b/GitCommands/Settings/SettingsCache.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Concurrent; using System.Collections.Generic; using JetBrains.Annotations; @@ -6,7 +7,7 @@ namespace GitCommands { public abstract class SettingsCache : IDisposable { - private readonly Dictionary _byNameMap = new Dictionary(); + private readonly ConcurrentDictionary _byNameMap = new ConcurrentDictionary(); public void Dispose() { @@ -18,23 +19,6 @@ protected virtual void Dispose(bool disposing) { } - public void LockedAction(Action action) - { - LockedAction(() => - { - action(); - return default; - }); - } - - protected T LockedAction(Func action) - { - lock (_byNameMap) - { - return action(); - } - } - protected abstract void SaveImpl(); protected abstract void LoadImpl(); protected abstract void SetValueImpl(string key, string value); @@ -45,48 +29,39 @@ protected T LockedAction(Func action) private void Clear() { - LockedAction(() => - { - ClearImpl(); - _byNameMap.Clear(); - }); + ClearImpl(); + _byNameMap.Clear(); } public void Save() { - LockedAction(SaveImpl); + SaveImpl(); } private void Load() { - LockedAction(() => - { - Clear(); - LoadImpl(); - }); + Clear(); + LoadImpl(); } public void Import(IEnumerable<(string name, string value)> keyValuePairs) { - LockedAction(() => + foreach (var (key, value) in keyValuePairs) + { + if (value != null) { - foreach (var (key, value) in keyValuePairs) - { - if (value != null) - { - SetValueImpl(key, value); - } - } - - Save(); - }); + SetValueImpl(key, value); + } + } + + Save(); } protected void EnsureSettingsAreUpToDate() { if (NeedRefresh()) { - LockedAction(Load); + Load(); } } @@ -96,29 +71,23 @@ protected virtual void SettingsChanged() private void SetValue(string name, string value) { - LockedAction(() => - { - // will refresh EncodedNameMap if needed - string inMemValue = GetValue(name); + // will refresh EncodedNameMap if needed + string inMemValue = GetValue(name); - if (string.Equals(inMemValue, value)) - { - return; - } + if (string.Equals(inMemValue, value)) + { + return; + } - SetValueImpl(name, value); + SetValueImpl(name, value); - SettingsChanged(); - }); + SettingsChanged(); } private string GetValue(string name) { - return LockedAction(() => - { - EnsureSettingsAreUpToDate(); - return GetValueImpl(name); - }); + EnsureSettingsAreUpToDate(); + return GetValueImpl(name); } public bool HasValue(string name) @@ -132,11 +101,8 @@ public bool HasADifferentValue(string name, T value, Func encode) ? encode(value) : null; - return LockedAction(() => - { - string inMemValue = GetValue(name); - return inMemValue != null && !string.Equals(inMemValue, s); - }); + string inMemValue = GetValue(name); + return inMemValue != null && !string.Equals(inMemValue, s); } public void SetValue(string name, T value, Func encode) @@ -145,12 +111,9 @@ public void SetValue(string name, T value, Func encode) ? encode(value) : null; - LockedAction(() => - { - SetValue(name, s); + SetValue(name, s); - _byNameMap[name] = s == null ? (object)null : value; - }); + _byNameMap.AddOrUpdate(name, value, (key, oldValue) => value); } public bool TryGetValue([NotNull] string name, T defaultValue, [NotNull] Func decode, out T value) @@ -160,40 +123,36 @@ public bool TryGetValue([NotNull] string name, T defaultValue, [NotNull] Func throw new ArgumentNullException(nameof(decode), $"The decode parameter for setting {name} is null."); } - T val = defaultValue; + value = defaultValue; - bool result = LockedAction(() => - { - EnsureSettingsAreUpToDate(); + EnsureSettingsAreUpToDate(); - if (_byNameMap.TryGetValue(name, out object o)) + if (_byNameMap.TryGetValue(name, out object o)) + { + switch (o) { - switch (o) - { - case null: - return false; - case T t: - val = t; - return true; - default: - throw new Exception("Incompatible class for settings: " + name + ". Expected: " + typeof(T).FullName + ", found: " + o.GetType().FullName); - } + case null: + return false; + case T t: + value = t; + return true; + default: + throw new Exception("Incompatible class for settings: " + name + ". Expected: " + typeof(T).FullName + ", found: " + o.GetType().FullName); } + } - string s = GetValue(name); + string s = GetValue(name); - if (s == null) - { - val = defaultValue; - return false; - } + if (s == null) + { + value = defaultValue; + return false; + } - val = decode(s); - _byNameMap[name] = val; - return true; - }); - value = val; - return result; + T decodedValue = decode(s); + value = decodedValue; + _byNameMap.AddOrUpdate(name, decodedValue, (key, oldValue) => decodedValue); + return true; } } } diff --git a/GitCommands/Settings/SettingsContainer.cs b/GitCommands/Settings/SettingsContainer.cs index c7567194adc..1bf431f21bc 100644 --- a/GitCommands/Settings/SettingsContainer.cs +++ b/GitCommands/Settings/SettingsContainer.cs @@ -17,21 +17,6 @@ public SettingsContainer([CanBeNull] TLowerPriority lowerPriority, [NotNull] TCa SettingsCache = settingsCache; } - public void LockedAction(Action action) - { - SettingsCache.LockedAction(() => - { - if (LowerPriority != null) - { - LowerPriority.LockedAction(action); - } - else - { - action(); - } - }); - } - public void Save() { SettingsCache.Save(); diff --git a/GitUI/UserControls/RevisionGrid/Columns/GraphColumnProvider.cs b/GitUI/UserControls/RevisionGrid/Columns/GraphColumnProvider.cs index 53367109a02..56f0f79d181 100644 --- a/GitUI/UserControls/RevisionGrid/Columns/GraphColumnProvider.cs +++ b/GitUI/UserControls/RevisionGrid/Columns/GraphColumnProvider.cs @@ -283,7 +283,7 @@ bool DrawItem(Graphics g, ILaneRow row) var oldSmoothingMode = g.SmoothingMode; - for (int lane = 0; lane < row.Count; lane++) + for (int lane = 0; lane < row.Count && lane <= MaxLanes; lane++) { int mid = g.RenderingOrigin.X + (int)((lane + 0.5) * _laneWidth);