search

angelozerr / tern.java

Use tern.js in Java context
http://ternjs.net/
Other
249 stars 52 forks source link

Improve tern file synchronisation #171

Closed angelozerr closed 9 years ago

angelozerr commented 9 years ago

The PR of @piotrtomiak https://github.com/angelozerr/tern.java/pull/154 has cleaned a lot tern file synchronization, but I think we should improve it because there is 2 problems :

To fix UI freeze, we must give the capability to stop the post of each JS files to tern server by the user.

Today, when completion, hover, validation, hyperlink is executed on the first, it loop for each tern script path and post JS files to tern server, but I tell me if we could post JS files on the background :

In other words, tern server is every time synchronized before completion, hover, validation, hyperlink is executed.

So I would like to provide 3 strategies (that user could choose with preferences) to synchronize tern files with tern server : 

"loadEagerly": ["scripts/**/**.js"]

@vrubezhny @dgolovin @mickaelistria @maxandersen @pascalleclercq @gamerson @PaulVI @fbricon @piotrtomiak I think it's an important changes, any comments are welcome. Thank's!

paulvi commented 9 years ago

no, comments. I don't really follow ideas

piotrtomiak commented 9 years ago

@angelozerr I think that current solution for synchronizing files is relatively good and it only requires some improvements rather than total redesign. There are a lot of different aspect of the synchronization, but I think it's best if I start with the script path:

  1. In MyEclipse we have been experimenting with a very tight integration with JSDT, such that TernProject directly uses JSDT script path and so far this works just fine. This approach requires that we make multiple modifications to JSDT and also make Tern fully dependent on JSDT, which we ship with MyEclipse. Given that Tern Java cannot completly depend on JSDT, the direction we're researching is not directly applicable to your project, but JSDT script path has all the required capabilities, including support for excluded paths, which (paths) could be easily modified by the user. So I think that capabilities of Tern Script path should be extended, so that it can handle all of types of JSDT scriptpath elements, instead of abandoning usage of JSDT scriptpath.
  2. As for scripts residing in the users Web Root folder for which there are Tern Modules. Tern.java could detect such scripts or even folders (like in case of Dojo), add them to excluded resources and configure appropriate Tern Modules. This feature can be even very simple and just looking at script names. When found some, it could present the user with a dialog with actions to do, which user could accept.

As for the script path strategies:

  1. I would be against using loadEagerly as this will only limit our influence on synchronization, because it will be happening outside of the IDE.
  2. As for the index manager - this will only increase complexity.

I am for extending capabilities of the current simple solution:

  1. When any JS or HTML file of the project is opened in the editor, a job could be fired calling ensureSynchronized() to upload all tern scripts to the server. You wouldn't want to do it automatically for all projects, as each project starts a new Tern server and that can be very costly with memory.
  2. Instead of keeping an index of files accepted by the server, you could keep an index of files scheduled to be sent to the server. That way files could be sent asynchronously from the calling thread. Sending small packs of files (20-60) per each request will allow to make a completion request even if not all files are synced.
  3. There would be a second index of files keeping those to refresh and those would be detected through the existing ResourceChangeListener in IDETernFileSynchronizer. If file is in the sent files set, add it to refresh set.
  4. When ensuring synchronized you scan the whole script path. That's very quick as it works on cached workspace contents model. During that scan, based on the list of files scheduled for sending to the server you can create set of newly added files and with addition of set of files to refresh you can calculate a set of files, which needs to be removed or updated.
  5. When doing the request, file on which the request is fired should be attached directly to the request (TernDoc) and removed from the list of files pending for sending to the server (if it is there). That way file, for which request is being done will always be there.

Last but not least, the UI freeze. That can be fixed by delegating server request call to a non-UI Job. When you kick off the job you can wait in UI thread for, let's say, 500ms for it to finish and if it doesn't you just continue with empty completion list, hover or hyperlink. I don't know if you can cancel a request, I guess not, so the request job will run until it finishes and until that moment a new one should not be kicked off again.

I hope that those ideas will help you deciding in which direction to push Tern file syncing.

angelozerr commented 9 years ago

Many thank's @piotrtomiak for your great answer.

I will take time to read yoru suggestion, but I would like just to say you that since 0.6.0, JSDT script path are managed too (but there is some bugs that I must fix it).

See https://github.com/angelozerr/tern.java/blob/master/eclipse/tern.eclipse.ide.jsdt/src/tern/eclipse/ide/jsdt/internal/JSDTClassPathManager.java This class transform the JSDT install path to tern script path. If you go at tern script path, you can see it (with (jsdt) label)

piotrtomiak commented 9 years ago

@angelo, I am aware of JSDTClassPathManager. That’s actually what we started with, but then it turned out that avoiding synchronization problems (between Tern and JSDT script path) and depending completely on JSDT script path was so much better. JSDT has even support for exclusion/inclusion patterns for each source folder and I think you should take their syntax into account when doing https://github.com/angelozerr/tern.java/issues/150 .

angelozerr commented 9 years ago

Many thank's again @piotrtomiak for your answer.

I have implemented a TernBuilder to validate the whole of JS files of the project with tern-lint (see https://github.com/angelozerr/tern.java/issues/174) and I tell me if I could use TernBuilder to call the tern file synchronizer (by using the IResourceDelta coming from builder) to manage add/remove file.

What do you think about that?

piotrtomiak commented 9 years ago

@angelozerr . I think that it is not a good idea. Here is why:

  1. Tying up resource synchronization with the builder will require the project to be built, for the content provider/AngularJS stuff to update. If user has switched off "Build automatically", especially, if the project is purely JavaScript, he may never figure out why content assist is not updating. Even being an eclipse savyy, I am having such troubles with normal java projects as well from time to time ;)
  2. Tern server times out and has to be treated temporary, so it is clearly not fit for the builder approach, which assumes that results are permament (like compiled java classes). On the contrary, validator results are permament, as they are persisted in the workspace, so even if validator uses a temporary Tern server, it's results are not temporary.
  3. You definitly don't want to start Tern server for each project in the workspace. Imagine user having 50 Web projects. That would kill his machine ;)

If you don't feel confident with the performance of current solution, I would go for listening to resource change event instead. Resource change event could trigger a job, which would asynchronously send script contents to the server. If the server times out in the meanwhile, content assist invocation itself can trigger scripts resent. However, I feel that an approach I have described in my previous comment results in a better user experience. When user activates an editor, that is a very good moment to resync project contents with the server in a background job for up-to-date content assist. Usually, for the first time, it won't take more than few seconds, which is a time before user invokes content assist. Of course in the AngularJS area, or for validation, resync would have to be fired off in a different moment, and probably synchronously.

angelozerr commented 9 years ago

@piotrtomiak thank's a lot for answer!

At first I have implemented TernBuilder https://github.com/angelozerr/tern.java/issues/174 to use tern-lint for the whole JS files. For the moment it's not really optimized that's it is disable by default.

When I speak about builder using to synchronize tern file, I meant NOT full building but with delta. My idea was to keep the existing synchronizer and for delta using builder. But you are right, if user disable the builder it will not work.

So if I have understood your idea :

My big problem is how to manage the first synchronization with freeze? I tell me if on the first time, the full JS synchronisation should be done inside a Job that user could stop. If we do that it means that completion, hyperlink, validation etc will be disable upon synchronsation is done.

What do you think about thoses ideas?

Thank's for your help.

piotrtomiak commented 9 years ago

@angelozerr I can see that the file deletion has been implemented - that's cool! In MyEclipse I have done that by uploading an empty file content for deleted files, but it's cool that there is an API for it now. We have spent a significant amount of time to get asynchronous upload of scripts along with an asynchronous content assist. The main problem was that there is no syncing in TernServer support with an assumption that stuff is run on UI thread. It would be cool, if you can download MyEclipse, check the behaviour and let me know what you think about it. A huge script library, like Dojo (~18MBs) copied before you open JS or HTML file, would serve well here. If you feel that this is something you would like to have, we are open to contribute :)

angelozerr commented 9 years ago

@angelozerr I can see that the file deletion has been implemented - that's cool!

Yes! great thanks to @marijnh who is everytime opened to improve ternjs (see https://github.com/marijnh/tern/issues/437)

The main problem was that there is no syncing in TernServer support with an assumption that stuff is run on UI thread.

perhaps it's an new issue for ternjs? Do you think it's possible to manage asynch request with node?

We have spent a significant amount of time to get asynchronous upload of scripts along with an asynchronous content assist.

Today I think it's a big problem with tern.java, upload a lot of JS files freezes the UI when completion is executed. It should be very fantastic if you could contribute to tern.java!

I'm very exciting with 0.8.0 https://github.com/angelozerr/tern.java/wiki/New-and-Noteworthy-0.8.0 I have too implemented tern lint builder https://github.com/angelozerr/tern.java/issues/174

But the very cool features that I'm working is about tern completion for object literal. See https://github.com/marijnh/tern/pull/449

Once it will will be implemented, I have the intention to support file completion for instance for templateUrl property of $routeProvider#when to have completion with HTML files!

So I'm very busy, but I would like to improve the tern files synchronization with big files, so any contribution are welcome!

piotrtomiak commented 9 years ago

The main problem was that there is no syncing in TernServer support with an assumption that stuff is run on UI thread.

perhaps it's an new issue for ternjs? Do you think it's possible to manage asynch request with node?

So far your implementation was mostly working in the UI thread, so no sync was required in your code. With asynchronous file upload, there was concurrent access to NodejsTernServer class, which resulted in errors. So now, the files are being uploaded in a Job, but I guess Tern server is processing them synchronously, which is still OK. The main thing is to not lock UI.

I would like to get your opinion on MyEclipse behaviour in this area, before we contribute, as this is going to be rather large contribution and I would like to have a plan in place, before I start. Anyway, I will be able to start working on this in the beginning of Jan as we are finally closing MyEclipse 2015 GA right now.

angelozerr commented 9 years ago

I would like to get your opinion on MyEclipse behaviour in this area, before we contribute, as this is going to be rather large contribution and I would like to have a plan in place, before I start.

I have tested and it's a really cool features! Completion is not available until tern server is initialized with a lot of JS files. So it fixes freeze UI like https://github.com/angelozerr/angularjs-eclipse/issues/109

Very cool! It should be fantastic if MyEclipse could contribute to tern.java with this feature.

It's out of the scope of this issue, but I think MyEclipse could be interested with https://github.com/angelozerr/eclipse-wtp-webresources (that you can install with AngularJS Eclipse).

piotrtomiak commented 9 years ago

@angelozerr In the first place: happy New Year :) I was planning to start working on the contribution this week, however we still have some stuff to fix before the GA release going out on 15-th of Jan. It looks like I will start work on 19-th of Jan. Sorry for such a delay!

It's out of the scope of this issue, but I think MyEclipse could be interested with https://github.com/angelozerr/eclipse-wtp-webresources (that you can install with AngularJS Eclipse).

I think so, I stumbled upon this plugin some time ago, but there was no time to put it into the release ;) I hope to include it into next releases.

angelozerr commented 9 years ago

@angelozerr In the first place: happy New Year :)

Happy New Year!

It looks like I will start work on 19-th of Jan. Sorry for such a delay!

No pb, it's a very cool news for tern.java. I'm very exciting to see this feature inside tern.java!

Do you think it's possible to turn off asynch completion with a preferences settings?

I think so, I stumbled upon this plugin some time ago, but there was no time to put it into the release ;) I hope to include it into next releases.

Cool!

I think too you could be interested by https://github.com/angelozerr/eclipse-wtp-xml-search

Liferay IDE uses now this project for their XML descriptor (portlet, etc). I could tell you more info, if you need.

angelozerr commented 9 years ago

Doen with https://github.com/angelozerr/tern.java/wiki/New-and-Noteworthy-0.9.0#async-tern-completion