diff --git a/PowerToys.slnx b/PowerToys.slnx index 645a61167d19..a1087e6c6619 100644 --- a/PowerToys.slnx +++ b/PowerToys.slnx @@ -381,6 +381,10 @@ + + + + diff --git a/src/modules/cmdpal/CommandPalette - no UI tests.slnf b/src/modules/cmdpal/CommandPalette - no UI tests.slnf index a9016d79cf2a..3e67befcfde2 100644 --- a/src/modules/cmdpal/CommandPalette - no UI tests.slnf +++ b/src/modules/cmdpal/CommandPalette - no UI tests.slnf @@ -1,4 +1,4 @@ -{ +{ "solution": { "path": "..\\..\\..\\PowerToys.slnx", "projects": [ @@ -19,11 +19,10 @@ "src\\modules\\cmdpal\\Tests\\Microsoft.CmdPal.Ext.Apps.UnitTests\\Microsoft.CmdPal.Ext.Apps.UnitTests.csproj", "src\\modules\\cmdpal\\Tests\\Microsoft.CmdPal.Ext.Bookmarks.UnitTests\\Microsoft.CmdPal.Ext.Bookmarks.UnitTests.csproj", "src\\modules\\cmdpal\\Tests\\Microsoft.CmdPal.Ext.Calc.UnitTests\\Microsoft.CmdPal.Ext.Calc.UnitTests.csproj", - "src\\modules\\cmdpal\\Tests\\Microsoft.CmdPal.Ext.ClipboardHistory.UnitTests\\Microsoft.CmdPal.Ext.ClipboardHistory.UnitTests.csproj", + "src\\modules\\cmdpal\\Tests\\Microsoft.CmdPal.Ext.ClipboardHistory.UnitTests\\Microsoft.CmdPal.Ext.ClipboardHistory.UnitTests.csproj", "src\\modules\\cmdpal\\Tests\\Microsoft.CmdPal.Ext.Indexer.UnitTests\\Microsoft.CmdPal.Ext.Indexer.UnitTests.csproj", "src\\modules\\cmdpal\\Tests\\Microsoft.CmdPal.Ext.PerformanceMonitor.UnitTests\\Microsoft.CmdPal.Ext.PerformanceMonitor.UnitTests.csproj", "src\\modules\\cmdpal\\Tests\\Microsoft.CmdPal.Ext.Registry.UnitTests\\Microsoft.CmdPal.Ext.Registry.UnitTests.csproj", - "src\\modules\\cmdpal\\Tests\\Microsoft.CmdPal.Ext.WindowsSettings.UnitTests\\Microsoft.CmdPal.Ext.WindowsSettings.UnitTests.csproj", "src\\modules\\cmdpal\\Tests\\Microsoft.CmdPal.Ext.RemoteDesktop.UnitTests\\Microsoft.CmdPal.Ext.RemoteDesktop.UnitTests.csproj", "src\\modules\\cmdpal\\Tests\\Microsoft.CmdPal.Ext.Shell.UnitTests\\Microsoft.CmdPal.Ext.Shell.UnitTests.csproj", "src\\modules\\cmdpal\\Tests\\Microsoft.CmdPal.Ext.System.UnitTests\\Microsoft.CmdPal.Ext.System.UnitTests.csproj", @@ -31,8 +30,9 @@ "src\\modules\\cmdpal\\Tests\\Microsoft.CmdPal.Ext.UnitTestsBase\\Microsoft.CmdPal.Ext.UnitTestBase.csproj", "src\\modules\\cmdpal\\Tests\\Microsoft.CmdPal.Ext.WebSearch.UnitTests\\Microsoft.CmdPal.Ext.WebSearch.UnitTests.csproj", "src\\modules\\cmdpal\\Tests\\Microsoft.CmdPal.Ext.WindowWalker.UnitTests\\Microsoft.CmdPal.Ext.WindowWalker.UnitTests.csproj", + "src\\modules\\cmdpal\\Tests\\Microsoft.CmdPal.Ext.WindowsSettings.UnitTests\\Microsoft.CmdPal.Ext.WindowsSettings.UnitTests.csproj", + "src\\modules\\cmdpal\\Tests\\Microsoft.CmdPal.UI.UnitTests\\Microsoft.CmdPal.UI.UnitTests.csproj", "src\\modules\\cmdpal\\Tests\\Microsoft.CmdPal.UI.ViewModels.UnitTests\\Microsoft.CmdPal.UI.ViewModels.UnitTests.csproj", - "src\\modules\\cmdpal\\Tests\\Microsoft.CmdPal.UITests\\Microsoft.CmdPal.UITests.csproj", "src\\modules\\cmdpal\\Tests\\Microsoft.CommandPalette.Extensions.Toolkit.UnitTests\\Microsoft.CommandPalette.Extensions.Toolkit.UnitTests.csproj", "src\\modules\\cmdpal\\ext\\Microsoft.CmdPal.Ext.Actions\\Microsoft.CmdPal.Ext.Actions.csproj", "src\\modules\\cmdpal\\ext\\Microsoft.CmdPal.Ext.Apps\\Microsoft.CmdPal.Ext.Apps.csproj", diff --git a/src/modules/cmdpal/CommandPalette.slnf b/src/modules/cmdpal/CommandPalette.slnf index fd543f357db2..d6624fd0ad5b 100644 --- a/src/modules/cmdpal/CommandPalette.slnf +++ b/src/modules/cmdpal/CommandPalette.slnf @@ -34,6 +34,7 @@ "src\\modules\\cmdpal\\Tests\\Microsoft.CmdPal.Ext.WebSearch.UnitTests\\Microsoft.CmdPal.Ext.WebSearch.UnitTests.csproj", "src\\modules\\cmdpal\\Tests\\Microsoft.CmdPal.Ext.WindowWalker.UnitTests\\Microsoft.CmdPal.Ext.WindowWalker.UnitTests.csproj", "src\\modules\\cmdpal\\Tests\\Microsoft.CmdPal.Ext.WindowsSettings.UnitTests\\Microsoft.CmdPal.Ext.WindowsSettings.UnitTests.csproj", + "src\\modules\\cmdpal\\Tests\\Microsoft.CmdPal.UI.UnitTests\\Microsoft.CmdPal.UI.UnitTests.csproj", "src\\modules\\cmdpal\\Tests\\Microsoft.CmdPal.UI.ViewModels.UnitTests\\Microsoft.CmdPal.UI.ViewModels.UnitTests.csproj", "src\\modules\\cmdpal\\Tests\\Microsoft.CmdPal.UITests\\Microsoft.CmdPal.UITests.csproj", "src\\modules\\cmdpal\\Tests\\Microsoft.CommandPalette.Extensions.Toolkit.UnitTests\\Microsoft.CommandPalette.Extensions.Toolkit.UnitTests.csproj", diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/AdaptiveCache`2.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/AdaptiveCache`2.cs index 9cdf410872c9..706cf2b706e0 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/AdaptiveCache`2.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/AdaptiveCache`2.cs @@ -24,10 +24,15 @@ internal sealed class AdaptiveCache private readonly ConcurrentStack _pool = []; private readonly WaitCallback _maintenanceCallback; + // ConcurrentDictionary.Count acquires every stripe lock. Keep an approximate count so + // cache maintenance never makes the XAML UI thread wait for all dictionary locks. + private int _entryCount; private long _currentTick; private long _lastDecayTicks = DateTime.UtcNow.Ticks; private InterlockedBoolean _maintenanceSwitch = new(false); + internal int ApproximateCount => Volatile.Read(ref _entryCount); + public AdaptiveCache(int capacity = 384, TimeSpan? decayInterval = null, double decayFactor = 0.5) { _capacity = capacity; @@ -66,7 +71,11 @@ public TValue GetOrAdd(TKey key, Func factory, TArg ar var tick = Interlocked.Increment(ref _currentTick); newEntry.Initialize(key, value, 1.0, tick); - if (!_map.TryAdd(key, newEntry)) + if (_map.TryAdd(key, newEntry)) + { + Interlocked.Increment(ref _entryCount); + } + else { newEntry.Clear(); _pool.Push(newEntry); @@ -117,7 +126,11 @@ public void Add(TKey key, TValue value) newEntry.Initialize(key, value, 1.0, tick); - if (!_map.TryAdd(key, newEntry)) + if (_map.TryAdd(key, newEntry)) + { + Interlocked.Increment(ref _entryCount); + } + else { newEntry.Clear(); _pool.Push(newEntry); @@ -133,6 +146,7 @@ public bool TryRemove(TKey key) { if (_map.TryRemove(key, out var evicted)) { + Interlocked.Decrement(ref _entryCount); evicted.Clear(); _pool.Push(evicted); return true; @@ -143,7 +157,9 @@ public bool TryRemove(TKey key) public void Clear() { - foreach (var key in _map.Keys) + // Enumerate the dictionary rather than _map.Keys: the enumerator is lock-free, + // while Keys snapshots under every stripe lock. + foreach (var (key, _) in _map) { TryRemove(key); } @@ -153,7 +169,7 @@ public void Clear() private bool ShouldMaintenanceRun() { - return _map.Count > _capacity || (DateTime.UtcNow.Ticks - Interlocked.Read(ref _lastDecayTicks)) > _decayInterval.Ticks; + return ApproximateCount > _capacity || (DateTime.UtcNow.Ticks - Interlocked.Read(ref _lastDecayTicks)) > _decayInterval.Ticks; } private void TryRunMaintenance() @@ -184,13 +200,9 @@ private void PerformCleanup() var score = CalculateScore(entry, currentTick); - if (score < 0.1 || _map.Count > _capacity) + if (score < 0.1 || ApproximateCount > _capacity) { - if (_map.TryRemove(key, out var evicted)) - { - evicted.Clear(); - _pool.Push(evicted); - } + TryRemove(key); } } } diff --git a/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.UnitTests/AdaptiveCacheTests.cs b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.UnitTests/AdaptiveCacheTests.cs new file mode 100644 index 000000000000..cf3cd1d4dc6d --- /dev/null +++ b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.UnitTests/AdaptiveCacheTests.cs @@ -0,0 +1,102 @@ +// Copyright (c) Microsoft Corporation +// The Microsoft Corporation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Collections.Concurrent; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.CmdPal.UI.Helpers; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace Microsoft.CmdPal.UI.UnitTests; + +[TestClass] +public class AdaptiveCacheTests +{ + [TestMethod] + public void ApproximateCountChangesOnlyForSuccessfulMutations() + { + var cache = new AdaptiveCache(capacity: 8); + + cache.Add(1, 1); + cache.Add(1, 2); + _ = cache.GetOrAdd(1, static (key, _) => key, 0); + + Assert.AreEqual(1, cache.ApproximateCount); + Assert.IsFalse(cache.TryRemove(2)); + Assert.AreEqual(1, cache.ApproximateCount); + Assert.IsTrue(cache.TryRemove(1)); + Assert.AreEqual(0, cache.ApproximateCount); + + _ = cache.GetOrAdd(2, static (key, _) => key, 0); + Assert.AreEqual(1, cache.ApproximateCount); + + cache.Clear(); + Assert.AreEqual(0, cache.ApproximateCount); + } + + [TestMethod] + [Timeout(15_000)] + public async Task ConcurrentCleanupAndFailedLoadsKeepApproximateCountConsistent() + { + const int capacity = 1; + const int workerCount = 8; + const int itemsPerWorker = 256; + var cache = new AdaptiveCache>(capacity, TimeSpan.FromHours(1)); + var faultRemovals = new ConcurrentBag(); + + cache.Add(-2, Task.FromResult(-2)); + cache.Add(-1, Task.FromResult(-1)); + + Assert.IsTrue( + SpinWait.SpinUntil(() => cache.ApproximateCount <= capacity, TimeSpan.FromSeconds(5)), + "Capacity cleanup did not run."); + + var workers = Enumerable.Range(0, workerCount) + .Select(worker => Task.Run(() => + { + for (var item = 0; item < itemsPerWorker; item++) + { + var key = (worker * itemsPerWorker) + item; + + if (item % 4 == 0) + { + var completionSource = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + cache.Add(key, completionSource.Task); + + faultRemovals.Add(completionSource.Task.ContinueWith( + completedTask => + { + _ = completedTask.Exception; + cache.TryRemove(key); + }, + CancellationToken.None, + TaskContinuationOptions.OnlyOnFaulted, + TaskScheduler.Default)); + + completionSource.SetException(new InvalidOperationException("Icon load failed.")); + } + else + { + _ = cache.GetOrAdd(key, static (currentKey, _) => Task.FromResult(currentKey), 0); + } + + _ = cache.TryGet(key, out _); + } + })) + .ToArray(); + + await Task.WhenAll(workers); + await Task.WhenAll(faultRemovals); + + cache.Clear(); + + Assert.AreEqual(0, cache.ApproximateCount); + for (var key = 0; key < workerCount * itemsPerWorker; key++) + { + Assert.IsFalse(cache.TryGet(key, out _), $"Cache still contains key {key} after Clear."); + } + } +} diff --git a/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.UnitTests/Microsoft.CmdPal.UI.UnitTests.csproj b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.UnitTests/Microsoft.CmdPal.UI.UnitTests.csproj new file mode 100644 index 000000000000..eda2f4893985 --- /dev/null +++ b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.UnitTests/Microsoft.CmdPal.UI.UnitTests.csproj @@ -0,0 +1,28 @@ + + + + + + false + true + Microsoft.CmdPal.UI.UnitTests + $(RepoRoot)$(Platform)\$(Configuration)\WinUI3Apps\CmdPal\tests\ + false + false + enable + preview + enable + + + + + + + + + + + + + +