Community
Participate
Working Groups
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.
(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.
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.
(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.
New Gerrit change created: https://git.eclipse.org/r/c/mmt/org.eclipse.qvto/+/181515
(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.
New Gerrit change created: https://git.eclipse.org/r/c/mmt/org.eclipse.qvto/+/181546
(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.
New Gerrit change created: https://git.eclipse.org/r/c/mmt/org.eclipse.qvto/+/181601
New Gerrit change created: https://git.eclipse.org/r/c/mmt/org.eclipse.qvto/+/181600
New Gerrit change created: https://git.eclipse.org/r/c/mmt/org.eclipse.qvto/+/181623
Important performance improvements pushed to cgerking/573449.
New Gerrit change created: https://git.eclipse.org/r/c/mmt/org.eclipse.qvto/+/181646
New Gerrit change created: https://git.eclipse.org/r/c/mmt/org.eclipse.qvto/+/181645
New Gerrit change created: https://git.eclipse.org/r/c/mmt/org.eclipse.qvto/+/181647
New Gerrit change created: https://git.eclipse.org/r/c/mmt/org.eclipse.qvto/+/181648
No idea why the latest builds failed. The log reports a problem with org.eclipse.qvto.releng.tycho. Locally everything is still green.
(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.
(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.
Created attachment 286548 [details] Performance demo Demo project that compiles forever (if org.eclipse.ocl.pivot is in the same workspace)
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?
(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.
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.
New Gerrit change created: https://git.eclipse.org/r/c/mmt/org.eclipse.qvto/+/181692
(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.
How about ewillink/573449 for the 3.10.4 R-build with the rest held over to 3.10.5?
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.
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
New Gerrit change created: https://git.eclipse.org/r/c/mmt/org.eclipse.qvto/+/181762
(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?
New Gerrit change created: https://git.eclipse.org/r/c/mmt/org.eclipse.qvto/+/184991
New Gerrit change created: https://git.eclipse.org/r/c/mmt/org.eclipse.qvto/+/185139
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.
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
(In reply to Ed Willink from comment #32) > Installing cgerking/master that aggregates this fix seems good. Pushed to master.
(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.
New Gerrit change created: https://git.eclipse.org/r/c/mmt/org.eclipse.qvto/+/189344
(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.
(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.
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
