Diagnose unresolved proxies

[eclipse-qvt-oml-bot]

| --- | --- | | Bugzilla Link | 533565 | | Status | NEW | | Importance | P3 normal | | Reported | Apr 13, 2018 14:13 EDT | | Modified | Jun 06, 2019 07:27 EDT | | See also | 547990, 547993 | | Reporter | Ed Willink |

Description

https://www.eclipse.org/forums/index.php/mv/msg/1092229/1785423/#msg_1785423 provides a repro whereby transfgormation of a model referencing platform:/plugin/org.eclipse.emf.ecore/model/Ecore.ecore#//EString gets an unresolved proxy and a poor diagnosis.

Surely a resolveAll should be performed on the input models to provide a decent bad-input diagnostic before obscure functionality results?

[eclipse-qvt-oml-bot]

By Ed Willink on Apr 13, 2018 16:08

Calling resolveAll is easy, scanning the results only slightly harder.

Available for review on branch ewillink/533565.

But it imposes a resolveAll cost on execution and now causes transformations that might have succeeded despite an unresolved proxy to fail (?? good / ?? bad). A WrappedException has to be used to throw an exception in the current no-exception API.

[eclipse-qvt-oml-bot]

By Ed Willink on Apr 14, 2018 04:07

Copyright/contributors corrected. Aggregated in branch ewillink/master in preparation for push to master for M7.

[eclipse-qvt-oml-bot]

By Christopher Gerking on Apr 18, 2018 17:07

(In reply to Ed Willink from comment #1)

But it imposes a resolveAll cost on execution and now causes transformations that might have succeeded despite an unresolved proxy to fail (?? good / ?? bad).

(In reply to Sergey Boyko from mail)

Changes impose huge performance penalty. Generally it's not good idea to call EcoreUtil.resolveAll() on the ResourceSet for every model's URI loading.

I'm undecided. There might be transformations that ignore proxies and therefore should not fail to execute on unresolved proxies. How is the problem currently diagnosed?

[eclipse-qvt-oml-bot]

By Ed Willink on Apr 19, 2018 01:44

I consider that a transformation result in the presence of unresolved proxies is very suspect and so arguably should be an outright error.

In practice most proxies will need to be resolved anyway, so resolving them upfront enables an upfront diagnostic rather than an obscure internal failure. The extra upfront resolveAll costs is mitigated by a reduced subsequent execution time.

If we don't resolveAll up front we need to add proxy handling to every model access instead.

[eclipse-qvt-oml-bot]

By Sergey Boyko on Apr 19, 2018 12:40

If I get it right the changes dedicated to this bug were the first attempt to resolve problem mentioned in https://www.eclipse.org/forums/index.php/mv/msg/1092229/1785423/#msg_1785423.

And the correct fix for the problem is to populate correct URI's with the help of EcorePlugin.computePlatformURIMap(..). So addition call to EcoreUtil.resolveAll() does nothing for the mentioned case.

I believe that "Generally it's not good idea to call EcoreUtil.resolveAll() on the ResourceSet for every model's URI loading.". The situation with performance degradation can be achieved when a UML model which references a number of vast UML Profiles.

In QVTo we have validation mechanism with 'quick validation' and 'full validation' options. For now (till dedicated UI option is introduced) we can place EcoreUtil.resolveAll() to 'full validation' conditional branch.

[eclipse-qvt-oml-bot]

By Ed Willink on Apr 19, 2018 13:01

Yes. But there are two sides to the coin.

a) In an ideal world users would only ever provide valid models. But for multi-file models EMF's flexibility provides a large number of opportunities for mistakes. We must therefore provide 'reasonable' validation. This bug.

b) We must not fail to resolve valid URIs. Bug 533564.

I am not advocating a mandatory full WFR validation since I agree that that would be too high a cost and one that the user can do themselves. (It might however be nice to offer a full validation option in the launch config, disabled by default.) The user must accept that garbage-in-garbage-out is their responsibility; WFR validation could easily double execution time.

I am advocating a full EMF load validation since load failures are obscure implementation details that can confuse an apparently valid suite of models. We could make this a distinct launch config checkbox, enabled by default.

Even for UML, I am not convinced that many realistic transformations complete without resolving all profiles; I suspect that UML2 support loads them as soon as they are needed. But a checkbox could disable this resolveAll.

[eclipse-qvt-oml-bot]

By Sergey Boyko on Apr 19, 2018 13:28

(In reply to Ed Willink from comment #6)

Yes. But there are two sides to the coin.

b) We must not fail to resolve valid URIs. Bug 533564.

I am not advocating a mandatory full WFR validation since I agree that that would be too high a cost and one that the user can do themselves. (It might however be nice to offer a full validation option in the launch config, disabled by default.) The user must accept that garbage-in-garbage-out is their responsibility; WFR validation could easily double execution time.

I am advocating a full EMF load validation since load failures are obscure implementation details that can confuse an apparently valid suite of models. We could make this a distinct launch config checkbox, enabled by default.

Even for UML, I am not convinced that many realistic transformations complete without resolving all profiles; I suspect that UML2 support loads them as soon as they are needed. But a checkbox could disable this resolveAll.

Eclipse (and Java) is highly utilizing "lazy loading" idiom. This is the way how Eclipse starts up and so on. Nothing should be loaded/computed in advance, only on demand. EMF model can be backed by quite "heavy" resources (database for example). Thus resolving all cross-refernces by means of resolveAll() contradicts conventional behavior.

[eclipse-qvt-oml-bot]

By Ed Willink on May 22, 2018 03:57

The Bug 533564 fix solves the problem that leads to unresolved proxies.

Invoking resoveAll proactively is indeed debateable and has not yet been instrumented. Catching all proxies throughout the code is tedious and error prone. But there may be another way. Once the transformation completes, the ResourceSet.resources.errors can be traversed to detect how many unresolved proxies actually occurred during the transfoprmation. These could be reported and probably should result in a failure even if the transformation appears to succeed.

RC1 is a bit late to start failing 'working' transformations, so deferring till after Photon. Proxy diagnosis is a courtesy not a necessity.

[eclipse-qvt-oml-bot]

By Ed Willink on Jun 06, 2019 06:45

(In reply to Ed Willink from comment #6)

I am advocating a full EMF load validation since load failures are obscure implementation details that can confuse an apparently valid suite of models. We could make this a distinct launch config checkbox, enabled by default.

I have just hit this problem in QVTr where, when transforming an Ecore model, a match for EDataType only finds EDataTypes that have been loaded; no match to the reference to

eType="ecore:EDataType http://www.eclipse.org/emf/2002/Ecore#//EString"

occurs since it is a proxy that is not part of the input model/extent. Naively QVTr executes as for-all-instances do-something-sensible-instance-wise, so all instances must be considered transitively to avoid a partial transformation.

I'm not convinced that QVTo with its imperative schedule is genuinely different. The imperative schedule certainly offers an opportunity for lazy loading of what the schedule needs, but this requires the lazy loading to be accurately triggered in many places.

Indeed. Bug 547990 raised complete with repro project.