Bugzilla – Full Text Bug Listing
| Summary: | Provide smart cross reference iterator in CDOTransactionImpl.removeCrossReferences method | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | [Modeling] EMF | Reporter: | Egidijus Vaisnora <vaisegid> | ||||||
| Component: | cdo.core | Assignee: | Egidijus Vaisnora <vaisegid> | ||||||
| Status: | CLOSED FIXED | QA Contact: | Eike Stepper <stepper> | ||||||
| Severity: | enhancement | ||||||||
| Priority: | P3 | CC: | caspar_d, saulius.tvarijonas | ||||||
| Version: | 4.1 | Flags: | stepper:
review+
|
||||||
| Target Milestone: | --- | ||||||||
| Hardware: | All | ||||||||
| OS: | All | ||||||||
| Whiteboard: | Lighter, Faster and Better | ||||||||
| Attachments: |
|
||||||||
|
Description
Egidijus Vaisnora
Created attachment 203663 [details]
Patch v1
(In reply to comment #0) > [...] could make undesired calculation and performance loss. Can you please explain what/where exactly this calculation/loss is and in which cases it "could" happen? I'm worried that the creation of a new list each time getCrossReferencesSkipDerived() is called is not optimal, either. If it's just about the two preceding lines: CDOID referencedOID = CDOUtil.getCDOObject(referencedObject).cdoID(); if (referencedOIDs.contains(referencedOID)) then what about just moving the isDerived() check more towards the beginning of removeCrossReferences()? More generally, would calling CDOClassInfo.getAllPersistentFeatures() be more straight forward and faster, as that is really the array of features that controls the values array in a CDORevision. Note that there is additional logic required to determine whether a feature is part of a revision, see EMFUtil.isPersistent(EStructuralFeature). (In reply to comment #2) > Can you please explain what/where exactly this calculation/loss is and in which > cases it "could" happen? "derived" means that this value is computed from other related data and this computation depends on implementation, which could be expensive. Especially it is noted with our UML model, where we have plenty of derived features, which in implementation tries to create subsets or joins from the existing values, searching in whole hierarchy. With previous implementation eCrossReferences collects all data from *all* eReferences and later filters out elements from the derived features. It is correct, however computation on all derived features is executed in this case, even if they are not needed in method logic. > I'm worried that the creation of a new list each time > getCrossReferencesSkipDerived() is called is not optimal, either. New object creation in current JVM is as expensive as malloc in C. I am not sure if it is target for optimization now, but if you see a way how we could make this, with less object creation and do not bring more complexity into code - we can try to make it more optimal. > If it's just about the two preceding lines: > CDOID referencedOID = CDOUtil.getCDOObject(referencedObject).cdoID(); > if (referencedOIDs.contains(referencedOID)) > > then what about just moving the isDerived() check more towards the beginning of > removeCrossReferences()? I do not understand your proposal. Maybe you could post sample code or patch? > More generally, would calling CDOClassInfo.getAllPersistentFeatures() be more > straight forward and faster, as that is really the array of features that > controls the values array in a CDORevision. Note that there is additional logic > required to determine whether a feature is part of a revision, see > EMFUtil.isPersistent(EStructuralFeature). It is not good. This will omit transient features. Created attachment 203883 [details]
Patch v2
A little refactored.
Committed revision 9226, trunk Closing. |