Switch CreateNodeAsync to an iterative approach

[Erarndt]

Bug

Fixes: NuGet/Home#13222

Regression? Last working version:

Description

This change updates the implementation of CreateNodeAsync to avoid async state machine allocations (<CreateGraphNodeAsync>d__3) caused by recursion. In larger restores, these were a significant amount of allocations (:

After:

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]

Manually run CI was passed: https://dev.azure.com/devdiv/DevDiv/_build/results?buildId=9041709&view=results

[nkolev92]

@jeffkl You can run the CI on refs/pull/5624/head to get the checks published.

[jeffkl]

@jeffkl You can run the CI on refs/pull/5624/head to get the checks published.

I can it against refs/pull/5624/merge here: https://dev.azure.com/devdiv/DevDiv/_build/results?buildId=9041709&view=results Does it have to be refs/pull/5624/head instead? That would be funny

[nkolev92]

The checks are based on the commit sha on top of the branch, so you have to use head

[jeffkl]

The checks are based on the commit sha on top of the branch, so you have to use head

Thanks for the tip! I've started CI again but this time the checks are coming through 🎉

[nkolev92]

Beyond allocations, are there wall clock improvements?

I know @jeffkl's running substrate, but we probably want more data here.

[nkolev92]

Aren't we losing some of the parallelism we had here previously?
Now the graph is walked completely synchronously.

[dotnet-policy-service]

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 90 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.