Roslyn: Analyzer performance tracking is only enabled in remote host

[Therzok]

Version Used:
2.9.0-beta4-63001-02

Steps to Reproduce:

1. Run a host workspace without a remote host service
2. Observe that analyzer performance telemetry is not sent

http://sourceroslyn.io/#Microsoft.CodeAnalysis.Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.InProcOrRemoteHostAnalyzerRunner.cs,bc3f6b509cfc8156

Expected Behavior:
Analyzer telemetry (perf stats) should be sent for in-process solution crawler too

Actual Behavior:
It is not.

We'd like to track analyzer performance for VSMac, so this would be useful to report in non-remote situations

[Therzok]

cc @jasonmalinowski @jinujoseph @heejaechang

[Therzok]

cc @mhutch @slluis @jmatthiesen

[heejaechang]

that will require little bit of refactoring but possible if we want to.

[KirillOsenkov]

@jinujoseph any chance we could get this slightly sooner than 16.0? It's OK if this doesn't get checked into any mainstream branch, as long as we can take the NuGets we should be able to consume it.

[cobey]

@jinujoseph is there any update on movement here?

[jinujoseph]

@CyrusNajmabadi can you sync with @KirillOsenkov and @cobey to understand the possibility/priority on this

[tmat]

I don't think we want to run analyzers in-proc going forward, so the priority would be low imo.

[Therzok]

@tmat VSMac does not run them analyzers in OOP

[tmat]

@Therzok Is there a reason why not? The reasons why we are moving features to OOP seems that these would apply to both VSMac and VS. E.g. reliability - if a 3rd party analyzer crashes you don't want to take down the IDE.

[KirillOsenkov]

I'll confess that one trend that kinda worries me is assuming that VS and VSMac are the only two blessed Roslyn hosts that the Roslyn project wants to support and cater towards. Since the spirit of Roslyn is historically oriented towards the open and welcoming 3rd parties I'd like to think that we don't want to alienate other Roslyn hosts or make assumptions about whether they need to buy in to the OOP model.

While I understand the need for OOP I think the danger as I see it is breaking or abandoning in-proc scenarios. For the last 10 years we have learned to operate in the in-proc world with I dare say astounding success, so I don't see how the stability arguments are suddenly forcing us to throw the baby out with the bath water.

All that said, I fully understand the team's priorities so not willing to die on the hill for this particular issue. Would be nice to get to it if possible, but we understand if it can't be a priority.

Plus, there are considerations that are just different for VSMac. For example, we don't care about OOM since we already are a 64-bit process always. An analyzer crashing in a different process has about the same user impact that an analyzer crashing in process for us. We don't die, we might or might not show a gold bar and we might or might not end up with some corrupted state.

Additionally, I suspect there's quite a lot of work in converting VSMac to support out-of-proc logic, and we simply have bigger fish to fry right now. On the long list of VSMac problems I'd say moving to running the analyzers out-of-proc is not even in the top 20 right now.

[tmat]

Would be nice to get to it if possible, but we understand if it can't be a priority.

To be clear, I didn't say we shouldn't do this. Just that it's low priority due to our current focus.

An analyzer crashing in a different process has about the same user impact that an analyzer crashing in process for us.

I don't think this is true. While you can recover from most exception, you can't easily recover from stack overflow, for example. Since syntax trees are deeply nested data structures, stack overflow is actually not that hard to cause in code that traverses them.

[CyrusNajmabadi]

FWIW, i think the "right" approach that is healthy for the long term future is that roslyn is purely a server and works wit all systems through LSP. That way we have one common way to be used/hosted/talked-to, and it's all through a standard API that everyone can settle on.

We def don't want to break existing important cases, but we should be strongly thinking about the healthiest future we can get to that supports a wide set of cases with effective engineering costs :)

[KirillOsenkov]

I'm all for it. It's a great thing to have.

Please just don't break existing scenarios and APIs too much.

[jmarolf]

Please just don't break existing scenarios and APIs too much.

Just use SemVer :)