Skip to content
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

Does JDT-LS support org.eclipse.jdt.ui.javaCompletionProposalComputer extensions? #613

Open
mickaelistria opened this issue Apr 3, 2018 · 15 comments

Comments

@mickaelistria
Copy link
Contributor

org.eclipse.jdt.ui.javaCompletionProposalComputer allows to add specific completion engines to JDT (think about Jax-RS, CDI...). Is JDT-LS able to consume those extensions? It seems to me that although this API is meant first to be consumed in JDT UI, there is almost nothing in it that requires a UI Thread to work.
So if JDT-LS doesn't support it yet, it'd be worth considering consuming this extension point (either through existing JDT APIs, or by reading directly the IConfigurationElements).

@gorkem gorkem added the question label Apr 3, 2018
@gorkem
Copy link
Contributor

gorkem commented Apr 3, 2018

No, it does not. We can not directly use jdt.ui extension. There are parts of jdt.ui which makes a lot of sense headless but overall the bundle does require the whole Eclipse UI. I am not sure about using org.eclipse.jdt.ui.javaCompletionProposalComputer through IConfigurationElements either. Usually, the implementors of this extension point are on UI bundles and it is possible that they may not be much usable in this context or even worse they may look like working at a glance and just trigger a whole UI dependency. We can introduce a non-ui version of the extension point in jdt.ls though.

@mickaelistria
Copy link
Contributor Author

Introducing a specific extension point in jdt.ls would be a bad idea. If we want content assist that works in any use-case of JDT, it has to be in jdt.core, and used by both JDT-UI and JDT-LS. I'm rather skeptical than many extenders would contribute to jdt.ls without being able to try it easily in Eclipse JDT/IDE first as it would make the development workflow rather tedious.
About reading the IConfigurationElements targeted to JDT-UI, I think it would be good to be able to do it anyway, but it would remain the responsibility of the LS packager to verify that the completion provider (or similar) does work properly.
About IJavaCompletionProposalComputer, it indeed seems to have a relatively strong dependency on TextViewer (via ContentAssistInvocationContext ), but this same class which clients are expected to consume can have a null viewer and rely only on document. So a given IJavaCompletionProposalComputer would already need to check whether the viewer in contentAssistInvocationContext is null or not to decide whether to do UI stuff of not. IMO, the API is relatively ready for the use case of jdt.ls, and it's mostly a matter of using it.

I think the constraint of not importing any UI bundle will (does?) quickly show its limits and will introduce a high cost of re-development where we can have a low cost of re-use. While I agree it's important to keep the default stuff as lean as possible, it's also even more important to quickly allow integration of features that have been around for about a decade in traditional Java IDEs if we want other IDEs to be successful for enterprise Java, and if we want enterprise Java to be successful in the Cloud development story.
Allowing dependencies on UI bundles (without necessarily starting the UI Harness) would be a good step forward. And even if it provide enough value, why not even allowing to starting a UI Harness in an invisible X server?

In the end, for cloud deployment of the LS, the main thing that's going to make a difference is multi-tenancy, way more than optimization of a the LS itself.

@gorkem
Copy link
Contributor

gorkem commented Apr 3, 2018

I agree, it would be ideal if jdt.core hosts such an extension. We could offer it to jdt.core and see if it sticks.

I am not convinced on the dependency to UI or even worse running an invisible X server. Adding all that UI dependency to jdt.ls for the purposes of using a small portion is just bad user experience. Right now jdt.ls is a small ~40MB engine, if we add the UI dependencies that will quickly go over 100MB, which will affect startup times and memory requirements. Unlike the Eclipse IDE the user of jdt.ls (vim, vscode, atom) very often start their editors many times a day and the agility of jdt.ls is important. Also on the cloud scenarios, running larger servers means larger memory consumption which makes running LSPs more expensive.

@yaohaizh
Copy link
Contributor

yaohaizh commented Apr 4, 2018

If we need integrate recommender: #60, this extension point should be provided?

@mickaelistria
Copy link
Contributor Author

I agree, it would be ideal if jdt.core hosts such an extension. We could offer it to jdt.core and see if it sticks.

+1.

Adding all that UI dependency to jdt.ls for the purposes of using a small portion is just bad user experience.

I don't get what makes you talk about user experience at this point as the LS is a blackbox and its content are not perceivable. So I imagine you're mostly considering the download size and the time for first startup at this point, and that more or less only in VSCode which does setup the size restriction for an extension. Maybe it can become a UX issue for VSCode because of this, but to be clear, all my thoughts here go to Eclipse Che and Cloud deployment way more than to VSCode.

Unlike the Eclipse IDE the user of jdt.ls (vim, vscode, atom) very often start their editors many times a day and the agility of jdt.ls is important.

I think that while it is important, users would be fine wasting a few more seconds at startup if the completion helps them much more. LSP is asyncronous exactly to handle those cases.
And again, those "light desktop editors" are not the use case that I have in mind at this point, and in cloud deployment like Eclipse Che, and even in other editors, there could be some smartness developed to pre-start LS according to project content and then do not make the user be affected by startup time too much.

Also on the cloud scenarios, running larger servers means larger memory consumption which makes running LSPs more expensive.

Sure, but for projects that we know require more advanced Java support (Java EE), wouldn't it be worth the price? I think it would, otherwise Che will basically never capture much real Java EE developer used to Eclipse IDE/NetBeans/IntelliJ and related power that's strongly missing in JDT-LS.
Running 1 3GB server for 100 users as less expensive than running 100 30MB servers and can drive to more powerful results. Multi-tenancy in JDT-LS for Cloud deployement is going to be the real problem and solution here.
Once we have something working for Java EE, we can consider some optimizations, but the use-case is currently blocked by JDT-LS rejecting all idea of adding extra-dependencies and the trend of duplicating existing extension point for JDT-UI is a very expensive and not so sustainable one.

Hence, I suggest the creation of a "org.eclipse.jdt.ls.ui" bundle which would have a similar application to run the LS, but that would start a display in order to try running some bundle which have requirements on a Display, and try to get the input from JDT extenders too (like CodeRecommenders as well as WebTools or JBoss Tools). From there, we can evaluate the user stories and user experience and consider which extensions we should work on first for better results.

@gorkem
Copy link
Contributor

gorkem commented Apr 4, 2018

User experience starts even before the user start the product so even how we package and where we provide downloads matters. Also jdt.ls (or any other LS) project does not have the luxury to ignore some clients and optimize for others.

Running 1 3GB server for 100 users as less expensive than running 100 30MB servers and can drive to more powerful results. Multi-tenancy in JDT-LS for Cloud deployement is going to be the real problem and solution here.

If you have a solution for this one I am all ears.

Once we have something working for Java EE, we can consider some optimizations, but the use-case is currently blocked by JDT-LS rejecting all idea of adding extra-dependencies and the trend of duplicating existing extension point for JDT-UI is a very expensive and not so sustainable one.

jdt.ls is not blocking the JavaEE use case, the dependencies you want to add for the solution that you believe is right require Eclipse UI and you are asking for a headless LSP implementation to support it. Please do not assume that adding 400MB of Eclipse JavaEE tooling is the correct approach and accuse jdt.ls of blocking or rejecting. What you are asking is a huge change for jdt.ls and has impact for all the adopters of jdt.ls which you admittedly did not consider.

@fbricon
Copy link
Contributor

fbricon commented Apr 4, 2018

BTW jdt.ls is not designed to support concurrent users/clients. So 1 server/100 users is not gonna happen any time soon.

@tsmaeder
Copy link
Contributor

tsmaeder commented Apr 4, 2018

Code completion contributions being tied to UI is and architectural mistake, IMO. I don't think we we should perpetuate it in jdt.ls.
@mickaelistria the Che team has invested quite a bit to reduce workspace startup time. While some of the work is asynchronous, you still have to wait for the LS to be ready before having a functional IDE.
But what is more important that memory consumption is indeed important: memory directly costs money, and especially on the free tier, we can't assume having copious amounts.
Can we come up with a change for jdt that would offer a headless extension point in jdt.core, with the current extension point being implemented in terms of the headless one? In this way, we would allow existing clients to migrate slowly to a better architecture.

@tsmaeder
Copy link
Contributor

tsmaeder commented Apr 4, 2018

@mickaelistria btw: what are you trying to achieve? Replace jdt with jdt.ls in Eclipse?

@mickaelistria
Copy link
Contributor Author

Also jdt.ls (or any other LS) project does not have the luxury to ignore some clients and optimize for others.

That's where the fun begins: in practice, it will most likely have to be optimized for some clients rather than others. Deploying on the cloud for usage in a Web IDE doesn't have the same constraints/power than deploying on workstation.
As desktop IDEs are already covering the Java/Java EE story very well and better than what JDT is very likely to achieve soon, I see more future and need for JDT-LS in the Cloud deployements, where we can do things differently.

If JDT doesn't have the luxury to have some optimized deliveries for 1 or 2 specific clients, I don't get how it can have the luxury to re-implement existing many extension points from JDT-UI and to make extenders adopt new extension points...

If you have a solution for this one I am all ears.
...
BTW jdt.ls is not designed to support concurrent users/clients. So 1 server/100 users is not gonna happen any time soon.

It would be mostly a matter of having multi-tenant workspaces in the same Eclipse IDE in general. I didn't evaluate the issue, I imagine it's a hard one, but if it's of interest for many developers, it might be tackled one day.
If you think this path is worse following, I can ask some other contributors who may have already evaluated the feasability/difficulty.

btw: what are you trying to achieve? Replace jdt with jdt.ls in Eclipse?

I wanted to evaluate whether JBoss Tools could contribute to JDT-LS in order to have something like a "JBoss Tools Language Server" containing CDI, Hibernate & Jax-RS edition features (completion, validation, hover...).
I indeed tried to use JDT-LS in Eclispse IDE, and it worked, and then I augmented JDT-LS with some pieces of JBoss Tools (but it could be Spring Tool Suite, Code Recommenders, SonarLint, Groovy-Eclipse...) and it failed.
It failed because like the vast majority of current Java-oriented Eclipse IDE ecosystem, those bundles are requiring a UI Thread and a Display to exist.

Code completion contributions being tied to UI is and architectural mistake, IMO. I don't think we we should perpetuate it in jdt.ls.

Basically, all the legacy Eclipse Platform ecosystem is built on top of that mistake. One question is whether JDT-LS can ignore all this legacy and got everyone to reimplement everything in a different way?
My opinion on that question is that JDT-LS will never manage to get as strong as Eclipse IDE or other desktop IDEs without allowing the legacy ecosystem; and that as a result, it will fail at providing a good offering for some typical stack of Enterprise Java and as a consequence Che and others will not attract bigger projects.

memory directly costs money, and especially on the free tier, we can't assume having copious amounts.

So a fat JDT-LS can be a way to enable Java EE completion in JDT-LS, but that "hosts" of the Che instance (who pays the bill for memory consumption) could restrict to commercial instance if they want to.

Overall, I've made my proposal, and I still think it's a good idea to find a way to allow dependencies which have dependencies on UI (ie having another -bigger- JDT-LS app which would be able to consume extensions coming to jdt.ui extension points and to enable all the legacy/current ecosystem of extensions (CodeRecommenders, STS, Java EE, SonarLint...). Without it, JDT-LS will have to re-implement progressively the same extension points than JDT-UI and hope the extenders would also adopt those new extension points quickly. It seems extremely optimistic.
Or JDT-LS goes in the direction of making the LS not extensible at that grain and assumes each extension to Java editor should start 1 dedicated Language Server (this is the approach of STS these days I believe). This approach also would have a cost in resources and would probably end up in duplicated efforts across various LS. In the end, it would probably become more expensive as the number of LS on the same file grows.

@tsmaeder
Copy link
Contributor

tsmaeder commented Apr 5, 2018

@mickaelistria I think you're proposing a false dichotomy here: the third way is to start moving stuff (extension points) that are not dependent on UI out of jdt.ui and into jdt.core proper (or a module at the same level). That way, we improve both Eclipse IDE AND jdt.ls. If indeed the code is not dependent on the UI, it would be simple for clients to consume the new extension point.

@maxandersen
Copy link

How about we lift things up here a bit and make this about how do we with LSP's somehow supporting extendability without requiring N full blown LSP's to be loaded - i.e. is it really necessary to do like STS guys do and run multiple JDT based LSP's or is there a way to share/extend ?

The idea @mickaelistria propose is to reuse jdt's UI bound completion proposal extension mechanism which on paper sounds like a great idea as it would enable fairly easy reuse of things like hibernate tools, jaxrs and others. Problem is though that due to the mistakes/decisions made in Eclipse on having these extensions be UI bound the overhead is very significant - and even if you ignore that part I'm sure Hibernate tools and jax assumes a lot of things about the running environment and setup that it will be hard to impossible to actually reuse in a world of LSP's.

Thus yes, to support these things in a good way they will need to be reimplemented as opposed to just blindly reused.

I would love to be able to directly reuse Eclipse existing ecosystem but I do not see how since majority of it is as you said built on the mistakes made that drags in a large dependency set AND have a lot of hard couplings that is just disastrous when it comes to reuse. Only reason jdt.ls exists is that JDT core is in itself decently decoupled from all this .ui coupling. If it was not we would be looking at other options.

Thus I think the question is better answered by looking into how LSP's could be extendable - once that is defined we can start doing things right BUT others could also go and map that into a UI dependent extension if they found a plugin that would be okey with it.

@mickaelistria
Copy link
Contributor Author

mickaelistria commented Apr 5, 2018 via email

@maxandersen
Copy link

Do you have examples of such constraints? I think I need those be be
convinced about the following stuff.

Hibernate tools needs a Console Configuration - that console configuration is tied to alot of eclipse specfic preferences and storage. There are some extensibility in hibernate tools as we used the same for ant tasks but even if that is solved you still have the issue of user classes having to be loaded into the running java VM that the LSP is running in. That is an old known issue/limitation that I always wanted to have fixed.

jaxrs have similar - settings and project configs coming from eclipse settings; but I don't think jaxrs needs to load users classes.

It seems to me that the startup time + disk space + memory usage is the
"only" drawback in reusing.

No, its also about continuing to build on an architecture that already proven to not scale well when it comes to how long time it takes to make extensions; how overhead accumulates etc.

Wouldn't it be simpler to improve existing extensions that we want to make
them easier to reuse in jdt.ls (using the jdt.ui extension points) than to
reimplement a set of extension points ?

I don't think there is a simple answer to this - but I don't think Enterprise Java Support is all about extending jdt - its also about making it easy to start servers, do deployments and incremental updates. This work might be as simple as just configuring maven/gradle in a good way for users. We don't have to do things as "full" as Eclipse WTP required.

The protocol is only a communication/messaging protocol, it wouldn't
specify anything about internal extensibility of a given LS which is inside
the back box.
So I assume you're specifically talking about making jdt.ls extendable.

not really, I don't think this issue of LSP's being able to serve/extend other LSP's is unique to jdt.ls.

Although it may not be the best approach ultimately, I don't think there
are more pragmatic and realistic way to enable more extensibility in jdt.ls
in a short-ish timeframe.

lets identify which features will actually be most useful and then see what is doable before we lock into "pollute" jdt.ls as the only delivery mechanism of eclipse ui bound extensions.

@mickaelistria
Copy link
Contributor Author

As I think we all agree that adding extension points should be placed at a lower level than jdt.ls for more reusability for both jdt.ls and jdt.ui simultaneously, I've opened https://bugs.eclipse.org/bugs/show_bug.cgi?id=533940

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants