Community
Participate
Working Groups
Created attachment 252140 [details] The used meta model When using QVTo from Luna the mapping below is executed in a few seconds. Using QVTo from Mars-M6 the same mapping takes multiple minutes. mapping inout Parent::addChilds() { var i : Integer := 0; while (i < 50000) { var child := object Child{ name:= i.toString() }; self.childs += child; i := i + 1; } } Almost all CPU time is being spent in the self.childs += child; line. We are using the InternalTransformationExecutor to execute the transformation.
As of bug 449445, all assignments are now carried out in terms of OCL collections for uniformity reasons. If you add 50000 elements, the engine will create 50000 OCL collections, each time you append a new element. I will have a look if we can avoid unnecesary conversions without breaking the uniform handling of assignments. What you could do to speed up the execution is using a mutable collection type temporarily, which avoids creating new collections on append. mapping inout Parent::addChildsNew() { var i : Integer := 0; var list : List(Child) = List{}; while (i < 50000) { var child := object Child{ name:= i.toString() }; list += child; i := i + 1; }; self.childs := list; }
I've done some performance evaluation. In my configuration the current implementation is 3 times slower than the old one (72.000ms now vs 27.000ms in Luna). I've committed some slight optimization which reduces performance downgrade for 30% (with it script performs in 54.000ms). Will be investigated further. Commit id: 6c403a1b00b14cbcec57cf5d8049538e58a65cbb
Beside the detected performance degradation, I see two more problems: In append(...), the collection given to CollectionUtil.createNewCollection(...) is often an EList, which cannot be handled properly and therefore yields a Sequence by default. I'm afraid that this could lead to undesired duplicates. I suggest to change the signature of append(...), such that the CollectionType becomes available. Then createNewCollection(CollectionKind,Collection) can be used to ensure the correct collection type. Furthermore, when resetting a property using :=, old elements can erroneously remain in case of a rejected null. That's because ECollections.setEList is not prepared for the case of rejected values. var c = object EClass {}; var e = object EPackage { eClassifiers := c; }; e.eClassifiers := OrderedSet{object EClass {}, null}; assert fatal (not e.eClassifiers->includes(c));
Further to Bug 463441 discussions. "OCL Collection" = Set/Bag etc (immutable, may contain null) "MOF Collection" = isOrdered/isUnique property value aggregate (mutable, may not contain null) Where x.y is a MOF Collection property: x.y := z requires an OCL Collection to MOF Collection conversion x.y += z is a MOF collection mutation, no OCL collections need be involved So IMHO the unnecessary use of OCL collections for "self.childs += child;" is a costly mistake for this example.
(In reply to Christopher Gerking from comment #3) > Beside the detected performance degradation, I see two more problems: > > In append(...), the collection given to > CollectionUtil.createNewCollection(...) is often an EList, which cannot be > handled properly and therefore yields a Sequence by default. I'm afraid that > this could lead to undesired duplicates. I suggest to change the signature > of append(...), such that the CollectionType becomes available. Then > createNewCollection(CollectionKind,Collection) can be used to ensure the > correct collection type. We can provide our own implementation for createNewCollection(..) in EvaluationUtil. > > Furthermore, when resetting a property using :=, old elements can > erroneously remain in case of a rejected null. That's because > ECollections.setEList is not prepared for the case of rejected values. > > var c = object EClass {}; > > var e = object EPackage { > eClassifiers := c; > }; > > e.eClassifiers := OrderedSet{object EClass {}, null}; > > assert fatal (not e.eClassifiers->includes(c)); Really a bug. Continuation of bug 449445.. Looks like the idea with collection's prefiltering is more appropriate here.
(In reply to Ed Willink from comment #4) > So IMHO the unnecessary use of OCL collections for "self.childs += child;" > is a costly mistake for this example. How would you distinguish between the above cases? At type level, Eclipse OCL (and QVTo) regard both as OCL immutable collections. Therefore, we would need to consider the runtime type of a collection in order to distinguish. We already have instanceof checks to handle the QVTo mutable collection types. Do you think it is appropriate to check for instanceof EList in order to find out whether a Java collection represents an OCL immutable collection or not?
(In reply to Christopher Gerking from comment #6) > How would you distinguish between the above cases? At type level At the metamodel level you should see perhaps PropertyCallExp for one case and VariableExp for the other.
(In reply to Ed Willink from comment #7) > At the metamodel level you should see perhaps PropertyCallExp for one case > and VariableExp for the other. Yes, separation is easy. Luna has a separate handling for the two cases, leading to problems with uncovered corner cases. For Mars we are striving for a common handling of assignments, regardless of whether it affects a property or variable. In particular, I'm looking for a helper method like Object doAssign(Object lhsValue, Object rhsValue, boolean isReset) which returns the result of the assignment. Inside that method, distinguishing between immutable OCL and mutable QVT seems unavoidable. But isn't there a way to avoid yet another special case in terms of mutable MOF collections (EList) ?
(In reply to Christopher Gerking from comment #8) > isn't there a way to avoid yet another special case in terms of mutable MOF > collections (EList) ? Until these discussions I had not appreciated the significant difference between OCL and MOF collections. OCL has no write support for MOF collections so don't look to OCL for a helper. Mutating a MOF collection is a uniquely QVTo requirement, so only QVTo is likely to support it. (QVTc/QVTr might do an intelligent overwrite for an update, but no piecemeal updates.)
