[impact] NCDFE in AllSubclassesFinder.updateOppositeCache()

[eclipse-ocl-bot]

| --- | --- | | Bugzilla Link | 346368 | | Status | CLOSED FIXED | | Importance | P3 normal | | Reported | May 19, 2011 01:38 EDT | | Modified | May 20, 2011 17:28 EDT | | Version | 3.1.0 | | Reporter | Ed Willink |

Description

As a result of Bug 346272 the erroneous pivot.jar was eliminated from the pivot plugin classpath. 15 impact analyzer tsts now fail on Hudson. This is reproducible in an interactive workspace by running the "ImpactAnalysisTests with NavigationSteps" launch configuration, where the first test is one of the failures.

This is very odd. Running the first test individually or as a subsuite passes.

The failure is on a transitive class lookup, so there must be a concern that something is wrong with the class loader.

The failure occurs in

    for (String packageUri : registryKeys) {\
        try {\
            EPackage ePackage = registry.getEPackage(packageUri);

which causes all packages to be loaded (103 on my machine; many more if Modisco etc also loaded.)

The subsequent catch of Exception, is inadequate to catch the NoClassDefFoundError; needs to be Throwable, since in general some packages may not be loadable and must not abort others.

Changing the catch to Throwable causes the test to run forever; perhaps because the exception traces are huge and killing console buffers (a single trace does not fit into 80000 characters), perhaps because listeners are talking to each other recursively,

so

a) fix the update so that it updates\ b) fix the update call to be less costly\ c) fix the update to break the infinite loop\ d) explain the classpath issue

Why is updateOppositeCache() called 50 recursions deep into getOrCreateNavigationPath? surely the cache should be updated before you start?

[?? wouldn't these tests run faster as standalone tests ??]

[eclipse-ocl-bot]

By Axel Uhl on May 19, 2011 02:14

As you may have noted, the impact analyzer comes with a number of different configurable algorithms that we benchmarked intensely for various situations. After all evaluations, for the standard cases and in almost all situations the TracebackStep variant came out best. NavigationStep performed slightly worse. We still left the NavigationStep code in for interested parties to review and because we still think that if someone spends some time optimizing the navigation step graph layout there may be potential for this variant to perform better than the TracebackStep approach.

One of the weak parts of the algorithm, as you have discovered now, is the need to construct subclass trees in an attempt to prune navigation step graphs early in case certain combinations of type filters can be proven to never match an EObject. However, this assumes (and this may be an assumption too risky) that we can determine full subclass trees ahead of time.

That's the background on why the NavigationStep implementation tries to compute the set of all of a class's subclass which then triggers the load of all packages.

I'm fine with excluding all NavigationStep-based tests from Hudson right now. Shall I remove the respective launch configurations or Adolfo, can you do this (I'm currently traveling and have very unreliable Internet connections unfortunately; back in office on Friday)?

[eclipse-ocl-bot]

By Ed Willink on May 19, 2011 02:22

I'll comment out the junit launch for now.

The 'impossible' classpath failure needs investigating; it may be a platform issue that we need to report. Perhaps the pivot jars really are an issue. Perhaps the transitive depth of the xtext model plugin paths is an issue.

[eclipse-ocl-bot]

By Axel Uhl on May 19, 2011 04:18

The way it looks, the Pivot bundles contain a metamodel that registers with the EPackage registry but doesn't at the same time provide the package class that it registers. Possible? Bug? Maybe the pivot model shouldn't register an extension for the EPackage registry?

[eclipse-ocl-bot]

By Adolfo Sanchez-Barbudo Herrera on May 19, 2011 07:02

BTW,

I've found another class loading problem [1], in the CompleteOcl editor (M7 + Nightly OCL Examples) while trying to open the RoyalAndLoyal.ocl file ... Let's see if the current running build which should fix the plugin, solves the problem...

Axel,

I don't follow your comment, when you say "doesn't provide the package class". The "org.eclipse.ocl.examples.pivot.PivotPackage" does exist (note that there is a emf-gen folder). From the plugin.xml file-> extensions tab -> org.eclipse.emf.ecore.generated_package extension, you can navigate/go through to the corresponding provided package class. Are you referring to a different thing ?

Best Regards,\ Adolfo.

[1] Caused by: java.lang.ClassNotFoundException: org.eclipse.ocl.examples.pivot.utilities.PivotConstants\ at org.eclipse.osgi.internal.loader.BundleLoader.findClassInternal(BundleLoader.java:513)\ at org.eclipse.osgi.internal.loader.BundleLoader.findClass(BundleLoader.java:429)\ at org.eclipse.osgi.internal.loader.BundleLoader.findClass(BundleLoader.java:417)\ at org.eclipse.osgi.internal.baseadaptor.DefaultClassLoader.loadClass(DefaultClassLoader.java:107)\ at java.lang.ClassLoader.loadClass(Unknown Source) ... 119 more

[eclipse-ocl-bot]

By Adolfo Sanchez-Barbudo Herrera on May 19, 2011 08:13

After updating the previous installation with the last build contents (nightly repository) the Complete OCL editor successfully opens the RoyalAndLoyal.ocl file :)

"ImpactAnalysisTests with NavigationSteps" tests still fail... Extra info of my try, the first test case(testAnalysisOfRecursiveOperationWithSelf) throws

java.lang.UnsupportedClassVersionError: Bad version number in .class file

Quite weird...

Regards,\ Adolfo.

[eclipse-ocl-bot]

By Ed Willink on May 19, 2011 13:18

(In reply to comment #5)

java.lang.UnsupportedClassVersionError: Bad version number in .class file

This is/was a different nightmare beyond the usual 1.5/1.6 conflicts. genmodel has an additional compliance level which may be different and which is inconsistently interpreted, particularly in standalone. You can end up doing 1.4 stuff.

As Adolfo observed, the PivotPackage does exist. The 'only' odd things are that the model is UML-derived and consequently/carelessly has a different prefix in "pivot.ecore" and "Pivot.merged.genmodel". In the Model Registry I try to navigate from genmodel to ecore via shared prefix which nearly always works, but I would be surprised if that's part of this issue.

Ahah! I think I've found it. The pivot genmodel has a "Runtime Jars" option set to true, whereas false is recommended and normal.

Initially changing to false makes no difference; perhaps the plug-in version is still causing trouble. Checking other genmodel properties highlights an inefficient resolve containment proxies true. Changing that and the tests work!\ Also works on CVS check out on another machine.

I'll just check the build is happy with the updarted pivot, then re-instate the tests.

[eclipse-ocl-bot]

By Ed Willink on May 19, 2011 14:03

Tests successfully re-instated - at least we flushed out a poor pivot plugin structure.

Sensitivity to bad EPackage registrations is however important. In the past some projects, notably WTP, had some very bad genmodel registrations.

[eclipse-ocl-bot]

By Axel Uhl on May 20, 2011 15:26

Given that the tests run again and that the original problem was an offending JAR, and given that the NavigationStep approach is in only for reference and potential future development, I'll close this bug. If someone is interested, they may open a feature / enhancement request for more intelligent handling of the suclass tree intersection test in case the NavigationStep approach is required / requested by someone.

[eclipse-ocl-bot]

By Ed Willink on May 20, 2011 15:38

(In reply to comment #8)

I'll close this bug.

Please leave a review interval between RESOLVE and CLOSE

This bug has probably two outstanding issues.

a) the malfunction when a java.lang.Error occurs in the registry loop

b) the suspect performance associated with updating very deep in a recusion.

Please do not just CLOSE bugs that have issues outstanding. It is prpbably appropriate to migrate the issues so please open the follow-ons.

[eclipse-ocl-bot]

By Axel Uhl on May 20, 2011 16:39

Adding a patch that fixes the NoClassDefFoundError issue

[eclipse-ocl-bot]

By Axel Uhl on May 20, 2011 16:39

Created attachment 196252 Catch NoClassDefFoundError

See previous discussion.

:notepad_spiral: 346368.patch

[eclipse-ocl-bot]

By Ed Willink on May 20, 2011 17:03

I think you need to just catch Error or more simply Throwable. +1 to commit.

The intent is to just ignore a bad registry entry so many really bad errors such as StackOverflow etc are recoverable.

See Bug 321432 to see how J2EE broke Xtext which therefore broke OCLinEcore.

The EPackage registry scan is very fragile. I think I've seen Modisco fail on it too.

[eclipse-ocl-bot]

By Axel Uhl on May 20, 2011 17:28

changed to catching Throwable; committed to HEAD