Add event tracing to DependencyGraphResolver #6284
Conversation
| // Keeps track of the number of times the entire walk is started over | ||
| int? restartCount = NuGetEventSource.IsEnabled ? -1 : null; | ||
|
|
||
| // Keeps track of the total number of items that have been queued for processing | ||
| int? totalQueuedItemCount = NuGetEventSource.IsEnabled ? 0 : null; |
There was a problem hiding this comment.
I don't understand the intent of making these counters nullable.
Two ints would normally increase the stack size by 8 bytes. Making them nullable makes that even larger, although I don't know the runtime implementation details well enough to know by exactly how much (but each nullable needs a bool, so maybe 2 more bytes).
Plus each time the counter is incremented, the compiler needs to emit "if not null" guards. Since CPUs have branch predictors, if the CPU guesses wrong, the pipeline needs to be flushed. All this to say that having nullable counters probably has worse theoretical perf than having a counter that's only written to, never read. Having said that I expect the impact to be so incredibly small that run to run noise/variation will completely drown out the difference. But, I don't feel like making the counters nullable helps.
Bug
Fixes: NuGet/Home#14134
Description
This adds event tracing to the two major parts of the new dependency resolver, resolving the graph and flattening it while reporting errors/downgrades/cycles.
It measures each project/framework/runtime identifier with how many items were resolved, the total number of items that were queued/processed, and the number of times it started over due to evictions.
For flattening, it tracks resolved/unresolved package count and success/failure.
PR Checklist