Skip to content
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

Cache global.json in NuGet SDK resolver #4380

Merged
merged 5 commits into from
Feb 2, 2022

Conversation

jeffkl
Copy link
Contributor

@jeffkl jeffkl commented Jan 3, 2022

Bug

Fixes: NuGet/Home#11450

Regression? No

Description

The MSBuild SDK resolver APIs provide some state but it is not thread-safe. This results in the NuGet SDK resolver loading and parsing global.json N times in Visual Studio, once for solution load and once for every design time build.

This change introduces a file cache based on path and last write time that is thread-safe.

I also changed how global.json is parsed to help speed up the process.

Related to NuGet/Home#11443 and NuGet/Home#11441

PR Checklist

  • PR has a meaningful title

  • PR has a linked issue.

  • Described changes

  • Tests

    • Automated tests added
    • OR
    • Test exception
    • OR
    • N/A
  • Documentation

    • Documentation PR or issue filled
    • OR
    • N/A

@jeffkl jeffkl requested a review from a team as a code owner January 3, 2022 20:56
@jeffkl
Copy link
Contributor Author

jeffkl commented Jan 3, 2022

Note to self, do not merge until dev targets 17.2

Copy link
Contributor

@erdembayar erdembayar left a comment

Choose a reason for hiding this comment

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

LGTM. Just 1 comment.

@jeffkl jeffkl added the Merge next release PRs that should not be merged until the dev branch targets the next release label Jan 6, 2022
@jeffkl jeffkl force-pushed the dev-jeffkl-global-json-reader-cache branch from f5af52f to 1a86290 Compare January 12, 2022 23:12
@ghost ghost added the Status:No recent activity PRs that have not had any recent activity and will be closed if the label is not removed label Jan 20, 2022
@ghost
Copy link

ghost commented Jan 20, 2022

This PR has been automatically marked as stale because it has no activity for 7 days. It will be closed if no further activity occurs within another 7 days of this comment. If it is closed, you may reopen it anytime when you're ready again, as long as you don't delete the branch.

@ghost ghost closed this Jan 27, 2022
@jeffkl jeffkl removed Merge next release PRs that should not be merged until the dev branch targets the next release Status:No recent activity PRs that have not had any recent activity and will be closed if the label is not removed labels Jan 31, 2022
@jeffkl jeffkl reopened this Jan 31, 2022
@jeffkl
Copy link
Contributor Author

jeffkl commented Jan 31, 2022

@zivkan do you want to review this again before I merge?

The MSBuild SDK resolver APIs provide some state but it is not thread-safe.  This results in the NuGet SDK resolver loading and parsing global.json N times in Visual Studio, once for solution load and once for every design time build.

This change introduces a file cache based on path and last write time that is thread-safe.

I also changed how global.json is parsed to help speed up the process.

Fixes NuGet/Home#11450
Related to NuGet/Home#11443
Add more logging and exception handling
Address feedback
@jeffkl jeffkl force-pushed the dev-jeffkl-global-json-reader-cache branch from 2992502 to 6ff3429 Compare February 2, 2022 18:21
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.

[DCR]: NuGet SDK resolver should cache global.json itself
5 participants