Limit the number of analyzer plugins per context to one
Closed this issue · 18 comments
In order to prevent users from suffering the various woes of running too many analyzer plugins, we will limit the number of plugins-per-context to one.
In this design we limit the number of plugins per analysis context (as specified by the presence of analysis_options.yaml files) to one. We do this at the level of parsing the analysis options file.
Before an analysis_options.yaml file is processed for plugins and other options, the included options files are merged with it (recursively) (AnalysisOptionsProvider.getOptionsFromSource). Because of this, we can limit the number of enabled plugins directly in the code that parses the plugins YAML section (_OptionsProcessor.applyToAnalysisOptions), at which point plugins from included analysis options files have already been merged in.
The simplest change to make would be to continue to accept the singular format, the list format, and the map format, and add code to the list- and map-parsing code that limits the number of plugins to one.
... add code to the list- and map-parsing code that limits the number of plugins to one.
I would have guessed that there was only one format (presumably the map) after the included options file and the current options file had been merged.
I would have guessed that there was only one format (presumably the map) after the included options file and the current options file had been merged.
No, any of the three can be found. In particular, if there is no included options file, then the format parsed is the format given in the options file.
I hope this is a temporary workaround until the underlying issues (it would have been nice if you had linked them) are addressed, and then this can be reverted?
As a follow-up to this, probably to @srawlins , has this changeset achieved what it meant to do? Is there ongoing work to allow us to use more than 1 plugin in the future again?
In order to prevent users from suffering the various woes of running too many analyzer plugins, we will limit the number of plugins-per-context to one.
As a plugin maintainer who has spent the last couple months planning a business case for package:sidecar, such a huge restriction decision feels like a blow to my investment into proving use cases for a dart plugin ecosystem, and I would have expected at least some transparency on a timeline or how we can work together to address a workaround.
I fully understand that the current memory consumption of each plugin makes the experience less than ideal, and that plugin support has been experimental for a reason, but restricting the amount of plugins that can be used on a project to one will surely kill the recent investment into plugins made by sidecar, DCM, and custom_lint, to name a few. Users will stick with whichever currently adds the most value to their project.
The consequence is that we will stifle the innovation of these tools before we fully understand the use cases of plugins in general.
Surely there is another way to warn users of the consequences of plugins while allowing them to make the decision for themselves. @srawlins @bwilkerson
I hope this is a temporary workaround until the underlying issues (it would have been nice if you had linked them) are addressed, and then this can be reverted?
Yes that is the hope. :) Not a simple, direct revert, but a new design for plugins.
has this changeset achieved what it meant to do?
We're not sure, only time will tell. CC @devoncarew @jacob314 @mraleph in case they have seen any changes.
As a plugin maintainer [...] I would have expected at least some transparency on a timeline or how we can work together to address a workaround.
Apologies. We don't have a timeline for implementing a new plugins architecture.
Surely there is another way to warn users of the consequences of plugins while allowing them to make the decision for themselves.
I hope there are. We're open to ideas.
It would be awesome if this could be configured on user-side... It can be enabled to a single plugin by default, I don't mind.
But, leaving this decision to the user would be great, because sometimes, the tradeoff of performance/memory is worth it (my case).
We have a use case where we want to use a static code analysis plugin that encourages best practices for dart code implementation, while we also use custom linting using custom_lints to encourage the use of custom classes we created within the app (I.E., a custom Widget state class). Would love to know that support for a use case like this would be reintroduced, because as is we would only be able to support either the custom linting or the general code quality linting.
We are definitely working to see whether we can design and implement plugin support that would allow for the use of multiple plugins. I don't have any timeframe that I can give for when we'll have a decision about whether such support will be added, nor, of course, any guess as to how long it will take to implement.
@bwilkerson would you consider, in the meantime, creating a flag to allow consumers to decide on using multiple plugins or not?
It can have warnings and what not, to tell the users about enabling it... kind of "do it at your own risk".
Our concern with doing so, and the reason we decided not to provide a flag to allow multiple plugins when we added the one-plugin limitation, is the performance problem I mentioned. We believe that the problem is severe enough that allowing multiple plugins (under the current implementation) makes the tooling unusable.
That said, we value user feedback and are always willing to reconsider our decisions. That doesn't mean that the decision will be changed, just that we'll think about it again.
Yes, I can understand and definitely respect your decision!
The only thing I am not fully in agreement is this quote: 'the problem is severe enough that allowing multiple plugins makes the tooling unusable'.
My reasoning is that, since the plugins are really configurable and can be even created from userland, there's no way that is 100% guaranteed for you to know if my specific use case is suffering from this problem. So, for my specific project, I suffered the opposite case, the tool itself became 'unusable'.
Again, just to emphasise that I do respect your reasoning and whatever is decided, well, I will adapt to it (as I try to do with so many other stuff on a daily basis... everyone does that I guess... hehe 😅 ).
My reasoning is that, since the plugins are really configurable and can be even created from userland, there's no way that is 100% guaranteed for you to know if my specific use case is suffering from this problem. So, for my specific project, I suffered the opposite case, the tool itself became
unusable.
Same here - I've personally never experienced problems regarding too much RAM consumption on my particular projects. Still unsure what causes some users to experience the "problem" mentioned (many barrel files, nested workspace packages, etc.).
@incendial - I know you were actively monitoring high RAM usage issues for a few DCM plugin users, can you speak to how severe the problem is from your perspective? (total users experiencing a problem vs. population of DCM users)
Most of the users that have a large project ended up removing DCM because of the analyzer + plugin performance issues. But the overall number of these users compared to all users is relatively small. I believe the number of packages within the workspaces contributes the most since the plugin is created for each context.
RAM usage has never been a problem for our team as we have been using the DCM plugin for our project for a good while. Though even if RAM usage is a concern, why couldn't we inflict a bit more ram usage on ourselves if we really want to implement additional plugins to make the development process easier? Is it because of some diagnostic analytics for usage data, and removing the ability to add in more than one plugin makes diagnostics data more consistent?
where is the issue tracking allowing multiple plugins?
To the best of my knowledge there isn't any issue tracking this work. The Dart team tends to create issues to track the implementation of a language feature (but some smaller features don't have any tracking issues), but other implementation work typically doesn't have a tracking issue associated with it (again, there are exceptions).