dependency resolution performance

[tomerd]

motivation: improve dependency resolution performance

changes:

• add informational diagnostics on manifest cache hit/miss
• disable expensive optimization in pubgrub resolver that has no functional value given changes from fix dependency resolution edge cases issues #3985
• adjust resolver workspace delegate to be less noisy given the difference in resolution callbacks

follow on from #4006

[tomerd]

@swift-ci please smoke test

[tomerd]

@swift-ci please test package compatibility

[tomerd]

some of the background conversation from #4006 (the wip version of this):

@tomerd
computation of the bounds here is very expensive and it needs to evaluate manifests throughout the dependency closure. afaict this is only used for the emittedIncompatibilities which is itself a performance optimization. disabling this gave 3x performance improvement on my initial tests on packages with medium to large dependencies graphs

@tomerd
cc @SDGGiesbrecht as this reflects our previous discussion about this optimization in #3985 which brought me to this finding

@SDGGiesbrecht
🤷 If it’s not actually faster, it’s not actually faster.

If you remove this, then emittedIncompatibilities is never written to and thus always empty, which in turn means...

• The line with if emittedIncompatibilities[node]?.contains(version) != true { is always true, and the contents of the if may as well be run unconditionally.
• The line with emittedIncompatibilities.isEmpty is also always true, although not necessarily the other conditions grouped with it.

While looking at it, I also noticed that this section and this section are now basically identical—comments and all. So you can probably reduce this to just the following and let it naturally fall through into the other, identical section.

if version == pinnedVersion {
  if self.emittedPinnedVersionIncompatibilities[node] ?? false { return [] }
  self.emittedPinnedVersionIncompatibilities[node] = true
} 

It might be worth checking whether those remaining lines are actually helpful for performance. If not, you could get rid of emittedPinnedVersionIncompatibilities too.

@SDGGiesbrecht
This particular optimization predates me. The commit that added it contains a bit of an explanation and a radar link (to which I lack access). It might be worth checking that radar for specific examples to double‐check for performance regressions. Also, if @aciidb0mb3r is still around out there somewhere, he might have something to add.

@abertelrud
Could it have an impact on the quality of the diagnostics when there are conflicts?

@tomerd
we have a fairly robust set of tests for dependency resolution diagnostics which have been impacted by #3985 but not by this. imo the diagnostics after #3985 are no less clear (and even more accurate) than the previous ones prior to #3985.

afaict the code in question was added in aee02eb as an attempt to improve diagnostics but because it is so expensive it is also cached. we already undo the part that impacted the diagnostics in #3985 (which was wrong afaict) by taking a bit of a different approach to recording incompatibilities and such this PR is purely about performance gain by not running that expensive calculation any more since it no longer has any functional value

[abertelrud]

Thanks for the background about diagnostics @tomerd. This looks great to me as far as I can tell.