Skip to content

SDK should disable default item globbing when asking MSBuild to perform implicit Restores #49415

@baronfel

Description

@baronfel
Member

Is your feature request related to a problem? Please describe.

@DamianEdwards pointed me at this review of the macbook w/ M4 Max processor over the weekend (timestamp is directly to the comparison I'll be talking about).

The repo in question is https://github.com/alexziskind1/machine_tests.

The gist is that a very large (over 100k) number of C# source files are generated, and then dotnet restore followed by dotnet build are called, and this is treated as a performance test.

I took binlogs on my Windows and WSL Ubuntu instances, and Damian took binlogs on his Ubuntu machine and his Mac, all with eval profiling enabled. This is accomplished by adding -bl to enable binlog generation, and --profileevaluation to get nice graphs of where eval time is spent. As seasoned MSBuild experts might expect, the problem is file globbing here - the vast majority of the time in the build is spent in (on my Windows machine):

  • SDK Default Compile Item Include globbing
  • Csc - this is out of our control, compiling 100k files just takes time

However, in the test thread @alexziskind1 is doing, he's evaluating 5 times total (times on my Windows box):

  • dotnet restore
    • once before the Restore target is executed, with MSBuildRestoreSessionId and MSBuildIsRestoring properties set (45s)
    • once as a side effect of the _FilterRestoreGraphProjectInputItems task when it calls the MSBuild Task on the same projects' _IsProjectRestoreSupported Target, but with an additional Global Property ExcludeRestorePackageImports set (45s)
  • dotnet build (with implicit restore because --no-restore is not passed in)
    • the same two from above, and
    • the 'normal' evaluation of the project without any of the Restore-related properties set (45s)

On my machine, this means that we spend ~2.25 minutes just on evaluations, almost all of which is globbing, and ~2m on actual compilation (Csc Task wall-clock time).

Describe the solution you'd like

Restore doesn't generally need to do Default Item expansion (this is pretty common on other commands that do MSBuild evaluation like dotnet run, so the dotnet cli should add the --restoreproperty:EnableDefaultItems=false argument to the command line that it constructs for any implicit restore it asks MSBuild to perform.

If we wanted to be very defensive here, we could also

  • enlighten the dotnet CLI about the MSBuild --restoreproperty CLI argument like we have for the MSBuild --property CLI argument
  • parse any user-supplied input for this argument, and if the user has specified a EnableDefaultItems value do not add our own

Additional context

If we do this, then on my machine at least every Restore-related evaluation goes from ~45s to ~300ms. That's a huge savings, amortized.

Activity

added
cli-uxIssues and PRs that deal with the UX of the CLI (exit codes, log output, verbs/options, and so on)
on Jun 16, 2025
rainersigwald

rainersigwald commented on Jun 16, 2025

@rainersigwald
Member

@jeffkl have you observed any problems from NuGet's static-graph glob avoidance that might inform this?

jeffkl

jeffkl commented on Jun 16, 2025

@jeffkl
Contributor

I remember a handful of complaints over the years about globbing not expanding on anything except *.*proj. I don't think I've ever seen a case where a user expects globbing to work on a <PackageReference /> since those are statically declared and not backed by anything on disk. From what I can remember, the only time globbing didn't work as expected for people was on a <ProjectReference /> in a traversal project because their projects didn't end in proj for whatever reason.

If we could make globbing only work for certain items that could be a potential way to disable other globs during evaluation. Lazy evaluation of items also but I think the regular restore paths it might not help since we're not using APIs.

I would not recommend setting global properties like EnableDefaultItems=false since it wouldn't help with users' random items. To really speed up evaluation for restore, disabling globbing is the way to go.

baronfel

baronfel commented on Jun 16, 2025

@baronfel
MemberAuthor

If we don't want to toggle everything, we can also disable the items responsible for most of the pain:

  • EnableDefaultCompileItems
  • EnableDefaultEmbeddedResourceItems
  • EnableDefaultNoneItems

Image

None of these should impact Restore.

ViktorHofer

ViktorHofer commented on Jun 17, 2025

@ViktorHofer
Member

I would not recommend setting global properties like EnableDefaultItems=false since it wouldn't help with users' random items. To really speed up evaluation for restore, disabling globbing is the way to go.

+100

baronfel

baronfel commented on Jun 17, 2025

@baronfel
MemberAuthor

@rainersigwald I'm happy to disable globbing entirely for restore, but it appears the only mechanism for that is an env var. We need a way to toggle that behavior for restore only from the SDK. Got any thoughts?

rainersigwald

rainersigwald commented on Jun 17, 2025

@rainersigwald
Member

We could expose that in the API, I think--IIRC there may be some caches we'll have to be careful to additionally key on "globs disallowed".

But I fairly strongly suspect that we could get very good results with just the few item types you named above; I'd want to see evidence otherwise. @ViktorHofer what makes you think custom-item globs are causing significant pain?

ViktorHofer

ViktorHofer commented on Jun 18, 2025

@ViktorHofer
Member

My reaction here was to build on top of what we already have (nuget static graph restore's opt-out) and avoid a potential diff between static graph and "normal" restore. I think this comment capture the concerns nicely: https://github.com/NuGet/NuGet.Client/blob/772ee13d2bafaa1414d90dbfbd77e0941115ef19/src/NuGet.Core/NuGet.Build.Tasks.Console/MSBuildFeatureFlags.cs#L65

baronfel

baronfel commented on Jun 23, 2025

@baronfel
MemberAuthor

I implemented this in #49526 and can confirm that this does seem to impact SDK scenarios around Restore - particular in RID-specific restores where the RID isn't in the Project but is passed from the command line. Checking out what's up with that.

EDIT: this is just user error the -rp flag explicitly states that it is exclusive with -p, so the SDK should send all SDK-defined -p as -rp if any -rp are sent at all. Working on that now as well.

If that doesn't prove fruitful, then there is at least one other pathway to get this benefit I can see:

  • The SDK must sometimes perform a 'separate Restore' - e.g. not adding -restore to an otherwise-reasonable build command line. This is a case where we could set the environment variable flag that @jeffkl and @ViktorHofer mention.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

Area-CLIcli-uxIssues and PRs that deal with the UX of the CLI (exit codes, log output, verbs/options, and so on)performance

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

    Development

    Participants

    @baronfel@rainersigwald@ViktorHofer@jeffkl

    Issue actions

      SDK should disable default item globbing when asking MSBuild to perform implicit Restores · Issue #49415 · dotnet/sdk