Skip to content

Commit ebe8c11

Browse files
[CmdPal] Phase 7b: parallel + off-lock search scoring (same settled order)
Throughput and render-unblocking for the main-list search scoring, with the final settled result order kept byte-identical to phase 7a. - Shrink the TopLevelCommands lock scope in MainListPage.UpdateSearchTextCore. Sources are now snapshotted under the lock, the heavy scoring/sort runs OFF the lock, and the lock is re-acquired only to publish the finished results. The swap is atomic and supersede-safe via the existing cancellation-token guard, so a newer keystroke always wins and a stale snapshot is dropped. - Parallelize the dominant apps scoring pass via FilterListWithScoresParallel. Contiguous partitions are scored on separate threads and concatenated in partition order, producing a pre-sort buffer identical to the serial path; the same Array.Sort then yields a byte-identical ordered result. Commands and fallbacks stay serial. RecentCommandsManager.Index is pre-warmed single-threaded (PrewarmIndex) before the parallel loop because its lazy dictionary build is not thread-safe. - Drop the redundant second commands.Where(IsFallback) pass by reusing the specialFallbacks list already built in the prefilter loop. - Add a committed output-equivalence MSTest (ScoringParallelEquivalenceTests) asserting the parallel path matches a sequential reference item-for-item and score-for-score across full-catalog, extend, retype-rebuild, commands, and determinism cases. Extend the throughput harness with a serial-vs-parallel before/after measurement. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent 5ff01db commit ebe8c11

5 files changed

Lines changed: 585 additions & 74 deletions

File tree

src/modules/cmdpal/Microsoft.CmdPal.Common/Helpers/InternalListHelpers.cs

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,90 @@ public static RoScored<T>[] FilterListWithScores<T>(
6060
}
6161
}
6262

63+
// Minimum item count before the parallel path is worth its partitioning/merge overhead.
64+
// Below this the serial path wins, so commands (hundreds) and small app sets stay serial.
65+
private const int ParallelScoringThreshold = 512;
66+
67+
/// <summary>
68+
/// Order-preserving parallel variant of <see cref="FilterListWithScores{T}"/>. Partitions the
69+
/// input into contiguous index ranges, scores each range on a separate thread, then
70+
/// concatenates the per-partition matched items back in partition order. This produces a
71+
/// pre-sort buffer BYTE-IDENTICAL to the serial path (matched items in original enumeration
72+
/// order), so the subsequent <see cref="Array.Sort(Array, int, int, IComparer)"/> - which is
73+
/// deterministic for a given input - yields the exact same ordered result. The scoring
74+
/// function must be pure over each item (each item is scored by exactly one thread, so any
75+
/// per-item cached state is never touched concurrently). Falls back to the serial path for
76+
/// small inputs where partitioning would not pay off.
77+
/// </summary>
78+
public static RoScored<T>[] FilterListWithScoresParallel<T>(
79+
IReadOnlyList<T>? items,
80+
in FuzzyQuery query,
81+
in ScoringFunction<T> scoreFunction)
82+
{
83+
if (items is null || items.Count == 0)
84+
{
85+
return [];
86+
}
87+
88+
var count = items.Count;
89+
var partitions = Math.Min(Environment.ProcessorCount, Math.Max(1, count / ParallelScoringThreshold));
90+
91+
if (count < ParallelScoringThreshold || partitions <= 1)
92+
{
93+
return FilterListWithScores(items, query, scoreFunction);
94+
}
95+
96+
// Copy the by-ref parameters into locals so they can be captured by the parallel body.
97+
// FuzzyQuery is an immutable value read concurrently (never mutated), so sharing one copy
98+
// across threads is safe.
99+
var q = query;
100+
var fn = scoreFunction;
101+
var source = items;
102+
103+
var partitionResults = new List<RoScored<T>>[partitions];
104+
105+
System.Threading.Tasks.Parallel.For(0, partitions, p =>
106+
{
107+
var start = (int)((long)p * count / partitions);
108+
var end = (int)((long)(p + 1) * count / partitions);
109+
110+
var local = new List<RoScored<T>>(end - start);
111+
for (var i = start; i < end; i++)
112+
{
113+
var item = source[i];
114+
var score = fn(in q, item);
115+
if (score > 0)
116+
{
117+
local.Add(new RoScored<T>(item, score));
118+
}
119+
}
120+
121+
partitionResults[p] = local;
122+
});
123+
124+
var total = 0;
125+
for (var p = 0; p < partitions; p++)
126+
{
127+
total += partitionResults[p].Count;
128+
}
129+
130+
// Concatenate partitions in order. Contiguous ranges merged in partition order reproduce
131+
// the exact original enumeration order of the matched items, matching the serial buffer.
132+
var buffer = GC.AllocateUninitializedArray<RoScored<T>>(total);
133+
var pos = 0;
134+
for (var p = 0; p < partitions; p++)
135+
{
136+
var list = partitionResults[p];
137+
for (var j = 0; j < list.Count; j++)
138+
{
139+
buffer[pos++] = list[j];
140+
}
141+
}
142+
143+
Array.Sort(buffer, 0, total, default(RoScoredDescendingComparer<T>));
144+
return buffer;
145+
}
146+
63147
private static void GrowBuffer<T>(ref RoScored<T>[] buffer, int count)
64148
{
65149
var newBuffer = ArrayPool<RoScored<T>>.Shared.Rent(buffer.Length * 2);

src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Commands/MainListPage.cs

Lines changed: 108 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -500,6 +500,19 @@ private void UpdateSearchTextCore(string oldSearch, string newSearch, bool isUse
500500
}
501501

502502
var commands = _tlcManager.TopLevelCommands;
503+
504+
// Materialized inputs captured under the lock so the heavy scoring below can run OFF the
505+
// lock. Scoring no longer converts directly into blocked settle/render time, because
506+
// GetItems() takes the SAME lock and now only contends with the short snapshot/publish
507+
// sections rather than the whole multi-thousand-item scoring pass.
508+
IReadOnlyList<IListItem> itemsSource;
509+
IReadOnlyList<IListItem> appsSource;
510+
IReadOnlyList<IListItem> fallbackSource;
511+
IListItem[] globalFallbackSources;
512+
bool includeAppsSnapshot;
513+
bool tookFullCatalog = false;
514+
515+
// ===== SNAPSHOT PHASE (under lock) =====
503516
lock (commands)
504517
{
505518
if (token.IsCancellationRequested)
@@ -553,52 +566,28 @@ private void UpdateSearchTextCore(string oldSearch, string newSearch, bool isUse
553566
return;
554567
}
555568

556-
// If the new string doesn't start with the old string, then we can't
557-
// re-use previous results. Reset _filteredItems, and keep er moving.
558-
if (!newSearch.StartsWith(oldSearch, StringComparison.CurrentCultureIgnoreCase))
559-
{
560-
ClearResults();
561-
}
562-
563-
// If the internal state has changed, reset _filteredItems to reset the list.
564-
if (_filteredItemsIncludesApps != _includeApps)
565-
{
566-
ClearResults();
567-
}
568-
569-
if (token.IsCancellationRequested)
570-
{
571-
return;
572-
}
573-
574-
var newFilteredItems = Enumerable.Empty<IListItem>();
575-
var newFallbacks = Enumerable.Empty<IListItem>();
576-
var newApps = Enumerable.Empty<IListItem>();
577-
578-
if (_filteredItems is not null)
579-
{
580-
newFilteredItems = _filteredItems.Select(s => s.Item);
581-
}
582-
583-
if (token.IsCancellationRequested)
584-
{
585-
return;
586-
}
587-
588-
if (_filteredApps is not null)
589-
{
590-
newApps = _filteredApps.Select(s => s.Item);
591-
}
592-
593-
if (token.IsCancellationRequested)
594-
{
595-
return;
596-
}
597-
598-
if (_fallbackItems is not null)
599-
{
600-
newFallbacks = _fallbackItems.Select(s => s.Item);
601-
}
569+
includeAppsSnapshot = _includeApps;
570+
571+
// If the new string doesn't start with the old string we can't re-use previous
572+
// results; likewise if the app-inclusion state changed. Either way we rebuild from the
573+
// full catalog. On an extend (the new query starts with the old and app state is
574+
// unchanged) we re-score only the previously matched subset, not the whole catalog.
575+
var reset = !newSearch.StartsWith(oldSearch, StringComparison.CurrentCultureIgnoreCase)
576+
|| _filteredItemsIncludesApps != includeAppsSnapshot;
577+
578+
var prevFilteredItems = reset ? null : _filteredItems;
579+
var prevApps = reset ? null : _filteredApps;
580+
var prevFallbacks = reset ? null : _fallbackItems;
581+
582+
IEnumerable<IListItem> newFilteredItems = prevFilteredItems is not null
583+
? prevFilteredItems.Select(s => s.Item)
584+
: Enumerable.Empty<IListItem>();
585+
IEnumerable<IListItem> newApps = prevApps is not null
586+
? prevApps.Select(s => s.Item)
587+
: Enumerable.Empty<IListItem>();
588+
IEnumerable<IListItem> newFallbacks = prevFallbacks is not null
589+
? prevFallbacks.Select(s => s.Item)
590+
: Enumerable.Empty<IListItem>();
602591

603592
if (token.IsCancellationRequested)
604593
{
@@ -609,6 +598,7 @@ private void UpdateSearchTextCore(string oldSearch, string newSearch, bool isUse
609598
// with a list of all our commands & apps.
610599
if (!newFilteredItems.Any() && !newApps.Any())
611600
{
601+
tookFullCatalog = true;
612602
newFilteredItems = commands.Where(s => !s.IsFallback);
613603

614604
// Fallbacks are always included in the list, even if they
@@ -621,9 +611,7 @@ private void UpdateSearchTextCore(string oldSearch, string newSearch, bool isUse
621611
return;
622612
}
623613

624-
_filteredItemsIncludesApps = _includeApps;
625-
626-
if (_includeApps)
614+
if (includeAppsSnapshot)
627615
{
628616
var allNewApps = AllAppsCommandProvider.Page.GetItems().Cast<AppListItem>().ToList();
629617

@@ -646,49 +634,88 @@ private void UpdateSearchTextCore(string oldSearch, string newSearch, bool isUse
646634
}
647635
}
648636

649-
var searchQuery = _fuzzyMatcherProvider.Current.PrecomputeQuery(SearchText);
637+
// Materialize every source sequence while still under the lock. After this point the
638+
// scoring passes never touch the live TopLevelCommands collection or the app provider,
639+
// so they are free to run off the lock. globalFallbackSources reuses the specialFallbacks
640+
// list already built above, dropping the redundant second commands.Where(...) pass.
641+
itemsSource = MaterializeSource(newFilteredItems);
642+
appsSource = MaterializeSource(newApps);
643+
fallbackSource = MaterializeSource(newFallbacks);
644+
globalFallbackSources = [.. specialFallbacks];
645+
}
650646

651-
// Produce a list of everything that matches the current filter.
652-
_filteredItems = InternalListHelpers.FilterListWithScores(newFilteredItems, searchQuery, _scoringFunction);
647+
if (token.IsCancellationRequested)
648+
{
649+
return;
650+
}
653651

654-
if (token.IsCancellationRequested)
655-
{
656-
return;
657-
}
652+
// ===== SCORING PHASE (OFF the lock) =====
653+
// The dominant apps pass is parallelized; commands and fallbacks stay serial. All of this
654+
// now runs without holding the TopLevelCommands lock, so it no longer blocks GetItems()/render.
655+
var searchQuery = _fuzzyMatcherProvider.Current.PrecomputeQuery(SearchText);
658656

659-
// Snapshot the global fallbacks and query, but do NOT score them here. Their dynamic
660-
// titles are updated asynchronously by the fallback update manager (BeginUpdate, above),
661-
// which runs off the typing path. Scoring is deferred to the render path so late fallback
662-
// resolutions fold in with fresh scores instead of a value frozen against a stale title.
663-
_globalFallbackSources = commands.Where(s => s.IsFallback && configuredGlobalFallbackIds.Contains(s.Id)).ToArray();
664-
_globalFallbackQuery = searchQuery;
657+
var scoredFilteredItems = InternalListHelpers.FilterListWithScores(itemsSource, searchQuery, _scoringFunction);
658+
659+
if (token.IsCancellationRequested)
660+
{
661+
return;
662+
}
663+
664+
var scoredFallbackItems = InternalListHelpers.FilterListWithScores(fallbackSource, searchQuery, _fallbackScoringFunction);
665+
666+
if (token.IsCancellationRequested)
667+
{
668+
return;
669+
}
670+
671+
RoScored<IListItem>[]? scoredApps = null;
672+
if (appsSource.Count > 0)
673+
{
674+
// Build the frecency lookup once, single-threaded, before the parallel pass reads it.
675+
// The lazy dictionary build in RecentCommandsManager is not thread-safe.
676+
_appStateService.State.RecentCommands.PrewarmIndex();
677+
678+
scoredApps = InternalListHelpers.FilterListWithScoresParallel(appsSource, searchQuery, _scoringFunction);
665679

666680
if (token.IsCancellationRequested)
667681
{
668682
return;
669683
}
684+
}
670685

671-
_fallbackItems = InternalListHelpers.FilterListWithScores<IListItem>(newFallbacks ?? [], searchQuery, _fallbackScoringFunction);
686+
#if CMDPAL_FF_MAINPAGE_TIME_RAISE_ITEMS
687+
var filterDoneTimestamp = stopwatch.ElapsedMilliseconds;
688+
#endif
672689

690+
// ===== PUBLISH PHASE (under lock): atomic swap, supersede-safe =====
691+
lock (commands)
692+
{
693+
// A newer keystroke cancels this token before doing its own work, so a stale snapshot's
694+
// finished results can never overwrite a newer query's. Skip the swap when superseded.
673695
if (token.IsCancellationRequested)
674696
{
675697
return;
676698
}
677699

678-
// Produce a list of filtered apps with the appropriate limit
679-
if (newApps.Any())
700+
if (tookFullCatalog)
680701
{
681-
_filteredApps = InternalListHelpers.FilterListWithScores(newApps, searchQuery, _scoringFunction);
682-
683-
if (token.IsCancellationRequested)
684-
{
685-
return;
686-
}
702+
_filteredItemsIncludesApps = includeAppsSnapshot;
687703
}
688704

689-
#if CMDPAL_FF_MAINPAGE_TIME_RAISE_ITEMS
690-
var filterDoneTimestamp = stopwatch.ElapsedMilliseconds;
691-
#endif
705+
_filteredItems = scoredFilteredItems;
706+
_fallbackItems = scoredFallbackItems;
707+
708+
// Snapshot the global fallbacks and query, but do NOT score them here. Their dynamic
709+
// titles are updated asynchronously by the fallback update manager (BeginUpdate, above),
710+
// which runs off the typing path. Scoring is deferred to the render path so late fallback
711+
// resolutions fold in with fresh scores instead of a value frozen against a stale title.
712+
_globalFallbackSources = globalFallbackSources;
713+
_globalFallbackQuery = searchQuery;
714+
715+
// Apps are only re-scored when there is an apps source (either the full catalog with
716+
// apps included, or a retained subset on the extend path). When there is none, publish
717+
// null so a rebuild clears any stale set - matching the previous ClearResults behavior.
718+
_filteredApps = appsSource.Count > 0 ? scoredApps : null;
692719

693720
// Queue a settled-search telemetry event. This measures the deterministic first-paint
694721
// results (commands + capped apps) and the latency to produce them, at this boundary -
@@ -727,6 +754,13 @@ private void UpdateSearchTextCore(string oldSearch, string newSearch, bool isUse
727754
}
728755
}
729756

757+
// Materializes a source sequence into a stable, indexable snapshot so the scoring passes can
758+
// run off the TopLevelCommands lock. Sequences that are already an IReadOnlyList (our own
759+
// freshly built lists and empty arrays) are used as-is; lazy LINQ over the live collection or
760+
// over previous results is copied so later mutation of the source can't affect scoring.
761+
private static IReadOnlyList<IListItem> MaterializeSource(IEnumerable<IListItem> items)
762+
=> items as IReadOnlyList<IListItem> ?? items.ToArray();
763+
730764
private bool ActuallyLoading()
731765
{
732766
var allApps = AllAppsCommandProvider.Page;

src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/RecentCommandsManager.cs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,15 @@ public RecentCommandsManager()
100100
{
101101
}
102102

103+
/// <summary>
104+
/// Forces the lazy command-id lookup (<see cref="Index"/>) to be built now, on the calling
105+
/// thread. The build itself is not thread-safe (it populates a shared dictionary field without
106+
/// locking), so callers that are about to score items in parallel MUST call this once,
107+
/// single-threaded, before the parallel loop. Once built, <see cref="GetCommandHistoryWeight(string)"/>
108+
/// only performs concurrent dictionary reads, which are safe.
109+
/// </summary>
110+
public void PrewarmIndex() => _ = Index;
111+
103112
public int GetCommandHistoryWeight(string commandId)
104113
=> GetCommandHistoryWeight(commandId, DateTimeOffset.UtcNow);
105114

@@ -177,4 +186,12 @@ public interface IRecentCommandsManager
177186
int GetCommandHistoryWeight(string commandId);
178187

179188
RecentCommandsManager WithHistoryItem(string commandId);
189+
190+
/// <summary>
191+
/// Builds any lazily-initialized internal state (e.g. the command-id lookup) on the calling
192+
/// thread so that subsequent <see cref="GetCommandHistoryWeight(string)"/> calls are safe to
193+
/// issue concurrently. Callers that score items in parallel must invoke this once, single-
194+
/// threaded, before the parallel loop.
195+
/// </summary>
196+
void PrewarmIndex();
180197
}

0 commit comments

Comments
 (0)