When using the properties view the editor becomes blocked till the analysis is finished, for each keystroke.

Without the properties view the editor does not get blocked and there gets a big list of “Analysing updates to …”. Which obviously is a waste of computation (because its doing analysis on outdated ASTs) but at least doesn’t block the editor.

How to reproduce:

1. create a language in which the analysis of a 200 line file takes more than a second.
2. compare the behaviour with and without properties view: editor-properties

Task: make the properties view not block the editor.

Submitted by Daco Harkes on 27 March 2014 at 15:11

My hunch is that the property view does not request or wait for the analyzed AST (because this is not possible in the current Spoofax architecture), but probably uses the getAnalyzedAst function to get an analyzed AST, which will (re)trigger analysis. This analysis probably then gets executed in the UI thread, which blocks the UI from rendering.

The properties view uses getCurrentAnalyzedAst which doesn’t trigger a re-analysis.
I think the problem is that things are not executed in parallel. It seems like the properties view transformation ends up in the same queue as the analysis, so if it’s currently analyzing and you change the selection in the editor (by typing or clicking), the editor blocks until analysis is finished.
You can see this when you change the selection within 500ms after editing: this doesn’t block the editor because the analysis is not yet queued. But if you click at let’s say 700 ms after the edit, the editor blocks.

Fixing this will requires some changes in the StrategoObserver. I had a quick look at the code and I think that because of getLock().lock(); everything gets executed sequentially. Probably something will break when you remove this lock, but I’ll have a try and see what happens.

Ah, good, do you somehow wait for the analyzed AST then? And yes, Stratego execution is sequential, and removing that lock will most likely break stuff. But shouldn’t both the regular analysis, and property view transformation be executed in a background thread, and feed back information if this is done? Or is this not how it works?

Fixed with https://github.com/metaborg/spoofax/commit/f84a98c30c4d2549d6d859e917b4cf36c3b9c40f
I removed the call to lock() from getCurrentAnalyzedAst and from the properties view service. It wasn’t necessary to remove it from the StrategoObserver.

Now, the properties view is blank while analyzing, which I think is desired behavior.

I think this introduces a race condition where you try to get the AST while it is being set in the StrategoObserver. If calls to resultingAsts.get and resultingAsts.put happen concurrently, things will go wrong. Do you think they can happen concurrently? If so, you need to change the observer to either use a concurrent hash map to store the resulting AST’s, or synchronize access to it.

I didn’t see your other comment earlier. But I think you’re right that it should be executed in the background. Also because everything feels a little bit slower now because of the extra transformation when you change the selection. Let me see if I can make Eclipse do it that way. I’ll undo my earlier changes because I think you’re right that multiple threads can access the map concurrently.

Things are now properly executed in the background with an Eclipse Job.
https://github.com/metaborg/spoofax/commit/2ef040ce3adacdb784fff216a47bf2c156cf419a

Great, thanks! The lock should prevent a race condition.