Skip to content

improved restore speedup - #3512

Merged
forki merged 5 commits into
fsprojects:masterfrom
viktor-svub:feature/restore-speedup
Feb 22, 2019
Merged

forki merged 5 commits into
fsprojects:masterfrom
viktor-svub:feature/restore-speedup

Conversation

@viktor-svub

Copy link
Copy Markdown
Contributor

hi everyone and sorry for the long silence...

  • this change improves performance of paket restore, specifically where way too many inter-connected dependencies are involved
  • we have projects depending on the majority of the AspNetCore.App bundle, multi-targeting net462, netstandard2.0, and netcoreapp2.1
  • paket restore and/or dotnet restore take tens of seconds, easily over minute, per project/platform combination
  • with ANTS profiler, I identified the result-generating loop of LockFile's GetOrderedPackageHull, as it was re-creating a lot of Set instances
  • after rewriting the loop to re/use mutable state, the performance improved at least 20 times, taking approx 2 seconds for the whole solution

@isaacabraham

Copy link
Copy Markdown
Contributor

Mutability for the win!

@baronfel

Copy link
Copy Markdown
Contributor

Better yet, profiling + judicious use of localized mutability 👍 Nice work @viktor-svub!

@forki

forki commented Feb 22, 2019 via email

Copy link
Copy Markdown
Member

@viktor-svub

Copy link
Copy Markdown
Contributor Author

Well, I could change them as well, but their operations did not light up under the profiler and I generally avoid touching working code -- using F# Set there does not hurt, so from my PoV it's the preferred way.

I suspect the inherently expensive slow operation here is Set.map (which seems to always enforce re/creation of new set), and rest of the refactoring was only done to keep the code readable, as an attempt to replace just the map/filter and keep the rest semi-immutable did not end well.

@forki

forki commented Feb 22, 2019

Copy link
Copy Markdown
Member

ok thanks

@forki
forki merged commit 82c341a into fsprojects:master Feb 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants