Enable blackbox resolution against workspace metamodels

[eclipse-qvt-oml-bot]

| --- | --- | | Bugzilla Link | 573449 | | Status | NEW | | Importance | P3 normal | | Reported | May 10, 2021 05:15 EDT | | Modified | Jan 10, 2022 12:05 EDT | | See also | Gerrit change 181515, Gerrit change 181546, Gerrit change 181601, Gerrit change 181600, Gerrit change 181623, Gerrit change 181646, Gerrit change 181645, Gerrit change 181647, Gerrit change 181648, Gerrit change 181692, Git commit 03169de7, Gerrit change 181762, Gerrit change 184991, Gerrit change 185139, Git commit 3bbe0662, Gerrit change 189344, Git commit ec636b11 | | Reporter | Christopher Gerking |

Description

Since bug 472482 has been closed, it is now possible to resolve Java blackboxes within the workspace. What is still missing is the possibility to resolve these blackboxes against metamodels that are also located in the workspace. Such metamodels are not found, so the blackboxes cannot be compiled successfully.

[eclipse-qvt-oml-bot]

By Christopher Gerking on May 10, 2021 09:17

(In reply to Christopher Gerking from comment #0)

Since bug 472482 has been closed

Apparently it was not yet closed.

Anyway, fix proposed on cgerking/573449. It uses EcorePlugin.getEPackageNsURIToGenModelLocationMap(...) to find the corresponding genmodel of a workspace metamodel, and then reads the Java class names from the genmodel.

Since workspace metamodels are dynamic, the corresponding Java blackbox providers must be unloaded and then reloaded. This was not the case before because registered metamodels were not expected to change.

Unfortunately, to read the genmodels, the fix introduces a new plugin dependency on org.eclipse.emf.codegen.ecore. Perhaps this could be avoided by using reflection, especially since this plugin should not be required during standalone execution.

[eclipse-qvt-oml-bot]

By Ed Willink on May 22, 2021 15:42

In reply to Christopher Gerking from Bug 573659

When it comes to bug 573449, I'm still unhappy about the new dependency to org.eclipse.emf.codegen.ecore. Standalone users might not have this plugin on their classpath.

org.eclipse.emf.codegen.ecore is a moderately common ecore plugin and the much less common org.eclipse.emf.ecore.change is already required. (org.eclipse.acceleo.engine has a org.eclipse.emf.codegen.ecore dependency.)

You could make the dependecy optional and catch the Class Load failure exception.

You could use eGet and dynamic EMF to avoid the dependency altogether.

A bit late for M3 on Tuesday but let's do thois for RC1 a week later.

[eclipse-qvt-oml-bot]

By Ed Willink on May 22, 2021 16:33

(In reply to Ed Willink from comment #2)

A bit late for M3 on Tuesday but let's do thois for RC1 a week later.

Opps. Got confused. Pushed to mastter for M3.

[eclipse-qvt-oml-bot]

Jun 07, 2021 08:22

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

[eclipse-qvt-oml-bot]

By Christopher Gerking on Jun 07, 2021 08:24

(In reply to Christopher Gerking from comment #1)

Since workspace metamodels are dynamic, the corresponding Java blackbox providers must be unloaded and then reloaded.

Fixing a minor concurrency issue on cgerking/573449. If the loading is not snchronized, it might happen that two different versions of the same blackbox are loaded.

Ready for RC2.

[eclipse-qvt-oml-bot]

Jun 07, 2021 16:51

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

[eclipse-qvt-oml-bot]

By Christopher Gerking on Jun 07, 2021 16:57

(In reply to Ed Willink from comment #2)

You could make the dependecy optional and catch the Class Load failure exception.

Done on cgerking/573449.

[eclipse-qvt-oml-bot]

Jun 08, 2021 10:02

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

[eclipse-qvt-oml-bot]

Jun 08, 2021 10:02

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

[eclipse-qvt-oml-bot]

Jun 08, 2021 12:36

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

[eclipse-qvt-oml-bot]

By Christopher Gerking on Jun 08, 2021 12:37

Important performance improvements pushed to cgerking/573449.

[eclipse-qvt-oml-bot]

Jun 08, 2021 16:51

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

[eclipse-qvt-oml-bot]

Jun 08, 2021 16:51

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

[eclipse-qvt-oml-bot]

Jun 08, 2021 16:51

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

[eclipse-qvt-oml-bot]

Jun 08, 2021 16:51

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

[eclipse-qvt-oml-bot]

By Christopher Gerking on Jun 08, 2021 18:09

No idea why the latest builds failed. The log reports a problem with org.eclipse.qvto.releng.tycho. Locally everything is still green.

[eclipse-qvt-oml-bot]

By Ed Willink on Jun 09, 2021 03:04

(In reply to Christopher Gerking from comment #16)

No idea why the latest builds failed.

"[cross-project-issues-dev] Problem with repo.eclipse.org and simrel gerrit verification" received "Repo.eclipse was down most of the afternoon https://www.eclipsestatus.io/incidents/9zv3vwwhn0rg" response. Do you subscribe to cross-project-issues-dev?

---

The "Simplify search for GenClassifiers" commit seems to be a useful rather than necessary fix, which if nothing else is a different algorithm with associated bug hazards. Definitely not necessary (or appropriate) as a post RC2 fix.

---

The "Reduce performance overhead for computing GenModel locations" fix seems to just duplicate EMF's static cache with a local non-static cache. For a first access, the new subroutine call offsets the EMF subrotione call overhead. For multiple accesses the new fix seems worse. Definitely not necessary as a post RC2 fix. Maybe I misunderstnad so it could be useful.

[eclipse-qvt-oml-bot]

By Christopher Gerking on Jun 09, 2021 03:46

(In reply to Ed Willink from comment #17)

Do you subscribe to cross-project-issues-dev?

Yes, sorry. I overlooked it.

The "Simplify search for GenClassifiers" commit seems to be a useful rather than necessary fix, which if nothing else is a different algorithm with associated bug hazards. Definitely not necessary (or appropriate) as a post RC2 fix.

Right. It probably doesn't make a big difference.

Unlike the "Make org.eclipse.emf.codegen.ecore dependency optional" commit. This one is important to avoid a new dependency (especially in standalone mode, where org.eclipse.emf.codegen.ecore is not required at all in 2021-03). So this could avoid a breaking change.

The "Reduce performance overhead for computing GenModel locations" fix seems to just duplicate EMF's static cache with a local non-static cache.

Is there actually a static cache? Is seems as if getEPackageNsURIToGenModelLocationMap(...) invokes PDEHelper.computeModels(...) again and again.

This and the other remaining fixes include important performance improvements. I will provide an example project for demonstration, which compiles forever using the current master.

[eclipse-qvt-oml-bot]

By Christopher Gerking on Jun 09, 2021 03:48

Created attachment 286548 Performance demo

Demo project that compiles forever (if org.eclipse.ocl.pivot is in the same workspace)

:compression: bug573449.zip

[eclipse-qvt-oml-bot]

By Ed Willink on Jun 09, 2021 04:35

Compiles forever is definitely not good, but surely there are a finite number of calls, so long time perhaps, but not forever. Forever implies an infinite loop, possibly by inadvertently triggering a rebuild. The loop needs to be identified.

Yes the PDEHelper is repeatedly invoked if there is PDE support, since I suppose detecting target platform changes is too hard. Does QVTo need PDE support?

[eclipse-qvt-oml-bot]

By Christopher Gerking on Jun 09, 2021 04:44

(In reply to Ed Willink from comment #20)

Compiles forever is definitely not good, but surely there are a finite number of calls, so long time perhaps, but not forever.

Right, it's very long not forever. The reasons are that the NsURIToGenModelLocationMap is repeatedly computed and that the same genmodels are reloaded again and again. In both cases, caching does the trick.

Does QVTo need PDE support?

Not necessarily, but org.eclipse.m2m.qvt.oml.runtime.jdt does.

[eclipse-qvt-oml-bot]

By Ed Willink on Jun 09, 2021 05:40

Reviewing your 4 rebased commits to try to do something to day.

---

"Make org.eclipse.emf.codegen.ecore dependency optional" looks harmless enough but have you actually tested it?

IIRC the "instanceof GenClassifier" will be compiled when the surrounding function is compiled and so outside the try catch block. If you want fine grained load detection you must use forName or load. Or wrap the failure in a function such as

if (genModelContent != null) {\ try { ... = resolveGenModelContent(...)\ }\ catch (...)

---

The GenClassifier changes worry me since I see a change to the support GenXXX hierarchy assuming a root GenModel. IIRC once usedGenPackages are involved things get harder. So I am far from convinced that your changes do not exchange rather than fix problems..

---

I would like to see a fix to the 'forever', but I want to see as little else changing as possible.

[eclipse-qvt-oml-bot]

Jun 09, 2021 06:31

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

[eclipse-qvt-oml-bot]

By Christopher Gerking on Jun 09, 2021 06:35

(In reply to Ed Willink from comment #22)

IIRC the "instanceof GenClassifier" will be compiled when the surrounding function is compiled and so outside the try catch block.

I wasn't aware of that.

Or wrap the failure in a function

Prototyped on cgerking/573449.

I see a change to the support GenXXX hierarchy assuming a root GenModel.

Yes, that was my assumption.

So I am far from convinced that your changes do not exchange rather than fix problems..

I see, however the previous code traversing the GenXXX hierarchy was new for 2021-06 as well, so previously there was no GenModel support at all. That's why the change shouldn't cause any regressions.

I would like to see a fix to the 'forever', but I want to see as little else changing as possible.

There is no 'forever', I just exaggerated. But the test project shows me that it can take a very very long time.

[eclipse-qvt-oml-bot]

By Ed Willink on Jun 09, 2021 09:29

How about ewillink/573449 for the 3.10.4 R-build with the rest held over to 3.10.5?

[eclipse-qvt-oml-bot]

By Ed Willink on Jun 09, 2021 09:32

Sorry for the delay. Bugzilla is not notifying again and I didn't notice that my previous reply collided in mid-air.

Yes there is no regression wrt 2021-03, but every change post RC2 has not been exercised by anyone so is really dangerous.

[eclipse-qvt-oml-bot]

Jun 09, 2021 11:49

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

[eclipse-qvt-oml-bot]

Jun 10, 2021 05:51

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

[eclipse-qvt-oml-bot]

By Christopher Gerking on Jun 10, 2021 06:44

(In reply to Ed Willink from comment #25)

How about ewillink/573449 for the 3.10.4 R-build with the rest held over to 3.10.5?

Addressed the GenModel traversal on cgerking/573449 again. Compared to the current aproach of traversing EcoreUtil.getAllContents(...), using GenModel.findGenClassifier(...) seems way more efficient. This is due to internal caching and more intelligent search strategies (scanning only the relevant GenPackage instead of all).

(In reply to Ed Willink from comment #22)

I see a change to the support GenXXX hierarchy assuming a root GenModel.

Is this assumption invalid? Do we need to prepare for a *.genmodel without a GenModel at its root?

[eclipse-qvt-oml-bot]

Sep 05, 2021 08:08

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

[eclipse-qvt-oml-bot]

Sep 08, 2021 05:52

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

[eclipse-qvt-oml-bot]

By Ed Willink on Sep 09, 2021 06:32

Installing cgerking/master that aggregates this fix seems good. I see a couple of momentary "QVTo Analyzing" in the progress view, but almost too quick to see. Excellent.

[eclipse-qvt-oml-bot]

Sep 13, 2021 05:54

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

[eclipse-qvt-oml-bot]

By Christopher Gerking on Sep 13, 2021 06:26

(In reply to Ed Willink from comment #32)

Installing cgerking/master that aggregates this fix seems good.

Pushed to master.

[eclipse-qvt-oml-bot]

By Christopher Gerking on Jan 06, 2022 07:38

(In reply to Christopher Gerking from comment #1)

Since workspace metamodels are dynamic, the corresponding Java blackbox providers must be unloaded and then reloaded.

Made a stupid mistake here because the EPackage.Registry used to control the unload/reload must obviously be local to the individual blackbox units, not to their overarching blackbox provider. Otherwise only the first unit provided by that provider will be reloaded.

This is a careless mistake caused by the nesting of the descriptor as an inner class of the provider. Thus the fPackageRegistry field was just placed inside the wrong class, so move it from the blackbox provider to the blackbox unit.

[eclipse-qvt-oml-bot]

Jan 06, 2022 07:39

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

[eclipse-qvt-oml-bot]

By Ed Willink on Jan 06, 2022 14:04

(In reply to Christopher Gerking from comment #35)

Otherwise only the first unit provided by that provider will be reloaded.

Is this the Bug 577992 use case?

If so, let's have that as a regression test.

[eclipse-qvt-oml-bot]

By Christopher Gerking on Jan 10, 2022 06:53

(In reply to Ed Willink from comment #37)

Is this the Bug 577992 use case?

No, it has nothing to do with it.

If so, let's have that as a regression test.

To make this a regression test, we would need to implement a whole usage scenario in which a blackbox is first compiled, then changed to a different signature, then recompiled. If this happens, and there is more than one blackbox candidate in the respective project, only the (lexicographically) first one will actually reload and reflect the signature change. As you can see, it's quite a corner case that will be hard to encode as an automated test.

[eclipse-qvt-oml-bot]

Jan 10, 2022 12:05

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