Modify TagHelperCollector to use it's per assembly cache more often

[ToddGrun]

The caching done in TagHelperCollector.Collect(TagHelperDescriptorProviderContext, CancellationToken) was only being done on the reference assemblies. This PR also adds that caching for the target or compilation assembly.

Test insertion: https://dev.azure.com/devdiv/DevDiv/_git/VS/pullrequest/678059

Results from the speedometer tests in the above insertion looked good. The table below shows the average CPU/Allocs in the CA process for the CompletionInCohosting test under the TagHelperCollector.Collect method.

Version	CPU (ms)	CPU (%)	Allocs MB	Allocs (%)
Original	4600	7.7	291	12.3
New	2965	5.4	110	5.9

[DustinCampbell]

FWIW, I'm surprised that we're being called for the same Compilation.Assembly more than once. I would have expected the source generator to use its own cached results if the Compilation.Assembly didn't change.

I'd love to know what this looks like for a modern Blazor app with lots of components, such as MudBlazor. Do our Speedometer tests still just measure legacy Razor? If so, we're kind of blind to what most users are doing with modern .NET, which should be much more performance intensive.

Also, could you do toolset run for this? Whenever we tweak tag helper discovery, it's a good to run that to spot potential regressions.

[ToddGrun]

I'm surprised that we're being called for the same Compilation.Assembly more than once. I would have expected the source generator to use its own cached results if the Compilation.Assembly didn't change.

I (and the perf tests) definitely see this hitting many times for the same assembly, @chsienki for whether that's expected.

Do our Speedometer tests still just measure legacy Razor?

Yes, but I do have a PR that I've created to add MudBlazor tests.
PR: https://devdiv.visualstudio.com/DefaultCollection/DevDiv/_git/VS/pullrequest/677393
Bad news: Razor has been broken much recently, and until it's stable, I'm not going to be able to add new speedometer tests like this.
Good news: Chris just checked in a change that should stabilize this considerably: #12308. Once that is inserted to VS Main (which may be a challenge as it has quite a bit of razor payload), this should open up proceeding on adding the speedometer scenarios.

could you do toolset run for this?

Yes, but this would be a first for me, so I would appreciate if you could point me to some docs on how to do this.

---

In reply to: 3324563145

[DustinCampbell]

Yes, but I do have a PR that I've created to add MudBlazor tests.

🎉

At the very least, it might be useful to get some before and after numbers for MudBlazor locally. I'm expecting that the savings will be more dramatic, but I'm not sure.

[DustinCampbell]

could you do toolset run for this?

Yes, but this would be a first for me, so I would appreciate if you could point me to some docs on how to do this.

I can go search our Teams chat for the last time you asked me how to do that. 😄

Actually, I think I showed you in person instead of writing something up.

It's super easy. You can use the same branch that you pushed to run a test insertion.

1. Go to the razor-toolset-ci pipeline
2. Click "Run pipeline"
3. Click "Resources"
4. Click "dotnet-razor [RazorInternalRepo]"
5. Select the branch that you just ran your insertion PR on.
6. Click "Use Selected version"
7. Click "Close"
8. Click "Run"

[ToddGrun]

could you do toolset run for this?

Yes, but this would be a first for me, so I would appreciate if you could point me to some docs on how to do this.

I can go search our Teams chat for the last time you asked me how to do that. 😄

I did search before asking, but maybe my search term wasn't good. I'm sure I can figure it out, just thought it would be easier if something was already written up.

[ToddGrun]

could you do toolset run for this?

Looks like both the toolset run you did and the one I kicked off passed. Thanks!

[davidwengier]

Given Dustin's comment, and because I don't understand the mechanisms of assembly symbols and the caching of them, I'm wary of this without some specific tests. None of the speedometer or RPS tests change the tag helpers in a compilation, and it seems like that is the big risk here. Correct me if I'm wrong, but if we get a bad cache hit, that presumably manifests as "I add a new Razor file and it doesn't appear in completion/has squiggles until VS is restarted", which is pretty egregious and I'm not sure how many of even our own testing covers that (which is probably a big gap we should address), but certainly nothing in RPS, Speedometer or the toolset does.

Also we just had a report of something very similar to that hypothetical, in #12306, across a project reference boundary. Maybe source assembly symbols in a CWT aren't behaving like we think they should? I had a look through history a bit and it doesn't look like any of the PRs that touched it have added scenario specific tests. Maybe they already exist though? Do we have any tests for the CWT that manipulate a compilation over time?