Community
Participate
Working Groups
The e4 model should be loaded and saved used EMF best practices, as described in section 15.1 of the EMF Book, 2nd edition. See also the following comments from the e4 performance bug: bug 373294 comment 3 bug 373294 comment 16 bug 373294 comment 22
Push a batch to Gerrit: https://git.eclipse.org/r/13270 Some very simple performance measures (System.nanoTime() around the model load code) suggest that there is approximately a 60% improvement in load times. On my test machine (Quad-core Core i7, running Win 7/64), I say times go from: Original load times: 1188 ms, average over 10 runs Patch: 421 ms, average over 10 runs
Other notes: I played with calling setIntrinsicIDToEObjectMap(), as suggested in bug 373294 comment 16, but found that this had limited impact on its own (about 40 ms improvement - IMHO, not significant). Furthermore, with XMLResource.OPTION_DEFER_IDREF_RESOLUTION set, the map appeared to go unused. Because it appeared to have no impact, I elected to omit setIntrinsicIDToEObjectMap() use from the patch.
One more comment: The numbers in comment 1 pertain to the time it takes to load the E4 model. The time to load the entire workbench is somewhere in the 5 to 6 second range.
The trace showed that the most significant loading cost was in org.eclipse.emf.ecore.resource.impl.ResourceImpl.getEObjectByID(String). That method should still be called even with XMLResource.OPTION_DEFER_IDREF_RESOLUTION is true, but it should only traverse the whole tree once to build the map. So the assessment in comment 2 appears questionable...
I stand by my statement in comment 2. To back it up, I instrumented ResourceImpl.getEObjectByID(), with the following results: Without the patch: Stats for .../.metadata/.plugins/org.eclipse.e4.workbench/workbench.xmi getEObjectByID(): 1187 calls, 0 cache adds, 0 cache hits getIntrinsicIDToEOjbectMap(): 5158 calls With the patch: Stats for .../.metadata/.plugins/org.eclipse.e4.workbench/workbench.xmi getEObjectByID(): 0 calls, 0 cache adds, 0 cache hits getIntrinsicIDToEOjbectMap(): 3971 calls (cache adds/hits refers to puts and successful gets on the result of getIntrinsicIDToEOjbectMap()). A bit more exploring shows that E4XMIResource extends XMIResourceImpl, which in turn extends XMLResourceImpl. XMLResourceImpl overrides getEObjectByID, and calls the super version only if its own idToEObjectMap has a miss - this doesn't appear to happen when OPTION_DEFER_IDREF_RESOLUTION is set.
I see, so you already do some type of mapped looked so no need for yet another maps. That all makes sense then. So just deferring all ID resolution to the end ensures that the map you're using is well populated and always finds the IDs quickly.
Committed to master/4.4 as: https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=c16a18468e0994317cd42d0f810b1dcb4f8e14a5
Fixed in 4.4
Sorry to intervene like this, but what I see in https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?h=R4_2_maintenance&id=ac86a080766c45beeece8062bbc096b71e13efbc looks a little suspect: the nameToFeatureMap field is defined like this: private final ThreadLocal<Map<Object, Object>> nameToFeatureMap = new ThreadLocal<Map<Object, Object>>(); which creates an empty ThreadLocal container object, with a null value by default. What is set as an option in loadOptions.put(XMLResource.OPTION_USE_XML_NAME_TO_FEATURE_MAP, nameToFeatureMap.get()); would be null, not a thread-local map. I suggest this instead: private final ThreadLocal<Map<Object, Object>> nameToFeatureMap = new ThreadLocal<Map<Object, Object>>() { @Override protected Map<Object, Object> initialValue() { return new HashMap<Object, Object>(); } } Same issue with the lookupTable field.
Pierre-Charles, thanks for the hint. Maybe it is good to open a new bug for this enhancements?
(In reply to Pierre-Charles David from comment #9) > ... snip ... I have confirmed that the ThreadLocal instances need to implement initialValue() in order for non-null values to be inserted into the options map. The interesting thing is that I see NO performance difference when I do this. I'm going to explore a little more...
