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

Fix hang in Solution Explorer search through transitive package items of dependencies tree #5917

Merged
merged 2 commits into from
Jul 13, 2024

Conversation

drewnoakes
Copy link
Contributor

@drewnoakes drewnoakes commented Jul 12, 2024

Bug

Fixes: NuGet/Home#13619
Fixes: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/2079818

Description

#5508 reduced the number of dataflow updates through code that led to redundant tree updates. It did that by inhibiting updates in cases where nothing changed.

AssetsFileDependenciesTreeSearchProvider uses GetLatestVersionAsync to obtain the latest snapshot value, relative to the current data versions of the project. However suppression of updates meant that the latest version might not be present, and the call would hang indefinitely. This would break Solution Explorer search for customers who enabled "Search External Items", have SDK-style .NET projects in the solution, and make certain project changes that lead to so-called "version-only" updates.

This PR reverts #5508 and adds a downstream check for unchanged data, skipping tree updates in such cases. This means that the latest data is always available for the search component to consume, meaning there's no hang.

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.

#5508)"

This reverts commit d85c6ea.

Turns out that the search provider uses `GetLatestVersionAsync` on this
data source, meaning it must propagate a value for every input version.
I will fix the original issue in a different way, by updating the
consumer to ignore version-only changes instead.
This replaces the approach taken in #5508 in a way that doesn't interfere with Solution Explorer search.
@drewnoakes drewnoakes requested a review from a team as a code owner July 12, 2024 11:33
@kartheekp-ms
Copy link
Contributor

kartheekp-ms commented Jul 12, 2024

This PR reverts #5508 and adds a downstream check for unchanged data, skipping tree updates in such cases.

I interpret this comment as the perf issue NuGet/Home#13019 will not regress even though we reverted #5508, correct?

@drewnoakes
Copy link
Contributor Author

That's right. With this change the update propagates one level further down the dataflow graph, but the code that updates the tree still avoids doing the redundant work. The fix here is that another branch of the graph (search) needs to see every version of the update, even when the snapshot is unchanged.

@drewnoakes drewnoakes merged commit bca3348 into dev Jul 13, 2024
27 of 29 checks passed
@drewnoakes drewnoakes deleted the dev-drnoakes-fix-solution-explorer-search branch July 13, 2024 00:04
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.

Solution Explorer search can be broken by skipped dataflow updates
4 participants