Bugzilla – Full Text Bug Listing
| Summary: | Package merge should copy references on receiving package | ||
|---|---|---|---|
| Product: | [Modeling] MDT.UML2 | Reporter: | Kenn Hussey <Kenn.Hussey> |
| Component: | Core | Assignee: | Christian Damus <give.a.damus> |
| Status: | VERIFIED FIXED | QA Contact: | |
| Severity: | enhancement | ||
| Priority: | P2 | CC: | Kenn.Hussey |
| Version: | 2.1.0 | Keywords: | plan |
| Target Milestone: | M3 | Flags: | give.a.damus:
luna+
Kenn.Hussey: review+ |
| Hardware: | PC | ||
| OS: | Windows XP | ||
| Whiteboard: | Community Support | ||
|
Description
Kenn Hussey
The package merge implementation was originally designed/intended to support merging package "increments" into an empty target package. It is too late in the release cycle (e.g. too risky) to revisit the design - we will address this issue as an enhancement in a future release. I have pushed a new branch bugs/176998 (commit 7048f48) with a proposed fix, including JUnit tests.
The PackageMerger adds a new step that scans the merge result for references to elements within the merged packages (i.e., elements that were merged into it) and replaces them with the corresponding merge results. This uses a new mapping of merged elements to their corresponding merge results (implementing the inverse of the merge-results-to-merged-objects map).
The new and existing unit tests pass.
Ad hoc testing of package merge on the UML source model shows that the classic merge scenario in which the receiving package has no references to the merged packages ("increments") is unaffected:
1. Convert the UML.merged.uml to a metamodel using the example action and save as a new resource.
2. Merge UML.uml, convert to a metamodel, and save as another new resource.
3. Textual comparison of the results of steps 1 and 2 shows no differences except the XMI IDs of
stereotype applications (for which the "Convert to Metamodel" action does not provide stable
replacements).
(In reply to comment #2) > This uses a new mapping > of merged elements to their corresponding merge results (implementing the > inverse of the merge-results-to-merged-objects map). How is this different from the intrinsic map that's already built in? The Package Merger is itself a map and already adds entries for each element that is merged (which reference the element it got merged into)... (In reply to Kenn Hussey from comment #3) > > How is this different from the intrinsic map that's already built in? The > Package Merger is itself a map and already adds entries for each element > that is merged (which reference the element it got merged into)... Oh,. right, yeah. The merge is an EcoreUtil.Copier. Seems I missed the forest for the trees ... I should be able just to eliminate the redundant map. I have pushed a new commit 7d958a0 to branch bugs/176998 that greatly simplifies the PackageMerger by removing the totally redundant (yes) mapping of source elements to merge results. Ad hoc and JUnit tests still pass as before. This includes a merge of the latest master branch into the bug branch. Thanks, Christian. I've been stewing on this a bit because I can't help but wonder whether it would be more efficient to cache the list of objects in the receiving package _before_ the copying is done and then iterate through (only) those objects after the references are copied to update their references. As it stands currently, all of the objects that have been merged will be iterated over redundantly whereas potentially only a small number of those objects existed in the receiving package pre-merge (especially in the case where the receiving package is empty)... Thoughts? (In reply to Kenn Hussey from comment #6) > Thanks, Christian. I've been stewing on this a bit because I can't help but > wonder whether it would be more efficient to cache the list of objects in > the receiving package _before_ the copying is done and then iterate through > (only) those objects after the references are copied to update their Well, that would add to the overall memory consumption of the algorithm at the expense of time. Iterating over the resulting package again has linear-time complexity and is considerably less work than the merge was in the first place (how many ephemeral matchers and switches are created over and over again during the merge step for GC to deal with?). I wouldn't be concerned about the time taken by this post-processing step in an already large operation that is run very infrequently. Here are some stats so we can get a clear picture. In a sample of 9 merge runs on the UML metamodel, after warming up with a couple of merges, I measured: - 412 ms total time to merge (std dev 69 ms) - 13.6 ms to fix references (std dev 6.3) - post-processing to fix references amounts to 3.19% of run time (std dev 0.99%) Yeah, you're probably right. Changes have been merged/pushed to the 'master' branch in git and will be available in an integration build early next week. The changes are now available in an integration build for UML2 4.2.0. |