Reduce blocking time in UpdateSolution_QueryDelayBuildAction by Erarndt · Pull Request #7454 · NuGet/NuGet.Client

Erarndt · 2026-06-04T16:26:24Z

Bug

Fixes:

Description

Users are reporting that there is an occasional lengthy delay before build starts. Some early telemetry shows that it can take over 10s just to construct the IVsTask that gets returned. This isn't the time to finish the restore that the task initiates.

The code now unconditionally yields as a first step to return to the caller immediately. Additionally, switching to explicit GetExport() calls avoid some of the cost associated with SatisfyImportsOnce(). There might be a few innacuracies here, but it looks like there were some recent exports/services that were added that could explain why the MEF construction is showing up now:

CopilotToolInvocationService.cs
FixVulnerablitiesService.cs
VulnerablePackagesInfoBar.cs (IVulnerabilitiesNotificationService)
AuditCheckResultCachingService.cs
InfiniteScrollListViewModel
LegacyProjectAdapter
several PackageSourceMapping options pages

PR Checklist

Meaningful title, helpful description and a linked NuGet/Home issue
Added tests
Link to an issue or pull request to update docs if this PR changes settings, environment variables, new feature, etc.

Nigusu-Allehu · 2026-06-04T17:02:23Z

Let's update the PR title

Erarndt · 2026-06-04T19:50:45Z

Let's update the PR title

Updated :)

donnie-msft

Is there evidence that this is resolving the issue you described, or is it just something to try and observe its impact?

Had a few questions inline...

donnie-msft · 2026-06-04T21:41:46Z

+            // explicit switch to a background thread, awaits inside the lambda (e.g.
+            // GetComponentModelAsync once the service is cached) can complete synchronously and
+            // run MEF composition on the caller's thread before pDelayTask is assigned.
+            pDelayTask = NuGetUIThreadHelper.JoinableTaskFactory.RunAsyncAsVsTask(


Existing code was using the SolutionRestoreWorker.Value.JoinableTaskFactory.
What is the justification for this change?

I think in the new code SolutionRestoreWorker is manually imported in line 148 instead of using componentModel.DefaultCompositionService.SatisfyImportsOnce(this);

Yes, there's additional overhead with SatisfyImportsOnce() compared to manually importing such as needing to reflect over the type to find import attributes.

donnie-msft · 2026-06-04T21:42:58Z

+                        // Resolve imports directly from the export provider rather than calling
+                        // SatisfyImportsOnce(this), which reflects over this type, validates
+                        // composability of the import graph, and is significantly more expensive.
+                        // The [Import] attributes above are kept for documentation only.


Would these imports still apply if SatisfyImportsOnce was called again?

Could you clarify? Taking a guess at what you might mean, yes, they would still apply. If SatisfyImportsOnce() runs after the manual assignment, it can reassign the properties, but it would be with the same instance. The same thing is true for the reverse order of execution.

I'm not sure what "for documentation only" means in this case.

The intent is to signal that they're satisfied via MEF. I can change it to a comment or something. Whatever works best.

donnie-msft · 2026-06-04T21:54:18Z

it looks like there were some recent exports/services that were added that could explain why the MEF construction is showing up now:

I'm not understanding some of the listed items. Such as:

several PackageSourceMapping options pages

I assume this means VS Options pages, such as PackageSourceMapping. There are no MEF imports in this page, but it does rely on VSSettings which comes from a MEF import in NuGet's AsyncPackage in one initial place - NuGetPackage.cs, and it is shared among those settings pages.

Nigusu-Allehu · 2026-06-04T22:09:53Z

-                    componentModel.DefaultCompositionService.SatisfyImportsOnce(this);
+                    await TaskScheduler.Default.SwitchTo(alwaysYield: true);
+
+                    if (!_isMEFInitialized)


now that this is used async, is there a race condition concern here? TBF the wors that could happen is we write these values below multiple times.

The query and write to the property can happen multiple times in some narrow cases. It should be pretty unlikely in the current implementation though since it would require the user to trigger a build, cancel it, and then quickly try to build again. If there's a concern, I can update things to be more resilient.

Nigusu-Allehu

LGTM

Erarndt · 2026-06-05T17:15:21Z

Is there evidence that this is resolving the issue you described, or is it just something to try and observe its impact?

Had a few questions inline...

I do not have a way to reproduce the issue locally, so I can't directly validate the fix. I had to use telemetry to diagnose the issue in the wild.

There are multiple instrumentation points, and the issue I'm attempting to address in this PR is the synchronous blocking in UpdateSolution_QueryDelayBuildAction() before the IVsTask is returned. The duration of the IVsTask completion is captured in a separate telemetry event. The call to RestoreAsync() immediately yields, so it can't be the culprit. There's not much other code here, and the most likely issue is the code in the JTF.Run(). Moving the blocking code into the task is sufficient to avoid the synchronous block.

The change to the import flow isn't required to stop the synchronous blocking, but telemetry indicates that this code is rather slow (taking over 17s in a few instances). This was my attempt at speeding things up.

Erarndt · 2026-06-05T17:31:46Z

it looks like there were some recent exports/services that were added that could explain why the MEF construction is showing up now:

I'm not understanding some of the listed items. Such as:

several PackageSourceMapping options pages

I assume this means VS Options pages, such as PackageSourceMapping. There are no MEF imports in this page, but it does rely on VSSettings which comes from a MEF import in NuGet's AsyncPackage in one initial place - NuGetPackage.cs, and it is shared among those settings pages.

It's entirely possible that list is inaccurate. That was a quick attempt to see what might have changed to make this path slower recently. This seems like a relatively new issue, so I took a quick look. I can't reproduce this locally, so I can't profile.

nkolev92 · 2026-06-05T23:50:23Z

-            if (!_isMEFInitialized)
-            {
-                ThreadHelper.JoinableTaskFactory.Run(async () =>
+            // Force a yield before any work so this method returns immediately. Without the


I think this PR may have caused a regression.

https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=14283627&view=ms.vss-test-web.build-test-results-tab&runId=164932973&resultId=100014&paneView=debug

30+ tests timed out.

These tests aren't perfect, a bunch of have flakiness, https://github.com/NuGet/Client.Engineering/issues/3827, but shouldn't be 30.

Example: https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=14296970&view=ms.vss-test-web.build-test-results-tab

I re-ran the tests;

https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=14300232&view=results

Erarndt added 2 commits June 4, 2026 08:57

UpdateSolution_QueryDelayBuildAction now always immediately yields

861d14a

Resolve MEF imports directly.

65f8475

Erarndt requested a review from a team as a code owner June 4, 2026 16:26

Erarndt requested review from donnie-msft and jebriede June 4, 2026 16:26

Erarndt changed the title ~~Dev erarndt query delay build action~~ Reduce blocking time in UpdateSolution_QueryDelayBuildAction Jun 4, 2026

donnie-msft reviewed Jun 4, 2026

View reviewed changes

Nigusu-Allehu reviewed Jun 4, 2026

View reviewed changes

Nigusu-Allehu approved these changes Jun 4, 2026

View reviewed changes

nkolev92 reviewed Jun 5, 2026

View reviewed changes

Conversation

Erarndt commented Jun 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Bug

Description

PR Checklist

Uh oh!

Nigusu-Allehu commented Jun 4, 2026

Uh oh!

Erarndt commented Jun 4, 2026

Uh oh!

donnie-msft left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

donnie-msft commented Jun 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Nigusu-Allehu left a comment

Choose a reason for hiding this comment

Uh oh!

Erarndt commented Jun 5, 2026

Uh oh!

Erarndt commented Jun 5, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Erarndt commented Jun 4, 2026 •

edited

Loading

donnie-msft commented Jun 4, 2026 •

edited

Loading