mirror of
https://github.com/microsoft/PowerToys.git
synced 2025-12-15 03:07:56 +01:00
[CmdPal] Optimise MainListPage's results display by merging already-sorted lists (#44126)
## Summary of the Pull Request This PR replaces the current LINQ-based results compilation query of combining, sorting and filtering the four result sources with a 3-way merge operation plus a final append. It provides a performance increase as well as a significant reduction in allocations. <!-- Please review the items on the PR checklist before submitting--> ## PR Checklist - [ ] Closes: #xxx <!-- - [ ] Closes: #yyy (add separate lines for additional resolved issues) --> - [ ] **Communication:** I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected - [x] **Tests:** Added/updated and all pass - [ ] **Localization:** All end-user-facing strings can be localized - [ ] **Dev docs:** Added/updated - [ ] **New binaries:** Added on the required places - [ ] [JSON for signing](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ESRPSigning_core.json) for new binaries - [ ] [WXS for installer](https://github.com/microsoft/PowerToys/blob/main/installer/PowerToysSetup/Product.wxs) for new binaries and localization folder - [ ] [YML for CI pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ci/templates/build-powertoys-steps.yml) for new test projects - [ ] [YML for signed pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/release.yml) - [ ] **Documentation updated:** If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/windows-uwp/tree/docs/hub/powertoys) and link it here: #xxx <!-- Provide a more detailed description of the PR, other things fixed, or any additional comments/features here --> ## Detailed Description of the Pull Request / Additional comments The existing code: 1. Limits the number of apps returned to a pre-defined maximum. 2. Sorts the apps list. 3. Appends filtered items, scored fallback items and the apps list together. 4. Sorts the three lists based on their score. 5. Appends the non-scored fallback items, with empty items excluded. 6. Selects just the `Item` from each. 7. Creates an array from the enumerable. ```csharp if (_filteredApps?.Count > 0) { limitedApps = _filteredApps.OrderByDescending(s => s.Score).Take(_appResultLimit).ToList(); } var items = Enumerable.Empty<Scored<IListItem>>() .Concat(_filteredItems is not null ? _filteredItems : []) .Concat(_scoredFallbackItems is not null ? _scoredFallbackItems : []) .Concat(limitedApps) .OrderByDescending(o => o.Score) // Add fallback items post-sort so they are always at the end of the list // and eventually ordered based on user preference .Concat(_fallbackItems is not null ? _fallbackItems.Where(w => !string.IsNullOrEmpty(w.Item.Title)) : []) .Select(s => s.Item) .ToArray(); ``` We can exploit the fact that each of the three 'scored' lists are pre-ordered, and replace the query with a 3-way merge and final append of the non-scored fallback items. By pre-sizing the results array we can avoid all the extra allocations of the LINQ-based solution. ### Proof of pre-ordering In `UpdateSearchText`, each of the lists is defined by calling `ListHelpers.FilterListWithScores`: ```csharp // Produce a list of everything that matches the current filter. _filteredItems = [.. ListHelpers.FilterListWithScores<IListItem>(newFilteredItems ?? [], SearchText, scoreItem)]; ``` ```csharp _scoredFallbackItems = ListHelpers.FilterListWithScores<IListItem>(newFallbacksForScoring ?? [], SearchText, scoreItem); ``` ```csharp var scoredApps = ListHelpers.FilterListWithScores<IListItem>(newApps, SearchText, scoreItem); ... _filteredApps = [.. scoredApps]; ``` In `FilterListWithScores`, the results are ordered by score: ```csharp var scores = items .Select(li => new Scored<T>() { Item = li, Score = scoreFunction(query, li) }) .Where(score => score.Score > 0) .OrderByDescending(score => score.Score); ``` (This also makes the existing `OrderByDescending()` for `_filteredApps` before the LINQ query redundant.) ### K-way merge Since the results are pre-sorted, we can do a direct merge in linear time. This is what the new `MainListPageResultFactory`'s `Create` achieves. As the lists may be different sizes, the routine does a 3-way merge, followed by a 2-way merge and a single list drain to finish. Each element is only visited once. ### Benchmarks A separate benchmark project is [here](https://github.com/daverayment/MainListBench), written with Benchmark.net. The project compares the current LINQ-based solution against: 1. An Array-based algorithm which pre-assigns a results array and still sorts the 3 scored sets of results. This shows a naive non-LINQ solution which is still _O(n log n)_ because of the sort. 2. The k-way merge, which is described above. _O(n)_ for both time and space complexity. 3. A heap merge algorithm, which uses a priority queue instead of tracking each of the lists separately. (This is _O(n log k)_ in terms of time complexity and _O(n + k)_ for space.) Care is taken to ensure stable sorting of items. When preparing the benchmark data, items with identical scores are assigned to confirm each algorithm performs identically to the LINQ `OrderBy` approach, which performs a stable sort. Results show that the merge performs best in terms of both runtime performance and allocations, sometimes by a significant margin. Compared to the LINQ approach, merge runs 400%+ faster and with at most ~20% of the allocations: <img width="1135" height="556" alt="image" src="https://github.com/user-attachments/assets/9f9d3932-1592-49d6-8a07-4ea3ba7a0cc5" /> <img width="1149" height="553" alt="image" src="https://github.com/user-attachments/assets/ae9e9e0a-b255-4c1a-af4b-e791dea80fa4" /> See here for all charts and raw stats from the run: https://docs.google.com/spreadsheets/d/1y2mmWe8dfpbLxF_eqPbEGvaItmqp6HLfSp-rw99hzWg/edit?usp=sharing ### Cons 1. Existing performance is not currently an issue. This could be seen as a premature optimisation. 2. The new code introduces an inherent contract between the results compilation routine and the lists, i.e. that they must be sorted. This PR was really for research and learning more about CmdPal (and a bit of algorithm practice because it's Advent of Code time), so please feel free to reject if you feel the cons outweigh the pros. <!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well --> ## Validation Steps Performed - Added unit tests to exercise the new code, which confirm that the specific ordering is preserved, and the filtering and pre-trimming of the apps list is performed as before. - Existing non-UI unit tests run. NB: I _could not_ run any UI Tests on my system and just got an early bail-out each time. - Manual testing in (non-AOT) Release mode.
This commit is contained in:
@@ -12,6 +12,7 @@ using Microsoft.CmdPal.Core.ViewModels.Messages;
|
||||
using Microsoft.CmdPal.Ext.Apps;
|
||||
using Microsoft.CmdPal.Ext.Apps.Programs;
|
||||
using Microsoft.CmdPal.Ext.Apps.State;
|
||||
using Microsoft.CmdPal.UI.ViewModels.Commands;
|
||||
using Microsoft.CmdPal.UI.ViewModels.Messages;
|
||||
using Microsoft.CmdPal.UI.ViewModels.Properties;
|
||||
using Microsoft.CommandPalette.Extensions;
|
||||
@@ -44,6 +45,9 @@ public partial class MainListPage : DynamicListPage,
|
||||
private List<Scored<IListItem>>? _filteredItems;
|
||||
private List<Scored<IListItem>>? _filteredApps;
|
||||
private List<Scored<IListItem>>? _fallbackItems;
|
||||
|
||||
// Keep as IEnumerable for deferred execution. Fallback item titles are updated
|
||||
// asynchronously, so scoring must happen lazily when GetItems is called.
|
||||
private IEnumerable<Scored<IListItem>>? _scoredFallbackItems;
|
||||
private bool _includeApps;
|
||||
private bool _filteredItemsIncludesApps;
|
||||
@@ -155,42 +159,18 @@ public partial class MainListPage : DynamicListPage,
|
||||
|
||||
public override IListItem[] GetItems()
|
||||
{
|
||||
if (string.IsNullOrEmpty(SearchText))
|
||||
lock (_tlcManager.TopLevelCommands)
|
||||
{
|
||||
lock (_tlcManager.TopLevelCommands)
|
||||
{
|
||||
return _tlcManager
|
||||
.TopLevelCommands
|
||||
.Where(tlc => !tlc.IsFallback && !string.IsNullOrEmpty(tlc.Title))
|
||||
.ToArray();
|
||||
}
|
||||
}
|
||||
else
|
||||
{
|
||||
lock (_tlcManager.TopLevelCommands)
|
||||
{
|
||||
var limitedApps = new List<Scored<IListItem>>();
|
||||
|
||||
// Fuzzy matching can produce a lot of results, so we want to limit the
|
||||
// number of apps we show at once if it's a large set.
|
||||
if (_filteredApps?.Count > 0)
|
||||
{
|
||||
limitedApps = _filteredApps.OrderByDescending(s => s.Score).Take(_appResultLimit).ToList();
|
||||
}
|
||||
|
||||
var items = Enumerable.Empty<Scored<IListItem>>()
|
||||
.Concat(_filteredItems is not null ? _filteredItems : [])
|
||||
.Concat(_scoredFallbackItems is not null ? _scoredFallbackItems : [])
|
||||
.Concat(limitedApps)
|
||||
.OrderByDescending(o => o.Score)
|
||||
|
||||
// Add fallback items post-sort so they are always at the end of the list
|
||||
// and eventually ordered based on user preference
|
||||
.Concat(_fallbackItems is not null ? _fallbackItems.Where(w => !string.IsNullOrEmpty(w.Item.Title)) : [])
|
||||
.Select(s => s.Item)
|
||||
.ToArray();
|
||||
return items;
|
||||
}
|
||||
// Either return the top-level commands (no search text), or the merged and
|
||||
// filtered results.
|
||||
return string.IsNullOrEmpty(SearchText)
|
||||
? _tlcManager.TopLevelCommands.Where(tlc => !tlc.IsFallback && !string.IsNullOrEmpty(tlc.Title)).ToArray()
|
||||
: MainListPageResultFactory.Create(
|
||||
_filteredItems,
|
||||
_scoredFallbackItems?.ToList(),
|
||||
_filteredApps,
|
||||
_fallbackItems,
|
||||
_appResultLimit);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,156 @@
|
||||
// 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.
|
||||
|
||||
#pragma warning disable IDE0007 // Use implicit type
|
||||
|
||||
using Microsoft.CommandPalette.Extensions;
|
||||
using Microsoft.CommandPalette.Extensions.Toolkit;
|
||||
|
||||
namespace Microsoft.CmdPal.UI.ViewModels.Commands;
|
||||
|
||||
internal static class MainListPageResultFactory
|
||||
{
|
||||
/// <summary>
|
||||
/// Creates a merged and ordered array of results from multiple scored input lists,
|
||||
/// applying an application result limit and filtering fallback items as needed.
|
||||
/// </summary>
|
||||
public static IListItem[] Create(
|
||||
IList<Scored<IListItem>>? filteredItems,
|
||||
IList<Scored<IListItem>>? scoredFallbackItems,
|
||||
IList<Scored<IListItem>>? filteredApps,
|
||||
IList<Scored<IListItem>>? fallbackItems,
|
||||
int appResultLimit)
|
||||
{
|
||||
if (appResultLimit < 0)
|
||||
{
|
||||
throw new ArgumentOutOfRangeException(
|
||||
nameof(appResultLimit), "App result limit must be non-negative.");
|
||||
}
|
||||
|
||||
int len1 = filteredItems?.Count ?? 0;
|
||||
int len2 = scoredFallbackItems?.Count ?? 0;
|
||||
|
||||
// Apps are pre-sorted, so we just need to take the top N, limited by appResultLimit.
|
||||
int len3 = Math.Min(filteredApps?.Count ?? 0, appResultLimit);
|
||||
|
||||
// Allocate the exact size of the result array.
|
||||
int totalCount = len1 + len2 + len3 + GetNonEmptyFallbackItemsCount(fallbackItems);
|
||||
var result = new IListItem[totalCount];
|
||||
|
||||
// Three-way stable merge of already-sorted lists.
|
||||
int idx1 = 0, idx2 = 0, idx3 = 0;
|
||||
int writePos = 0;
|
||||
|
||||
// Merge while all three lists have items. To maintain a stable sort, the
|
||||
// priority is: list1 > list2 > list3 when scores are equal.
|
||||
while (idx1 < len1 && idx2 < len2 && idx3 < len3)
|
||||
{
|
||||
// Using null-forgiving operator as we have already checked against lengths.
|
||||
int score1 = filteredItems![idx1].Score;
|
||||
int score2 = scoredFallbackItems![idx2].Score;
|
||||
int score3 = filteredApps![idx3].Score;
|
||||
|
||||
if (score1 >= score2 && score1 >= score3)
|
||||
{
|
||||
result[writePos++] = filteredItems[idx1++].Item;
|
||||
}
|
||||
else if (score2 >= score3)
|
||||
{
|
||||
result[writePos++] = scoredFallbackItems[idx2++].Item;
|
||||
}
|
||||
else
|
||||
{
|
||||
result[writePos++] = filteredApps[idx3++].Item;
|
||||
}
|
||||
}
|
||||
|
||||
// Two-way merges for remaining pairs.
|
||||
while (idx1 < len1 && idx2 < len2)
|
||||
{
|
||||
if (filteredItems![idx1].Score >= scoredFallbackItems![idx2].Score)
|
||||
{
|
||||
result[writePos++] = filteredItems[idx1++].Item;
|
||||
}
|
||||
else
|
||||
{
|
||||
result[writePos++] = scoredFallbackItems[idx2++].Item;
|
||||
}
|
||||
}
|
||||
|
||||
while (idx1 < len1 && idx3 < len3)
|
||||
{
|
||||
if (filteredItems![idx1].Score >= filteredApps![idx3].Score)
|
||||
{
|
||||
result[writePos++] = filteredItems[idx1++].Item;
|
||||
}
|
||||
else
|
||||
{
|
||||
result[writePos++] = filteredApps[idx3++].Item;
|
||||
}
|
||||
}
|
||||
|
||||
while (idx2 < len2 && idx3 < len3)
|
||||
{
|
||||
if (scoredFallbackItems![idx2].Score >= filteredApps![idx3].Score)
|
||||
{
|
||||
result[writePos++] = scoredFallbackItems[idx2++].Item;
|
||||
}
|
||||
else
|
||||
{
|
||||
result[writePos++] = filteredApps[idx3++].Item;
|
||||
}
|
||||
}
|
||||
|
||||
// Drain remaining items from a non-empty list.
|
||||
while (idx1 < len1)
|
||||
{
|
||||
result[writePos++] = filteredItems![idx1++].Item;
|
||||
}
|
||||
|
||||
while (idx2 < len2)
|
||||
{
|
||||
result[writePos++] = scoredFallbackItems![idx2++].Item;
|
||||
}
|
||||
|
||||
while (idx3 < len3)
|
||||
{
|
||||
result[writePos++] = filteredApps![idx3++].Item;
|
||||
}
|
||||
|
||||
// Append filtered fallback items. Fallback items are added post-sort so they are
|
||||
// always at the end of the list and eventually ordered based on user preference.
|
||||
if (fallbackItems is not null)
|
||||
{
|
||||
for (int i = 0; i < fallbackItems.Count; i++)
|
||||
{
|
||||
var item = fallbackItems[i].Item;
|
||||
if (!string.IsNullOrEmpty(item.Title))
|
||||
{
|
||||
result[writePos++] = item;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return result;
|
||||
}
|
||||
|
||||
private static int GetNonEmptyFallbackItemsCount(IList<Scored<IListItem>>? fallbackItems)
|
||||
{
|
||||
int fallbackItemsCount = 0;
|
||||
|
||||
if (fallbackItems is not null)
|
||||
{
|
||||
for (int i = 0; i < fallbackItems.Count; i++)
|
||||
{
|
||||
if (!string.IsNullOrEmpty(fallbackItems[i].Item.Title))
|
||||
{
|
||||
fallbackItemsCount++;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return fallbackItemsCount;
|
||||
}
|
||||
}
|
||||
#pragma warning restore IDE0007 // Use implicit type
|
||||
@@ -0,0 +1,161 @@
|
||||
// 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.Generic;
|
||||
using System.Linq;
|
||||
using Microsoft.CmdPal.UI.ViewModels.Commands;
|
||||
using Microsoft.CommandPalette.Extensions;
|
||||
using Microsoft.CommandPalette.Extensions.Toolkit;
|
||||
using Microsoft.VisualStudio.TestTools.UnitTesting;
|
||||
using Windows.Foundation;
|
||||
|
||||
namespace Microsoft.CmdPal.UI.ViewModels.UnitTests;
|
||||
|
||||
[TestClass]
|
||||
public partial class MainListPageResultFactoryTests
|
||||
{
|
||||
private sealed partial class MockListItem : IListItem
|
||||
{
|
||||
public string Title { get; set; } = string.Empty;
|
||||
|
||||
public string Subtitle { get; set; } = string.Empty;
|
||||
|
||||
public ICommand Command => new NoOpCommand();
|
||||
|
||||
public IDetails? Details => null;
|
||||
|
||||
public IIconInfo? Icon => null;
|
||||
|
||||
public string Section => throw new NotImplementedException();
|
||||
|
||||
public ITag[] Tags => throw new NotImplementedException();
|
||||
|
||||
public string TextToSuggest => throw new NotImplementedException();
|
||||
|
||||
public IContextItem[] MoreCommands => throw new NotImplementedException();
|
||||
|
||||
#pragma warning disable CS0067 // The event is never used
|
||||
public event TypedEventHandler<object, IPropChangedEventArgs>? PropChanged;
|
||||
#pragma warning restore CS0067 // The event is never used
|
||||
|
||||
public override string ToString() => Title;
|
||||
}
|
||||
|
||||
private static Scored<IListItem> S(string title, int score)
|
||||
{
|
||||
return new Scored<IListItem>
|
||||
{
|
||||
Score = score,
|
||||
Item = new MockListItem { Title = title },
|
||||
};
|
||||
}
|
||||
|
||||
[TestMethod]
|
||||
public void Merge_PrioritizesListsCorrectly()
|
||||
{
|
||||
var filtered = new List<Scored<IListItem>>
|
||||
{
|
||||
S("F1", 100),
|
||||
S("F2", 50),
|
||||
};
|
||||
|
||||
var scoredFallback = new List<Scored<IListItem>>
|
||||
{
|
||||
S("SF1", 100),
|
||||
S("SF2", 60),
|
||||
};
|
||||
|
||||
var apps = new List<Scored<IListItem>>
|
||||
{
|
||||
S("A1", 100),
|
||||
S("A2", 55),
|
||||
};
|
||||
|
||||
// Fallbacks are not scored.
|
||||
var fallbacks = new List<Scored<IListItem>>
|
||||
{
|
||||
S("FB1", 0),
|
||||
S("FB2", 0),
|
||||
};
|
||||
|
||||
var result = MainListPageResultFactory.Create(
|
||||
filtered,
|
||||
scoredFallback,
|
||||
apps,
|
||||
fallbacks,
|
||||
appResultLimit: 10);
|
||||
|
||||
// Expected order:
|
||||
// 100: F1, SF1, A1
|
||||
// 60: SF2
|
||||
// 55: A2
|
||||
// 50: F2
|
||||
// Then fallbacks in original order: FB1, FB2
|
||||
var titles = result.Select(r => r.Title).ToArray();
|
||||
#pragma warning disable CA1861 // Avoid constant arrays as arguments
|
||||
CollectionAssert.AreEqual(
|
||||
new[] { "F1", "SF1", "A1", "SF2", "A2", "F2", "FB1", "FB2" },
|
||||
titles);
|
||||
#pragma warning restore CA1861 // Avoid constant arrays as arguments
|
||||
}
|
||||
|
||||
[TestMethod]
|
||||
public void Merge_AppliesAppLimit()
|
||||
{
|
||||
var apps = new List<Scored<IListItem>>
|
||||
{
|
||||
S("A1", 100),
|
||||
S("A2", 90),
|
||||
S("A3", 80),
|
||||
};
|
||||
|
||||
var result = MainListPageResultFactory.Create(
|
||||
null,
|
||||
null,
|
||||
apps,
|
||||
null,
|
||||
2);
|
||||
|
||||
Assert.AreEqual(2, result.Length);
|
||||
Assert.AreEqual("A1", result[0].Title);
|
||||
Assert.AreEqual("A2", result[1].Title);
|
||||
}
|
||||
|
||||
[TestMethod]
|
||||
public void Merge_FiltersEmptyFallbacks()
|
||||
{
|
||||
var fallbacks = new List<Scored<IListItem>>
|
||||
{
|
||||
S("FB1", 0),
|
||||
S(string.Empty, 0),
|
||||
S("FB3", 0),
|
||||
};
|
||||
|
||||
var result = MainListPageResultFactory.Create(
|
||||
null,
|
||||
null,
|
||||
null,
|
||||
fallbacks,
|
||||
appResultLimit: 10);
|
||||
|
||||
Assert.AreEqual(2, result.Length);
|
||||
Assert.AreEqual("FB1", result[0].Title);
|
||||
Assert.AreEqual("FB3", result[1].Title);
|
||||
}
|
||||
|
||||
[TestMethod]
|
||||
public void Merge_HandlesNullLists()
|
||||
{
|
||||
var result = MainListPageResultFactory.Create(
|
||||
null,
|
||||
null,
|
||||
null,
|
||||
null,
|
||||
appResultLimit: 10);
|
||||
|
||||
Assert.IsNotNull(result);
|
||||
Assert.AreEqual(0, result.Length);
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user