Unroll LINQ usage to trim some allocations in AddMSBuildAssets

[Erarndt]

Bug

Fixes: NuGet/Home#13223

Regression? Last working version:

Description

Unrolls some LINQ usage to avoid allocation overhead.

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

[aortiz-msft]

@Erarndt - Could we please get some microbenchmarking to understand any perf gains with this commit? This is a requirement for all changes done to improve performance.

[aortiz-msft]

Needs perf benchmark as stated before.

[nkolev92]

Change looks good.

I think given that the linked issue is not directly related to the change here, I think it'd be save to just assume we need a new issue for these PRs.

[davkean]

@Erarndt Can you post the PerfView results blaming these allocations?

@aortiz-msft Unrolling LINQ is a well-known pattern that we've applied 1000s of times in this and other code bases. Micro-benchmarking a change like this adds a overhead to the contribution of these fixes and slows down the work. I think benchmarking should be saved for larger/riskier changes and not patterns we just know from experience are faster.

We've also contributed similar fixes in this codebase without micro-benchmarks.

[ghost]

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.

[ghost]

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.

[aortiz-msft]

Unrolling LINQ is a well-known pattern that we've applied 1000s of times in this and other code bases. Micro-benchmarking a change like this adds a overhead to the contribution of these fixes and slows down the work. I think benchmarking should be saved for larger/riskier changes and not patterns we just know from experience are faster.

Makes sense. Sorry, I didn't realize I had marked my comment as blocking.

[jebriede]

LGTM 🚀