Update the project with an empty classpath container on initialize

[laeubi]

Currently there are some cases where an open JavaEditor is not updated, even though the build of the project is fine with that. Strange enough one can make it "work" by simply make some actions slower(!) e.g. setting breakpoints somewhere in the code that initialize classpath containers. The reason is that if initialize happens "too fast" it leads to missing classpath change notification because it looks like the saved to disk state is up-to-date with the new state and no event happens.

This now set an empty intermediate container state to the JDT model so that any code arising at this point in time will see something has changed (from n > empty), and if then at any later time an update is performed from empty > n.

Special thanks and kudos to @merks that debugged the case and lead me on the right track by finding out that PDE is to fast where one would assume its a problem to be to slow when something is not updated.

[iloveeclipse]

Please check this patch for unnecessary rebuilds triggered on consequent startups without changed code.
We don't want such rebuilds, that can be blocker for big workspaces.

[iloveeclipse]

missing classpath change notification because it looks like the saved to disk state is up-to-date with the new state and no event happens

If that is the case (classpath saved is same to classpath set) there shouldn't be any notification, because otherwise we would have unwanted rebuilds. Please don’t merge the patch without extensive testing. For me it seems to be yet another unwanted side effect of #1552.

[laeubi]

If that is the case (classpath saved is same to classpath set) there shouldn't be any notification, because otherwise we would have unwanted rebuilds.

I plan to further improve this to let PDE restore classpath state from saved state (like m2e does) but first priority is to get correct state. I can't tell if its a bug or a feature, but obviously some code in JDT is not recovering correct state and than falls apart. So if you can suggest a patch in JDT that makes it unnecessary that would of course be good, but in the meanwhile we need to make progress and as the actual causing issues in JDT seem not be addressed its only thing PDE can do at the moment.

[github-actions]

Test Results

   765 files  + 13     765 suites  +13   51m 13s ⏱️ + 1m 58s
 3 611 tests ±  0   3 557 ✅ +  1   54 💤 ±0  0 ❌  - 1 
10 834 runs  +194  10 671 ✅ +188  163 💤 +7  0 ❌  - 1 

Results for commit b0d7fdb. ± Comparison against base commit 95e6822.

♻️ This comment has been updated with latest results.

[iloveeclipse]

but first priority is to get correct state

Where it is incorrect? Probably this is missing in the bug description? I'm not aware about opened bugs related ro wrong PDE project state.

What I mean is: have few plugin projects in the workspace. Start Eclipse, do a clean build, shutdown. On next restart no build should be triggered (autobuild would start on startup but the delta should be empty so they will not recompile). To verify that, either start with tracing at compiler (so one can see no class files were written), or check timestamps or simply use bigger projects and check build time.

If sou see now rebuilds with this change, this would be a blocker.

[laeubi]

Where it is incorrect? Probably this is missing in the bug description?

It seems JDT requires for proper function that we set an empty classpath as otherwise it reports NO_DELTA, what does not trigger any events if PDE is too fast later on. So unless there is no "Empty Delta" event there is no chance for components to react to classpath changes (and obviously it makes a difference here).

So if we can fix any of those things it would not need any change in PDE of course.

[iloveeclipse]

It seems JDT requires for proper function that we set an empty classpath

Proper function of what? What is not correct and requires setting intermediate classpath? If you are talking about missing delta in the case where nothing changed, it is expected that there is no delta if there are no classpath changes on the project, otherwise we would have permanent rebuilds.

[laeubi]

Proper function of what?

e.g. Eclipse Java Editor is showing error markers even though there are no errors at all if it is opened and you restart the IDE and the InitClasspathJob takes longer than PDE requires to update the classpath.

[merks]

@iloveeclipse

I've spent the whole morning trying to determine why a delta may or may not be propagated to the editors. Basically the problem is one of timing (event ordering).

If/when this happens:

before this happens

then the former will have asked for the classpath, gotten it (depending on how fast PDE has computed it) and then when the processing for setClasspathContainer kicks in, it finds the old classpath and the new classpath equals and therefore sends no notification.

This is getting the classpath earlier:

With the empty classpath set, those calls don't happen and the subsequent setting of the actual computed classpath produces delta:

That that, the errors remain:

There are many different jobs doing different things in parallel that interact in and change behavior depending on which one finishes first.

In any case, I see no additional builds kicking in as a result of this change.

[iloveeclipse]

Thanks Ed. I will check the details later, but it is good to know that no extra builds are triggered.

[merks]

With this change, and with this print statement:

I see a single call:

Injection.java : setEditorActive=true -> true
TMException.java : setEditorActive=true -> true
IToken.java : setEditorActive=true -> true
ITokenizeLineResult.java : setEditorActive=true -> true
Injection.java : setEditorActive=true -> true
setupClasspath=[org.eclipse.tm4e.core, org.eclipse.tm4e.core.tests, org.eclipse.tm4e.samples, org.eclipse.tm4e.languageconfiguration, org.eclipse.tm4e.language_pack, org.eclipse.tm4e.languageconfiguration.tests, org.eclipse.tm4e.markdown, org.eclipse.tm4e.registry, org.eclipse.tm4e.ui, org.eclipse.tm4e.ui.tests]
TMException.java : setJavaModelChanged=false -> true
IToken.java : setJavaModelChanged=false -> true
ITokenizeLineResult.java : setJavaModelChanged=false -> true
Injection.java : setJavaModelChanged=false -> true
TMException.java : setJavaModelChanged=true -> true
IToken.java : setJavaModelChanged=true -> true
ITokenizeLineResult.java : setJavaModelChanged=true -> true
Injection.java : setJavaModelChanged=true -> true

Otherwise I see a bunch of calls like this:

Injection.java : setEditorActive=true -> true
TMException.java : setEditorActive=true -> true
IToken.java : setEditorActive=true -> true
ITokenizeLineResult.java : setEditorActive=true -> true
Injection.java : setEditorActive=true -> true
setupClasspath=[org.eclipse.tm4e.samples]
setupClasspath=[org.eclipse.tm4e.languageconfiguration]
setupClasspath=[org.eclipse.tm4e.language_pack]
setupClasspath=[org.eclipse.tm4e.languageconfiguration.tests]
setupClasspath=[org.eclipse.tm4e.markdown]
setupClasspath=[org.eclipse.tm4e.registry]
setupClasspath=[org.eclipse.tm4e.core.tests]
setupClasspath=[org.eclipse.tm4e.ui]
setupClasspath=[org.eclipse.tm4e.ui.tests]
setupClasspath=[org.eclipse.tm4e.core]

And then no notifications about a change leading to lingering errors.

[merks]

@iloveeclipse

I have another very simple change pending that also eliminates the transient error annotations in the editor. But that's in this same file on top of this change...

[laeubi]

Thanks @merks again for verification, lets merge and do other improvements on top, as said I plan to further go ahead to cache the last computed state and return it here so we can check for updates in the background entirely.
This then will likely improve initial startup times as currently much time can be spend to resolve the target platform and compute the container just then to see nothing has changed.

[merks]

Yes, just the other day when I was testing restarting the Oomph IDE it ended up resolving the target platform (for whatever reason I don't know because it doesn't normally do that), and that took almost 1.5 minutes to complete. If that were on the main thread on startup, I wouldn't have know what was going on...