Performance regression in profile definition (introduced by correction of bug #544487)

[eclipse-uml2-bot]

| --- | --- | | Bugzilla Link | 546727 | | Status | UNCONFIRMED | | Importance | P3 normal | | Reported | Apr 25, 2019 05:43 EDT | | Modified | May 31, 2019 10:18 EDT | | See also | 544487, Gerrit change https://git.eclipse.org/r/143129 | | Reporter | Alain Le Guennec |

Description

Hi,\ Correction of bug #544487 introduced a performance regression:\ See https://git.eclipse.org/r/#/c/137198/4/plugins/org.eclipse.uml2.uml/src/org/eclipse/uml2/uml/util/UMLUtil.java@5413\ The calls to eAttribute.getEType().getInstanceClass() can have a very significant impact on performance of the conversion, especially for models containing many attributes typed by primitive types such as EBoolean, for which the call to getInstanceClass() will repeatedly return null after a useless but possibly very costly look-up by class loaders (especially in presence of buddy-policy stuff).\ Maybe EClassifierImpl.getInstanceClass() should introduce a cache or a boolean flag indicating that lookup was executed already but failed, so that there is no need to try again?\ Or if EMF cannot be modified to achieve that, maybe the UML2EcoreConverter should itself maintain a map of failed lookups?

[eclipse-uml2-bot]

By Camille Letavernier on Apr 25, 2019 05:59

EBoolean, for which the call to getInstanceClass() will repeatedly return null

That's interesting; I didn't actually expect null values here. All Primitive Types should match a real java type/class at some point in order to be used at runtime.

So maybe getInstanceClass() is simply not the right method to use?

Do you know why the instance class would be null for this specific case? Should we use instanceTypeName/instanceClassName instead?

I only tried this patch with UML Primitive Types and Enums.

[eclipse-uml2-bot]

By Ed Willink on Apr 25, 2019 06:31

(In reply to Camille Letavernier from comment #1)

So maybe getInstanceClass() is simply not the right method to use?

You can use a getInstanceClassName() != null guard around the resolution that attempts to use the type thereby avoiding the class loader costs. You could emulate many routines such as EClassifierImpl.getPrimitiveOrArrayClass that has a hard enumeration of all special Java classes.

(I've hit problems with getInstanceClass before, but can't find the Bugzilla.) Certainly worth avoiding installing the Ecore-to-Java mappings if you don't actually need them. UML2Ecore should be UML-to-Ecore, not UML-to-Ecore-and-Java. But if you rally must do UML-to-Ecore-and-Java, surely you should configure Java correctly so that getInstanceClass() never gets a CNFE?

[eclipse-uml2-bot]

By Camille Letavernier on Apr 25, 2019 07:05

You can use a getInstanceClassName() != null guard around the resolution that attempts to use the type thereby avoiding the class loader costs

If I read the EMF implementation correctly, this test is already included in getInstanceClass()

So I guess that in this case, there is an instanceClassName, that EMF can't resolve to an actual Java class (for some reason). So it would be interesting to understand why this happens in that specific case (I'd expect EBoolean to resolve to either 'boolean' or 'java.lang.Boolean' rather than 'null')

[eclipse-uml2-bot]

By Ed Willink on Apr 25, 2019 07:34

(In reply to Camille Letavernier from comment #3)

If I read the EMF implementation correctly, this test is already included in getInstanceClass()

Very hard, since the relative functionality of generatedInstanceClassName and instanceClassName is far from obvious. A quick debug suggest that generatedInstanceClassName is set by model loading so that while instanceClassName may be null generatedInstanceClassName is not, bypassing the test in getInstanceClass().

It is possible that the way UML2Ecore creates its Ecore is incompatible with the instanceClass protocol, however EMF hasn't changed here since 2006 so bugs should have been found by now.

(I think that some of the complexity derives from GenModel's need to use future class names for which true Java classes have not yet been generated. UML2Ecore should similarly avoid relying on future class names.)

[eclipse-uml2-bot]

By Kenn Hussey on Apr 25, 2019 09:10

EMF uses the instance class name to determine whether something is a (Java) primitive type during code generation - see GenTypedElement#isPrimitiveType() and underlying methods. If the various UML type testing methods that already exist on UMLUtil - e.g., isBoolean(Type), isInteger(Type), isReal(Type), isString(Type), and isUnlimitedNatural(Type) - are insufficient, perhaps a variant of the utilities in the EMF code generator needs to be implemented to avoid the overhead of attempting to obtain the instance class directly.

[eclipse-uml2-bot]

By Camille Letavernier on Apr 25, 2019 09:59

The only thing we do with this instanceClass is to check if it's a java primitive (Or String); so I guess we can test all known primitive type names, rather than relying on Java's Class#isPrimitive.

[eclipse-uml2-bot]

May 31, 2019 10:18

New Gerrit change created: https://git.eclipse.org/r/143129