Skip to content

Resolve WinGet search results in parallel and once per package - #5687

Open
Gabriel Dufresne (GabrielDuf) wants to merge 2 commits into
mainfrom
worktree-winget-search-speed-5040
Open

Gabriel Dufresne (GabrielDuf) wants to merge 2 commits into
mainfrom
worktree-winget-search-speed-5040

Conversation

@GabrielDuf

Copy link
Copy Markdown
Contributor

Fixes #5040.

Why WinGet search was slow

Through the COM API, the search read CatalogPackage.DefaultInstallVersion for every match, one at a time. On the winget source's V2 index that property returns the newest version that isn't pinned and has an installer this machine can use. Deciding that means downloading the package's versionData.mszyml and its manifest from the CDN. A search for "firefox" made 630 sequential requests for 315 packages, while winget search reads the version from its local index and downloads nothing. Each package was also processed up to 3 times, because the Name, Id and Moniker queries run separately and overlap (941 matches for 316 packages).

Behind a slow proxy this goes past the 60 s listing timeout. The search is then retried once and shows no results, which is what the reporters described.

Change

  • Parallel, deduplicated reads: matches are read in one parallel pass, 8 at a time. The first thread to see a (catalog, id) pair resolves its name and version, so duplicates are never resolved twice. Results keep the order of their first occurrence (SelectUniqueSearchMatches, unit-tested).
  • DefaultInstallVersion is kept on purpose. AvailableVersions[0] would skip the manifest download, but it ignores winget pin and installer applicability, so the displayed version would change.
  • The three queries stay separate. Merging Name/Id/Moniker into a single request with selectors was tried and dropped: the msstore REST source doesn't union them, and 9 Microsoft Store results disappeared.
  • Listings can now be cancelled. Each listing gets a cancellation token next to its process list. It's cancelled after the processes are killed, both on timeout and by StopRunningListings, so an abandoned search stops instead of running alongside its retry.
  • Errors are summarised: read failures are logged once (count plus the first error). A search where nothing could be read still returns an empty list rather than throwing. Throwing would send it into AttemptFastRepair, which reconnects the COM server and clears NativePackageHandler over a single transient failure.

Verification

  • A/B against the previous build: winget's download cache was cleared before each run, then 10 queries were searched (thunderbird, firefox, vlc, discord, spotify, python, visual studio, 7zip, chrome, netflix). Results were identical: 590 lines with the same packages, versions, catalogs and order, all 14 msstore entries included.

    First search Before After
    firefox 7–30 s 1.4–1.9 s
    thunderbird 3.6–27 s 1.0–1.7 s

    The download count is unchanged; the downloads now overlap.

  • New tests: cancellation on stop, on timeout (made overridable for the test; confirmed to fail when the cancel is removed), one token per listing, no token outside a listing, and dedupe ordering, unreadable matches and id casing.

  • Full UniGetUI.PackageEngine.Tests: passes (2136 + 3157; 9 opt-in E2E tests skipped). Both dotnet format gates exit 0.

Not covered

  • Winget versions older than 1.29.380.
  • Lower-trust COM activation.
  • The reporters' proxy environment.

The 60 s timeout should now be far harder to hit, but a very broad query behind a high-latency proxy still needs one version-data and one manifest download per result. Removing that cost entirely would mean showing the index's latest version in search results instead of winget's applicable one, which is a separate decision.

🤖 Generated with Claude Code

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

A stop/search cancellation race can trigger unintended WinGet repair and cache clearing, while concurrent metadata resolution lacks deterministic tests for duplicate handling, output ordering, cache consistency, and cancellation.

2 open issues
What changed in this PR

This PR addresses #5040 by parallelizing WinGet search metadata reads and resolving each package once, while preserving applicable-version selection and result ordering.

Changes:

  • Resolves metadata with up to eight workers, deduplicates overlapping matches, and summarizes read failures.
  • Adds per-listing cancellation on timeout and explicit stop.
  • Adds tests for cancellation, token isolation, and deduplication ordering.
File Description
src/​UniGetUI.PackageEngine.Tests/​WinGetManagerTests.cs Tests deduplication, ordering, unreadable IDs, and casing.
src/​UniGetUI.PackageEngine.Tests/​PackageManagerTests.cs Tests listing cancellation and token isolation.
src/​UniGetUI.PackageEngine.Tests/​Infrastructure/​Fakes/​TestPackageManager.cs Exposes cancellation and configurable timeouts for tests.
src/​UniGetUI.PackageEngine.PackageManagerClasses/​Manager/​PackageManager.cs Manages cancellation tokens alongside listing processes.
src/​UniGetUI.PackageEngine.Managers.WinGet/​WinGet.cs Exposes listing cancellation to the native helper.
src/​UniGetUI.PackageEngine.Managers.WinGet/​ClientHelpers/​NativeWinGetHelper.cs Resolves unique search results concurrently with cancellation and summarized errors.

Review effort: Balanced

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Parallel WinGet COM property reads still need Windows compatibility validation on supported older WinGet versions and lower-trust activation paths, which the reported A/B run and fake-reader tests do not verify.

No open issues 🎉

2 resolved since last review

Review effort: Balanced

@randy-but-a-ro randy-but-a-ro Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Pull request was approved automatically: the AI review is complete and all its review threads are resolved. 🎉

Integration Details
{
	"deliveryId": "2cb9aca0-d946-11f1-81b8-0a680e0f20d6",
	"headSha": "3a1db9b1755a1e4bd6cc758694f3ad91c227b292",
	"reviewer": "copilot-pull-request-reviewer[bot]"
}

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working winget

Development

Successfully merging this pull request may close these issues.

[BUG] winget search sometimes takes an excessively long time.

2 participants