-
Notifications
You must be signed in to change notification settings - Fork 258
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
NuGet Package Manager UI for Solution is slow because restore is run for every project instead of after all projects are updated #6010
Comments
The UI verifies that all projects are restored successfully for each project change, if it fails things are rolled back. This could probably be improved with some clever restore batching, optimistically restoring the entire solution and assuming it will just work, or by removing the rollback behavior for PackageReference. The workaround for this is to set package versions using a common .props file that all projects import. This allows you to update all packages by changing a single file. |
For the love of god, please fix this because NuGet is hilariously unusable at the moment!! NuGet will be turning 10 this year and it's still riddled with performance issues. Incidentally, 10 years is about the amount of time required for the Updates tab to load in VS. |
I cannot believe this isn't getting more attention. It is literally unusable in a larger solution. |
/cc: @rconard, @cristinamanum (to verify this issue in CPVM context) |
CPVM will suffer from the same problem @anangaur. |
Note for the implementer. |
I think there's also a correctness element here in addition to perf, see #9224 |
Hi. I start working on this problem. Here is my sample solution with 20 projects used for my test: Here is my restore log: |
@erdembayar So we have 300 projects. I think that might be where you are falling over. |
@erdembayar We only have 44 projects but to better reflect reality, you'll probably want to add more than jQuery and JSON .NET to each project. I'll attempt to upload a sample sln which vaguely matches our one sometime in the next few days. I've never properly investigated it myself so I'm not sure what the exact cause is but it definitely feels like there's an algorithm with unintended exponential complexity somewhere in the mix and it probably suffers due to some combination of number of projects and number of NuGet refs per project. If it's firing off n^2 network calls or something for routine operations, that would explain a lot. Anyway, thanks for giving this some attention :) |
I synced up with @erdembayar. The challenge is when we have multiple PackageReference projects referencing each other. Let's take for example a simple sln with 2 projects. If we try to install a new package to both projects at the same time, we'd run separate restores for each project and it's parents.
Now, we do sort projects in a dependency order, so it's not going to be undeterministic, but it arguably more frequently than we need. So the algorithm is going to be n * m at worst, where n is the number of projects, while m is the parent count of each project. We are not making extra network calls, but we are wasting some cycles. Code pointers:
The suggestion is to batch all the PackageReference changes into one, and only restore each project once. This will have different correctness characteristics from before, but I'd argue that it's better this way because of #9224. |
@Seltzer Just in case checking. Would you be able to create sample? I have 1 small sample which I am working on, but more samples are welcome to cover all possible cases. |
Hi @erdembayar, sorry for the delay. I've created a sample solution based on our codebase and added you and @nkolev92 as collaborators to its repo. I timed 3 mins for the Updates tab to load the first time, and 1 min for it to load after restarting VS and 1 min for it load a third time after closing and reopening 'Manage NuGet Packages for Solution' with no changes. For performing a single update, waiting a minute isn't a huge issue but if it becomes more painful when doing multiple updates. I haven't had the chance to benchmark the actual addition/update of dependencies as above but feel free to test it out with that sln. Also, please let us know if you think we're doing anything wrong. That code is kinda old and we had to do some slightly dubious things to support projects being used both in the context of their own self-contained solutions as well as in larger mono-repo solutions. Cheers, |
Hey @Seltzer, Thanks for the hard work creating that repro solution. |
Oops, I accidentally based the sample on the pre-PackageReference branch of our codebase. I've now migrated as many projects as possible to PackageReference, so that's basically all of them except for the ASP .NET projects which the VS migration tool can't handle. I've added:
Hopefully this will help. When I get a chance after work, I'll try to come up with some concrete examples of packages being slow to add/upgrade though it might be harder as we have fewer projects than others, esp. once you ignore the packages.config ones. |
@Seltzer I don't see any new repository. |
We sync-ed up offline, I provided the link. :) |
Details about Problem
Currently Nuget PackageReference type project are still very slow and customers reported same project restored several times over and over again, which not only slows also causes many repeated network calls.
So need of smarter/streamlined restore process which only restore
n
projects in O(n) runtime which means we restore each project only once.Runtime analysis: This is best we can achieve, we can't go better than this because each project from input need to be evaluated at least once. We're also doing all in memory evaluation in parallel for more performance gain.
This change decreased runtime by %50 for large test solutions like OrchardCore and another one which is contributed by user to us.
OrchardCore (158 projects) upgrade package is now 34 sec
Before it was between : 1min 4 sec to 7 min 12 sec. This big variation happen due to ordering which projects are evaluated because of same project restored several times, but now it's more consistent because restore happen just once per project.
So improvement for upgrade/downgrade is between about 2- 14 times faster, which is great and more sub-optimization are coming soon.
The PR#3559 to fix for this problem contains commits for fixing #9224 downgrade error problem too. Please check it this PR#3553 for description.
PM UI upgrade/downgrade actions:
Before improvement verbose downgrade log: 25 sec.
https://gist.github.com/erdembayar/06574b16e8395f2fcd308df480d5dc55
In above log you'll find
Restoring packages for C:\Users\eryondon\Downloads\nuget-sample-master\nuget-sample-master\A\Dtwt\Dtwt.csproj...
2 times, that meansDtwt.csproj
project was restored 2 times.Before improvement verbose upgrade log: 26 sec.
https://gist.github.com/erdembayar/b8b465842e164603a12fb20a9df6e3b7
In above log you'll find
Restoring packages for C:\Users\eryondon\Downloads\nuget-sample-master\nuget-sample-master\A\Dtwt\Dtwt.csproj...
2 times, that meansDtwt.csproj
project was restored 2 times.After improvement verbose downgrade log: 14 sec which down from 25 sec.
https://gist.github.com/erdembayar/286bc50683668656467c3f12ff3f5481
But now you'll see
Dtwt.csproj
project was restored 1 only time.After improvement verbose upgrade log: 12 sec which is 60% down from 26 sec.
https://gist.github.com/erdembayar/00f881be9f421479e1fdb59e323105cd
But now you'll see
Dtwt.csproj
project was restored 1 only time.PM UI update action:
Before improvement verbose upgrade log:
https://gist.github.com/erdembayar/31badeb81717955048c57ca42e08e7cb
ConsoleApp1.csproj... was restored 2 times.
After improvement verbose upgrade log:
https://gist.github.com/erdembayar/ee05724bd7a5281408014f641e93760e
ConsoleApp1.csproj... was restored only 1 time.
PM Consolidate:
After improvement verbose upgrade log:
https://gist.github.com/erdembayar/62f3b7c6210abd1f00eac9884609d9a8
ConsoleApp1.csproj... was restored only 1 time.
PM UI
Uninstall
is 2 times faster now forOrchardCore
solution.Before https://gist.github.com/erdembayar/2ba554f169e4f078baa61976affa62d4
2 min 1 seconds
.After https://gist.github.com/erdembayar/2f9aebfbdc78e31d229a03223c8b1888
58 seconds
.Note: Before
Preview build integrated action time
telemetry event was emitted for each individual project restore evaluation, but now it's emitted once for whole solution or several projects. So you might see less number of this events but might see some of them is much longer duration than previously. It might skew your averages if anyone use for any purpose I am not aware of it.Details about Problem
I upgraded to PackageReference because previous complaints about Nuget being SLOW was met with the response that I should upgrade to PackageReference because it is super fast
OS version (i.e. win10 v1607 (14393.321)):
win10
Detailed repro steps so we can see the same problem
Create a solution in VS2017 with many projects.
goto the nuget packages for solution UI
Add a nuget packge to all projects at once.
You will see that instead of package restore being run once at the end it is
run many many times.
See the attached log for my project
Verbose Logs
https://gist.github.com/bradphelan/daab6cf4bcf2037de065b6d3d2172577
The text was updated successfully, but these errors were encountered: