From 0997c1a013766b046547fea0414c82cf153db707 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ji=C5=99=C3=AD=20Pol=C3=A1=C5=A1ek?= Date: Tue, 5 Aug 2025 23:26:50 +0200 Subject: [PATCH] CmdPal: Coalesce top-level commands list changes into a single task (#40943) ## Summary of the Pull Request Self-refresh of `MainListPage` introduced in #40132 causes unnecessary spawning of tasks by `ReapplySearchInBackground` and pushing the code down the scenic route instead of taking shortcut. This drop-in fix introduces a single-worker coalescing refresh loop to eliminate thread-pool churn and syncs state in early-return paths. ## PR Checklist - [x] Closes: #40916 - [x] **Communication:** I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected - [ ] **Tests:** no change - [ ] **Localization:** nothing - [ ] **Dev docs:** nothing - [ ] **New binaries:** none - [ ] **Documentation updated:** nothing ## Detailed Description of the Pull Request / Additional comments ## Validation Steps Performed --- .../Commands/MainListPage.cs | 53 +++++++++++++++++-- 1 file changed, 48 insertions(+), 5 deletions(-) diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Commands/MainListPage.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Commands/MainListPage.cs index af2a3d76be..71c0a4e810 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Commands/MainListPage.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Commands/MainListPage.cs @@ -6,6 +6,7 @@ using System.Collections.Immutable; using System.Collections.Specialized; using CommunityToolkit.Mvvm.Messaging; using ManagedCommon; +using Microsoft.CmdPal.Common.Helpers; using Microsoft.CmdPal.Core.ViewModels.Messages; using Microsoft.CmdPal.Ext.Apps; using Microsoft.CommandPalette.Extensions; @@ -29,6 +30,9 @@ public partial class MainListPage : DynamicListPage, private bool _includeApps; private bool _filteredItemsIncludesApps; + private InterlockedBoolean _refreshRunning; + private InterlockedBoolean _refreshRequested; + public MainListPage(IServiceProvider serviceProvider) { Icon = IconHelpers.FromRelativePath("Assets\\StoreLogo.scale-200.png"); @@ -83,18 +87,47 @@ public partial class MainListPage : DynamicListPage, private void ReapplySearchInBackground() { - _ = Task.Run(() => + _refreshRequested.Set(); + if (!_refreshRunning.Set()) { - try + return; + } + + _ = Task.Run(RunRefreshLoop); + } + + private void RunRefreshLoop() + { + try + { + do { + _refreshRequested.Clear(); + lock (_tlcManager.TopLevelCommands) + { + if (_filteredItemsIncludesApps == _includeApps) + { + break; + } + } + var currentSearchText = SearchText; UpdateSearchText(currentSearchText, currentSearchText); } - catch (Exception e) + while (_refreshRequested.Value); + } + catch (Exception e) + { + Logger.LogError("Failed to reload search", e); + } + finally + { + _refreshRunning.Clear(); + if (_refreshRequested.Value && _refreshRunning.Set()) { - Logger.LogError("Failed to reload search", e); + _ = Task.Run(RunRefreshLoop); } - }); + } } public override IListItem[] GetItems() @@ -126,6 +159,15 @@ public partial class MainListPage : DynamicListPage, var aliases = _serviceProvider.GetService()!; if (aliases.CheckAlias(newSearch)) { + if (_filteredItemsIncludesApps != _includeApps) + { + lock (_tlcManager.TopLevelCommands) + { + _filteredItemsIncludesApps = _includeApps; + _filteredItems = null; + } + } + return; } } @@ -138,6 +180,7 @@ public partial class MainListPage : DynamicListPage, // Cleared out the filter text? easy. Reset _filteredItems, and bail out. if (string.IsNullOrEmpty(newSearch)) { + _filteredItemsIncludesApps = _includeApps; _filteredItems = null; RaiseItemsChanged(commands.Count); return;