eclipse-qvto / org.eclipse.qvto

Eclipse Public License 2.0
0 stars 1 forks source link

Workspace blackboxes cannot resolve classes from bundles #1091

Open eclipse-qvt-oml-bot opened 1 week ago

eclipse-qvt-oml-bot commented 1 week ago

| --- | --- | | Bugzilla Link | 573752 | | Status | NEW | | Importance | P3 normal | | Reported | May 25, 2021 05:28 EDT | | Modified | Jun 07, 2021 16:13 EDT | | See also | Gerrit change 180968, Gerrit change 181158, Gerrit change 181512, Gerrit change 181513, Git commit 5544a0c3, Git commit b467ff0a, Gerrit change 181541 | | Reporter | Christopher Gerking |

Description

Another shortcoming of bug 472482 is that classes from bundles are not correctly recognized as blackbox classes, so that blackboxes based on these classes cannot be resolved successfully. This is because ProjectClassLoader will reload all classes instead of reusing the classes already loaded by the bundles' class loaders. Obviously, the reloaded classes are different from those obtained from the corresponding EPackage.Registry, which causes the bug.

Why does the problem not occur during the tests? Well, because these are based on Ecore. Since QVTo depends on EMF, the Ecore classes can be loaded by ProjectClassLoader.class.getClassLoader(), which is used as a parent class loader. However this does not work for arbitrary user-defined metamodels.

A possible solution would be to use the class loaders of required bundles as a composite parent loader. But this is likely to cause the same performance bottlenecks that we have already experienced for bug 571406. Is there another way to create a class loader against a given EPackage.Registry?

eclipse-qvt-oml-bot commented 1 week ago

By Ed Willink on May 25, 2021 05:53

Standalone, everything is on the one classpath so loading is not an issue except that dynamically created classes may be too-late and so need a custom loader that defers back to the standard.

Within Eclipse, each plugin has its own class loader seeing only its own classes. It is therefore necessary for shared tooling to use an outer/user/test class rather than the inner classpath of e.g. org.eclipse.emf.ecore. The Resource's class loader is often a good choice since the Resource was created by the user/test harness. Sometimes it is necessary to allow the outer code to pass in a class loader.

I'm not sure that I really understand the problem. A good/bad example might help.

eclipse-qvt-oml-bot commented 1 week ago

May 25, 2021 06:32

New Gerrit change created: https://git.eclipse.org/r/c/mmt/org.eclipse.qvto/+/180968

eclipse-qvt-oml-bot commented 1 week ago

By Christopher Gerking on May 25, 2021 06:39

(In reply to Christopher Gerking from comment #0)

A possible solution would be to use the class loaders of required bundles as a composite parent loader.

Prototyped on cgerking/573752. As expected, it drastically prolongs the initial compilation of projects like org.eclipse.ocl.examples.build.

(In reply to Ed Willink from comment #1)

I'm not sure that I really understand the problem. A good/bad example might help.

Will provide an example.

eclipse-qvt-oml-bot commented 1 week ago

By Christopher Gerking on May 25, 2021 08:05

Created attachment 286446 Example QVTo project

The example transformation is based on Pivot. Provided that org.eclipse.ocl.pivot is installed (but not in the workspace), bug573752.qvto should compile without any warning. This does not work with the current master, showing a warning because the blackbox cannot be resolved. It works using the patch from cgerking/573752 (but takes forever to compile on other projects like org.eclipse.ocl.examples.build).

:compression: bug573752.zip

eclipse-qvt-oml-bot commented 1 week ago

May 30, 2021 07:35

New Gerrit change created: https://git.eclipse.org/r/c/mmt/org.eclipse.qvto/+/181158

eclipse-qvt-oml-bot commented 1 week ago

By Christopher Gerking on May 30, 2021 08:05

(In reply to Christopher Gerking from comment #4)

...but takes forever to compile on other projects like org.eclipse.ocl.examples.build.

Using Bundle.loadClass(...) seems to be much faster, which is what I did now on cgerking/573752 by calling CommonPlugin.loadClass(...). Test case still to be added.

eclipse-qvt-oml-bot commented 1 week ago

By Christopher Gerking on May 30, 2021 08:58

(In reply to Christopher Gerking from comment #6)

Using Bundle.loadClass(...) seems to be much faster

Of course it's not faster, the compilation time just depends on how many required bundles are workspace projects. The more required workspace projects, the more classes will be loaded. So this doesn't yet solve the performance problem.

eclipse-qvt-oml-bot commented 1 week ago

Jun 07, 2021 07:50

New Gerrit change created: https://git.eclipse.org/r/c/mmt/org.eclipse.qvto/+/181512

eclipse-qvt-oml-bot commented 1 week ago

Jun 07, 2021 07:50

New Gerrit change created: https://git.eclipse.org/r/c/mmt/org.eclipse.qvto/+/181513

eclipse-qvt-oml-bot commented 1 week ago

By Christopher Gerking on Jun 07, 2021 07:59

Test case and fix pushed to cgerking/573752. The class loader now handles workspace classes only. Otherwise, it delegates to the bundle class loaders of the required plugins.

eclipse-qvt-oml-bot commented 1 week ago

By Ed Willink on Jun 07, 2021 08:01

re you confident enough for this to go in RC2 tomorrow?

eclipse-qvt-oml-bot commented 1 week ago

By Christopher Gerking on Jun 07, 2021 08:08

(In reply to Ed Willink from comment #11)

re you confident enough for this to go in RC2 tomorrow?

Yes in general, but could you try compiling org.eclipse.ocl.examples.build? I have tried it successfully, without any major performance loss. But I'm not 100% sure about the impact of the local setup.

eclipse-qvt-oml-bot commented 1 week ago

By Ed Willink on Jun 07, 2021 09:08

(In reply to Christopher Gerking from comment #12)

(In reply to Ed Willink from comment #11)

re you confident enough for this to go in RC2 tomorrow? Yes in general

I see no commits newer than 30-May. Looks like you haven't pushed you branches.

eclipse-qvt-oml-bot commented 1 week ago

By Christopher Gerking on Jun 07, 2021 09:32

(In reply to Ed Willink from comment #13)

I see no commits newer than 30-May. Looks like you haven't pushed you branches.

Done, sorry.

eclipse-qvt-oml-bot commented 1 week ago

By Ed Willink on Jun 07, 2021 13:14

(In reply to Christopher Gerking from comment #12)

but could you try compiling org.eclipse.ocl.examples.build?

The branch-tests job has a build of the three branches which I installed.

The main QVTo tx for OCL Pivot seems ok.

The blackbox build time is a bit mixed. Never truly dreadful, but not exactly fast, and clearly lacking any dependency awareness. (Saving a Java file in org.eclipse.ocl.pivot that is on the org.eclipse.ocl.examples.build classpath but not on the BlackBoxLibrary.qvto import path triggers a re-analysis. (Perhaps 5 seconds when one might hope for much less normally and zero for an irrelevant increment.)

There are opportunities for improvement but no reason to hold back from RC2.

eclipse-qvt-oml-bot commented 1 week ago

Jun 07, 2021 14:26

Gerrit change https://git.eclipse.org/r/c/mmt/org.eclipse.qvto/+/181513 was merged to [master].\ Commit: http://git.eclipse.org/c/mmt/org.eclipse.qvto.git/commit/?id=5544a0c3499c3c4ed9b0180e13add59673660001

eclipse-qvt-oml-bot commented 1 week ago

Jun 07, 2021 14:26

Gerrit change https://git.eclipse.org/r/c/mmt/org.eclipse.qvto/+/181512 was merged to [master].\ Commit: http://git.eclipse.org/c/mmt/org.eclipse.qvto.git/commit/?id=b467ff0ac154b485b90f71a2b6e99bcc0869418f

eclipse-qvt-oml-bot commented 1 week ago

Jun 07, 2021 15:53

New Gerrit change created: https://git.eclipse.org/r/c/mmt/org.eclipse.qvto/+/181541

eclipse-qvt-oml-bot commented 1 week ago

By Christopher Gerking on Jun 07, 2021 16:13

Removed a wrong check that prevents classes from being loaded from required plugins. This was a relic that made sense before I changed the class loading delegation, but is now wrong and should be removed.