LSP and Analyzer Plugin - Multi-Root Workspace Versioning

[Patt O'Brien]

Hi Analyzer Team,

I'm developing an analyzer plugin, and had a question about how versioning is handled via the Dart Analysis Server, Dart Code extension that spins up the server for a workspace, etc.

For context, I've read through many materials, including the following:

- package_config.json v2 language spec

- dart versioning language spec

- analyzer package docs

As I've mentioned in previous discussions, I've been working on a plugin that would allow lints/edits defined in external packages to run off of a single set of resources (as opposed to duplicating resources for each plugin). I do have a working PoC, but something I overlooked was how versioning would be handled for projects with conflicting plugin dependencies, and I wanted to get your 2 cents before proceeding.

I'm not 100% sure about this, but from what I can tell analyzer plugins run with a plugin-workspace relationship of one-to-one , where any workspace can have 1+ AnalysisContexts. I'm not exactly sure how the analysis server determines which version of "my_plugin" is used, but from brief testing I believe the server first looks at each context's analysis_options.yaml for the list of activated plugins, and then attempts to find a matching package name within the pubspec, then copies the plugin's bootstrapper from root>tools>analyzer_plugin to .dartServer/.plugin_manager/, etc. etc.

The problems that I foresee with this architecture is in the one-to-one server-to-workspace relationship and the possibility that AnalysisContexts have different package plugin configurations - and in my case, if these packages have different 3rd party lint dependencies. For example, one context may depend on my_plugin v1.0.0, while another depends on my_plugin v2.0.0.

With most other dart tools, take build_runner for example, the analyzer is versioned and run within the context of one single root package. With the analysis server + LSP, the server version seems to run from the SDK version defined via the Dart Code extension, and therefore seems to attempt to run just 1 plugin instance for the whole workspace / AnalysisContextCollection.

If this is the case, some brainstormed solutions to allow different packages to define their own plugin dependencies, while minimizing the amount of duplicated resources, include the following:

Option #1:

Combine lint dependencies from all AnalysisContexts into one package_config.json (e.g. the one defined in ~/.dartServer/.plugin-manager), run the plugin, determine which lints/edits to use for each context during runtime via pubspec/analysis options. This would be very easy to develop. Risks: 

- different plugin / lint versions cannot co-exist

- inevitable dependency conflicts would not be associated with a single root package, and reporting/debugging conflicts would require users to learn new workflows specific to analyzer_plugin

Option #2:

Server runs middleman-plugin (1:1 relationship with analysis server), which bootstraps and runs 1+ instances of my_plugin with a 1:1 relationship between my_plugin and AnalysisContexts that have the plugin declared, where the my_plugin isolate would be initialized with its own package_config from the analysis context root. The plugin would analyze the context root and all of its dependencies. Risks: 

- In the case of workspace-package-A depending on workspace-package-B, resource overlap would exist, meaning duplicated resources

- Higher complexity from adding another plugin layer

Option #3

Middleman-plugin runs my_plugin with a 1:1 relationship between my_plugin:package, where the package is any root package or dependency in all package_configs within the workspace. We would have theoretically 0 resource overlap while allowing any plugin version or dependency to run alongside one another (within reason, of course). Risks:

- Cross-isolate communication is essential and would require designing a new and fairly advanced communication protocol and communication architecture

- Much higher complexity from communication protocol and additional plugin layer

Option #4, 5, 6

??

I'm leaning towards option #2, as it seems to have the best combination of meeting the goal of the project, matching the existing pub dependency developer workflows, and - in the case of high resource load due to significant overlap - it would be the easiest to control from the users end (e.g. disable plugins in dependency packages, only enable the plugin in your main application).

As always, I seriously appreciate your time and any input you can provide. Thanks and hope you have a great weekend!

Patt

[Brian Wilkerson]

Lest this sound incredibly negative, I would love for someone to find a better way to define and implement plugins than what we came up with, and I'm glad you're looking at this problem with a fresh perspective. It's quite possible that many of the constraints we placed on our design were not really requirements, but we have considered most of the alternatives you mentioned and had reasons to reject them. My giving you those reasons is not intended to discourage you, but just to let you know our thought process. If we were thinking about things from the wrong perspective please let us know.

I will also say that I'm not sure that I understood accurately everything that you said, so please don't hesitate to correct me.

... from what I can tell analyzer plugins run with a plugin-workspace relationship of one-to-one , where any workspace can have 1+ AnalysisContexts.

If by "workspace" you mean the set of files and directories open in the client, then you are correct.

I'm not exactly sure how the analysis server determines which version of "my_plugin" is used, but from brief testing I believe the server first looks at each context's analysis_options.yaml for the list of activated plugins, and then attempts to find a matching package name within the pubspec, then copies the plugin's bootstrapper from root>tools>analyzer_plugin to .dartServer/.plugin_manager/, etc. etc.

That's essentially correct. It doesn't actually look in the pubspec, it looks in the `package_config.json` file produced by `dart pub` after `pub` has performed its version solving and downloaded the resolved version of each package.

The problems that I foresee with this architecture is in the one-to-one server-to-workspace relationship and the possibility that AnalysisContexts have different package plugin configurations - and in my case, if these packages have different 3rd party lint dependencies. For example, one context may depend on my_plugin v1.0.0, while another depends on my_plugin v2.0.0.

That's correct. One of our assumptions was that most plugins would be designed to provide support for a single package and would be developed by the package authors. The needs of developers using the package will change over time as the package evolves. That means that the plugin supporting the package will also likely need to change over time and the package developers would want to control which version of the corresponding plugin was used based on the version of the package that was being used. Our design supports that by allowing multiple versions of a plugin to be loaded when there are different versions of the package being depended on by the packages that are open. But in order to support that, we are required to load each plugin in a separate isolate.

With the analysis server + LSP, the server version seems to run from the SDK version defined via the Dart Code extension, and therefore seems to attempt to run just 1 plugin instance for the whole workspace / AnalysisContextCollection.

I'm not sure whether that statement matches reality, so let me give you my understanding of what we're doing. The DartCode plugin (or the IntelliJ plugin) knows which Dart/Flutter SDK it is supposed to use. It starts up the version of the analysis server that shipped with the installed SDK. That means that the server, running in the main isolate uses the version of the `analyzer` package that was shipped with the SDK. The server handles all of the code that's open in the client; it doesn't start a separate server for each analysis context. But when there are plugins to be run, each version of each plugin is loaded into a separate isolate so that it can use whatever version of the `analyzer` package it wants to use. There's a 1 to 1 relationship between the client and the server, but a one to many relationship between the server and the plugins it's running.

Assuming that the options are still interesting after getting this far, I'll add some comments.

Option #1

Combining all of the plugins / lints into a single executable has the problems you noted: you can't run multiple versions of the same plugin and resolving versioning conflicts is likely to make it difficult or impossible to actually build an executable that can run more than one set of lints. This places a burden on the plugin authors to update their plugins quickly every time any new version of the underlying packages (like the `analyzer` package) is published, or, conversely, puts pressure on them to never update to a newer set of dependencies. Both cases are bad, imo.

It also makes it hard to respond to changes in the set of directories that are open. If the user opens a new directory that specifies lints you'll need to shut down the running merged executable, build a new merged executable, and start it up. The start-up behavior of the `analyzer` package is suboptimal at best, so having to restart the merged executable isn't ideal. The same would be true when a directory is closed. Worse yet, when rebuilding the merged executable you might very well end up with a different set of lints loaded, which will make the user's experience unstable ("wait, this lint wasn't showing up until I opened an unrelated package").

Option #2

I'm very likely missing something, but it sounds like the middle-man plugin would be doing exactly what the analysis server is doing today. I fail to see how this improves the situation.

Option #3

I know I'm missing something because I don't see what the difference is between options 2 and 3 other than an attempt to share data between isolates.

What I can tell you is that we've looked at sharing data between isolates and there are 2 problems we've run into. First, the data we can share is extremely limited in nature while the data we want to share is extremely rich. That means that we'd have to have some significant amount of serialization/deserialization logic running on both sides. We haven't built a POC implementation, but our back-of-the-envelope estimates indicate that the overhead would be almost as high as just computing the data from the user's source code. Second, it makes it extremely difficult for us to update our data structures. We'd just end up introducing yet another way in which plugins could become incompatible with the server.

--
You received this message because you are subscribed to the Google Groups "Dart Analyzer Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to analyzer-discu...@dartlang.org.
To view this discussion on the web visit https://groups.google.com/a/dartlang.org/d/msgid/analyzer-discuss/001cfae8-b50d-4082-a773-a530e9c09fb5n%40dartlang.org.

[Patt O'Brien]

Lest this sound incredibly negative, I would love for someone to find a better way to define and implement plugins than what we came up with, and I'm glad you're looking at this problem with a fresh perspective. It's quite possible that many of the constraints we placed on our design were not really requirements, but we have considered most of the alternatives you mentioned and had reasons to reject them. My giving you those reasons is not intended to discourage you, but just to let you know our thought process. If we were thinking about things from the wrong perspective please let us know.

I really appreciate this preface, and FWIW i've noticed you and many others on the Dart team really do a great job at encouraging the community while giving feedback in a positive way. I can't stress how much that's appreciated. Anyways, this kind of feedback ("negative" or positive) is exactly what I was looking for, as it provides me either something that I hadn't thought of (saving me many hours of work) or something where I do see things differently and can use to mold a better proposal. Either way, its a win-win.

If by "workspace" you mean the set of files and directories open in the client, then you are correct.

Great! And as a side note, this client "workspace" is represented as an AnalysisContextCollection, correct?

I'm not sure whether that statement matches reality, so let me give you my understanding of what we're doing.The DartCode plugin (or the IntelliJ plugin) knows which Dart/Flutter SDK it is supposed to use. It starts up the version of the analysis server that shipped with the installed SDK. That means that the server, running in the main isolate uses the version of the `analyzer` package that was shipped with the SDK. The server handles all of the code that's open in the client; it doesn't start a separate server for each analysis context.

Amazing, this was my understanding as well.

But when there are plugins to be run, each version of each plugin is loaded into a separate isolate so that it can use whatever version of the `analyzer` package it wants to use. There's a 1 to 1 relationship between the client and the server, but a one to many relationship between the server and the plugins it's running.

...

I'm very likely missing something, but it sounds like the middle-man plugin would be doing exactly what the analysis server is doing today. I fail to see how this improves the situation.

Ahh, this is news to me! I didn't realize that multiple plugins were being run, and in this case what I was attempting to do was in fact going to replicate the behavior of the server. Just to be clear: for every "my_plugin" declared in analysis_options.yaml>analyzer>plugins, the server attempts to spin up the respective plugin package based on the pub-resolved package_config.json. As long as 'pub get' resolves dependencies, then there should be 100% guaranteed success of the plugin executing (and any other lint-related dependencies, for that matter). This is exactly what I had planned in option #2, so this saves me a ton of work :)

The misleading part of this implementation IMO is that the AnalysisContextCollection received by my_plugin includes all contexts of the workspace. If I declare "my_plugin" in "example_application", then one would think that the Collection would only include the "example_app" context + contexts for all of  "example_app"s dependencies. Otherwise, if I have 5 packages open in my workspace, with a unique plugin version for each package, the server will spin up 5 plugins (this is OK), and they will all have access to all 5 packages (this seems useless if the version doesn't match). That said, this can be prevented by having the plugin check each context to see if the plugin version in package_config.json (if any) matches the version of itself, and ignore the context if there's no match - I guess I'm just double checking that I'm understanding this all correctly.

I know I'm missing something because I don't see what the difference is between options 2 and 3 other than an attempt to share data between isolates.

...

What I can tell you is that we've looked at sharing data between isolates and there are 2 problems we've run into...

That all makes sense... I also estimated that #3 was the most unrealistic solution to implement and maintain anyways, and so (at least for now) I'll take your previous estimates on inter-isolate data sharing at face value. 

[Brian Wilkerson]

Great! And as a side note, this client "workspace" is represented as an AnalysisContextCollection, correct?

Correct. The collection is built from a list of the open files and directories and builds the smallest number of analysis contexts possible to cover the analysis of everything that's open.

... the server attempts to spin up the respective plugin package based on the pub-resolved package_config.json.

Assuming that the same version of the plugin isn't already running, yes. Plugins are expected to be able to handle multiple analysis contexts.

As long as 'pub get' resolves dependencies, then there should be 100% guaranteed success of the plugin executing (and any other lint-related dependencies, for that matter).

And assuming that there are no bugs in the plugin that cause it to crash, yes.

The misleading part of this implementation IMO is that the AnalysisContextCollection received by my_plugin includes all contexts of the workspace.

If a plugin is passed any analysis contexts for which it shouldn't be run, then that's a bug. It wasn't intended to work that way. The design was that each plugin would be passed exactly the list of analysis contexts for which the plugin had been enabled and nothing else.

Option #3

As I'm sure I've said before, we haven't yet made a decision about whether or not we're supporting plugins, whether in their current form or in some other form. But if we do I'm reasonably confident that it will only happen if we can find a reasonable way to share more data across isolates. The current architecture just doesn't scale the way it needs to.

[Patt O'Brien]

If a plugin is passed any analysis contexts for which it shouldn't be run, then that's a bug. It wasn't intended to work that way. The design was that each plugin would be passed exactly the list of analysis contexts for which the plugin had been enabled and nothing else.

Got it, then there's definitely a bug with v0.1.11, as every context within a client workspace is received by the plugin. I can test with .12 and open up an issue for this, if that makes sense.

As I'm sure I've said before, we haven't yet made a decision about whether or not we're supporting plugins, whether in their current form or in some other form. But if we do I'm reasonably confident that it will only happen if we can find a reasonable way to share more data across isolates. The current architecture just doesn't scale the way it needs to.

Right, I've seen lack of official support mentioned before, but have never seen any mention of the reasoning. Fortunately the open source nature of this project means building an analyzer project doesn't live or die by the Dart team, and I've done a bit of early prototypes of my project to get an idea of risks, including understanding how much effort would be required if Dart was to not support plugins in the long run.

That said, any chance you could elaborate on what the main obstacles to official support are? Strictly Google resource limitations, or more so lack of use cases to make investment worthwhile? What are the use cases that you see isolate data-sharing being a necessity for? 

FWIW, my project uses a middleman-plugin to aggregate LintRule subclasses (defined in any depended-on packages) into an array which is passed to the plugin's main entrypoint; so lints from different packages can run on the same thread (I'm fairly certain this is how build_runner works behind the scenes too). This design seems solve resource issues around scalability, at least for the use cases I've envisioned, but I'm curious if you've tried something similar and didn't find it suitable for some reason?

[Brian Wilkerson]

Got it, then there's definitely a bug with v0.1.11, as every context within a client workspace is received by the plugin. I can test with .12 and open up an issue for this, if that makes sense.

It seems quite likely that the bug will still be there in 0.1.12. I'm not aware of any work that would have fixed it.

Fortunately the open source nature of this project means building an analyzer project doesn't live or die by the Dart team ...

That's true, but if we were to decide to not support plugins then I wouldn't be surprised by a decision to remove the hooks from the analysis server that allow plugins to run at all. 

That said, any chance you could elaborate on what the main obstacles to official support are? Strictly Google resource limitations, or more so lack of use cases to make investment worthwhile? What are the use cases that you see isolate data-sharing being a necessity for?

I suppose it boils down to resource constraints at some level, but the biggest issue is that the current implementation isn't performant enough to provide a high quality experience for our users, and we don't want to ship something that will make the Dart / Flutter developer experience worse. The current hypothesis for why performance is so bad is that it's the result of having to (a) analyze everything multiple times and (b) having to store all of the data in memory multiple times.

I don't know whether it's realistic, but ideally I'd like the server to be able to fully analyze everything and then pass the results across to the plugins so that they wouldn't use any extra memory or processor cycles beyond what it takes for them to perform their own incremental analysis. We might never get there, but there might be something part of the way there that would be good enough.

FWIW, my project uses a middleman-plugin to aggregate LintRule subclasses (defined in any depended-on packages) into an array which is passed to the plugin's main entrypoint; so lints from different packages can run on the same thread (I'm fairly certain this is how build_runner works behind the scenes too). This design seems solve resource issues around scalability, at least for the use cases I've envisioned, but I'm curious if you've tried something similar and didn't find it suitable for some reason?

No, the only approach we explored was the one that's currently implemented. It meets the objectives that we outlined when we started and proves that it possible to have some kind of plugins, which was the goal of the effort.

My understanding is that what you're doing is looking at the depended-on packages and generating something that effectively a mini-plugin that contains all of the `LintRule` subclasses. When you say "the plugin's main entrypoint" I'm guessing that the plugin you're referring to is the generated plugin (not the middleman plugin).

If that's accurate, there are a couple of things that concern me about that approach.

The first, as I think we've touched on before, is that it's hard to ensure that the `LintRule` subclasses are all compatible with the version of the packages loaded into the isolate running the generated `main`. If you update your generator (the middleman plugin) to use a newer version of the analyzer so that lints can deal with newer language features, but one of the lint rules is expecting on older version of the AST structure, then you'll likely get either a compile-time error or a runtime error. The problem of keeping all of the community developed lint rules on the same version of the analyzer seems hard to solve.

The second is that we have had enough reports of excessive memory and CPU usage when there are no plugins loaded that even if we're only doubling those amounts by having a single plugin it's hard to say that we've solved the scalability issues.