-
Notifications
You must be signed in to change notification settings - Fork 417
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enhancements for roslyn analyzers after initial version. #1301
Comments
I renamed the title 😃 |
I have a suggestion. I am not sure if this is already happening. I think analysis should prioritize any 'already opened' documents and 'user opened' documents, over any 'unopened' documents. My understanding is that it would increase the perceived speed of analysis and allow the user to begin working on their projects more quickly. @savpek What do you think? Not sure how feasible this is, but possibly have a toggle that analyzes only the current project the open document is in. So for solutions with many projects, opening a document in that solution would perform analysis of only that documents entire project. A last suggestion is to maybe have an excluded projects list from analysis. |
I like that idea 👍 However omnisharp doesn't support open/close states of documents 🙁 They are implemented in pr #884 but it has been hold for quite long while. Personally i think that non full solution analysis (VS has full solution analysis option) should be supported when working with large workspaces, that could be current document or current project (that current project actually could be nice, however project can be very large in some cases). Current priority is simply based for 'lastModified' of document (not file, workspace document). Which is bit dummy. Prioritizing open documents (or projects with open documents) is much better solution. One thing needed to be implemented to analyzers is foreground/background analysis, where foreground is document like current one and background is rest. That way current document feedback can be instant even when theres lots of analysis going on (like during full solution analysis at startup). That opens doors for further optimizations. |
Another suggestion would be to make the analysis timeout configurable Line 209 in 62b3b52
I indeed often experience analysis timeouts on some files, with a bunch of analyzers
|
@fmauNeko is there specific files that triggers that timeout? 10 seconds for single file analysis is very long time. I have noticed that some kind of partial classes can cause built in analyzers go crazy and basically hang few cpu cores until they are cancelled. If its specific file that always causes issue can you paste it here? I skipped partial issue because at that point roslyn 3.0 was on beta/preview phase, have to check does it still occur. |
@savpek Sorry for the delay namespace Selene.Common
{
using System.Collections.Generic;
public class RabbitMQQueueDefinition
{
public string Name { get; set; }
public bool Durable { get; set; } = false;
public bool Exclusive { get; set; } = false;
public bool AutoDelete { get; set; } = false;
public IDictionary<string, object> Arguments { get; } = null;
}
} Here's the trace, using
|
Note: Based on dotnet/roslyn#35621 full roslyn e2e support for .editorconfig is nearly ready. If so it will close bullet about editorconfig (originanted from review feedback https://github.com/OmniSharp/omnisharp-roslyn/pull/1076/files#r215436172). Test and add atleast some basic level test(s) that editorconfig is loaded at omnisharp-roslyn side once available. |
I've got the same timeout issue that @fmauNeko reported using FxCopAnalyzers & StyleCop.Analyzers in my projects. Seems to happen on basically all files. |
@felschr thanks for reporting, i will try repro. So basically dotnet new + StyleCop.Analyzers/FxCopAnalyzers should to the trick? |
I haven't tried it but the project that I have is essentially a small |
@felschr I can repro issue, specifically it is This isn't specifically timeout issue, i tried with version that has like 3 min timeout, basically analysis never exits and takes several cores with 100% from pc. At quick profiling it seems like time is spent inside of analysis drivers somehere in roslyn: Analyzers doesn't use same api as visual studio (visual studio use internal apis i think), this may be bug in analyzers that is triggered how roslyn analysis api is used in omnisharp. I think this requires simplified repro repo without omnisharp (like console app that calls analyzer api same way as omnisharp does) and proper issue on roslyn code base side to understand what is going on. I will do it once #1507 vscode side is ready. Next on list so soon ™️ This can be triggered with built in analyzers too with very special kind of partial classes, for that reason i think roslyn is correct place to query with this instead of fxcop analyzers. Tested it and it still occurs. |
I would love for this timeout issue to be resolved. FxCop and StyleCop are our Analysis tools of choice, but right now they are unusable with VSCode. |
Certain parts of review updates and requests are moved to do after initial release (parts i think are good to do but are not showstoppers).
Pull requests which these are based on
#1076 and especially review https://github.com/OmniSharp/omnisharp-roslyn/pull/1076/files/3aaf7707f48fc36869c5cf288ab4dc424a9e83b4
The text was updated successfully, but these errors were encountered: