fix #2315: fine grain marker tree computation

[akosyakov]

No description provided.

[akosyakov]

@marcdumais-work please try, it turned out that:

• the whole marker tree was rebuilt even if markers only for one resource were reported, now it should be incremental
• saving data to the local storage is expensive, we store markers whenever any marker is changed. I've added debouncing, but I think we can actually save them only once on page unload similar to how we store the layout info or don't store markers at all

[svenefftinge]

I think we should even consider not storing markers in storage at all. It might auto-fix a couple of issues we have with stale markers.

[akosyakov]

@svenefftinge @marcdumais-work I've removed storing markers by default, a specialization of marker manager can decide on their own. For cases which I know it does not make sense to store them, but can imagine that it can be useful for breakpoints for example.

[marcdumais-work]

will try this PR now, thanks @akosyakov

[marcdumais-work]

I've removed storing markers by default, a specialization of marker manager can decide on their own. For cases which I know it does not make sense to store them

True, e.g. for TypeScript, where the markers are created by the LS when a file is opened and cleared when it's closed. In that case, the markers will be re-created when we reload a workspace, since the opened files will be restored.

When we have markers created during a build, we will need to re-trigger the build to get back the markers after a reload, I think? Or have the build component itself res/store the markers maybe?

[svenefftinge]

The problems widget should also not store its state.

[akosyakov]

When we have markers created during a build, we will need to re-trigger the build to get back the markers after a reload, I think? Or have the build component itself res/store the markers maybe?

In VS Code they don't store markers and expect that a language server reports all of them each time. For us, it should be safe to assume the same. Also, I see that Java LS reports them each time.

[marcdumais-work]

I see that Java LS reports them each time.

This might be incidental. From a previous discussion, I remember that the jdt creates an eclipse workspace each time Theia is started, IIRC to avoid issues with (Eclipse) WS corruption. If not for that, it might not build each time.

[marcdumais-work]

I tried the PR and it's really fast now - as fast as jdt can provide the problems, it seems.

I noticed a bug: when I toggle the Problems view, it comes back empty. Similar, if I start with Problems view not present and open it later, it's also empty.

[akosyakov]

This might be incidental.

They have to report each time, otherwise, there won't be problem markers in VS Code.

I noticed a bug: when I toggle the Problems view, it comes back empty. Similar, if I start with Problems view not present and open it later, it's also empty.

thanks! i will have a look

[AlexTugarev]

From a previous discussion, I remember that the jdt creates an eclipse workspace each time Theia is started, IIRC to avoid issues with (Eclipse) WS corruption.

Actually, it's @theia/java configuration. For the same workspace, I think it should reuse the same eclipse workspace, cf. #1745. That would speed up reloading a lot.

If not [recreating a workspace], it might not build each time.

We can test it easily, by using a fixed path for the eclipse workspace, and see if the markers are reported again. AFAIK resource markers are persisted in eclipse, might be that there are reused.

[marcdumais-work]

it's @theia/java configuration

@AlexTugarev do you know where that config is, that I could point to a fixed path?

[AlexTugarev]

@marcdumais, yes it's here: https://github.com/theia-ide/theia/blob/master/packages/java/src/node/java-contribution.ts#L45

[marcdumais-work]

@AlexTugarev Thanks!

[marcdumais-work]

We can test it easily, by using a fixed path for the eclipse workspace, and see if the markers are reported again. AFAIK resource markers are persisted in eclipse, might be that there are reused.

I did this and indeed the markers are reported again after F-E reload. It did not feel as the markers were re-used, as they came-in at the same pace over a minute or two, like when a new workspace is used.

[akosyakov]

@marcdumais-work I've removed storing the state of the problems view. Could you try again? it could be that an issue with an empty problems view was caused by a race between layout restoration and markers from Java LS. If you can reliably reproduce please describe how. I struggle to reproduce it.

[marcdumais-work]

ok, will re-test with latest

[marcdumais-work]

I still have the issue. For me it's very simple to reproduce. Open the Problems view and open a .java file. Wait until the LS is done reporting the issues it found. Then close and re-open the view:

Additionally, I had not noticed before, it looks like when the view is filling-up, we are missing some refresh, that makes it so that we do not see more than the markers for the originally opened file. You can see both in this video:

[akosyakov]

I still have the issue. For me it's very simple to reproduce. Open the Problems view and open a .java file. Wait until the LS is done reporting the issues it found. Then close and re-open the view:

Got you. I was collapsing the view, not closing it. Please pull and try again, it should be fixed. thanks!

Additionally, I had not noticed before, it looks like when the view is filling-up, we are missing some refresh, that makes it so that we do not see more than the markers for the originally opened file. You can see both in this video:

I did it on purpose that the view is not rerendered while Java LS is spamming with diagnostics. In practice after initial diagnostics reported there won't be delays as you type.

[marcdumais-work]

LTGM. Performance with a lot of problem markers from lots of files is amazing with this PR!