[YANGTOOLS-948] Encapsulate ModificationApplyOperation.getChild() overrides Created: 31/Jan/19 Updated: 13/Feb/19 Resolved: 13/Feb/19 |
|
| Status: | Resolved |
| Project: | yangtools |
| Component/s: | data-impl |
| Affects Version/s: | None |
| Fix Version/s: | None |
| Type: | Improvement | Priority: | Medium |
| Reporter: | Robert Varga | Assignee: | Unassigned |
| Resolution: | Won't Do | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||
| Epic Link: | unlabeled-YANGTOOLS-940 | ||||||||
| Description |
|
We currently have seven distinct implementations of this method, which really come down to following behaviors:
Options 3-5 are only accessible through SchemaAwareApplyOperation, hence we should be to make the method bimorphic and delegate internally to a utility class. |
| Comments |
| Comment by Robert Varga [ 06/Feb/19 ] |
|
The key problem with this is that 'UnsupportedOperationException' is used by leaf nodes, hence moving this to a field value would increase our memory overhead significantly. In-memory layout of SchemaAwareApplyOperation needs to be looked at to evaluate the impact. Maybe that behavior can be set as the default, with others overriding it? Item 3 is used by invisible nodes (lists), but LeafSets are value-based and hence they our outside of the class hierarchy of MapNodes. Item 4 and 5 look very much alike, so could probably be unified at the cost of paying a volatile deref in ChoiceModificationStrategy by using a completely pre-computed mapping. The cost of doing this is moving the backing field and acquire/store into a common subclass. |
| Comment by Robert Varga [ 13/Feb/19 ] |
|
I don't think the added indirection is worth it, unless we can we end up having a cozy place in SchemaAwareApplyOperation on which we can piggy-back this behavior. |