Community
Participate
Working Groups
This bugs is now fixed. https://bugs.eclipse.org/bugs/show_bug.cgi?id=246494 See if we still have performance issue using uniqueResourceContents to false. If not.. remove it. Simon
If we are not otherwise incompatible with EMF 2.4 I'd suggest to keep this API for a while. I assume that it does not harm anything else...
The bugs that Ed is fixing right now will do the job only for containments :-(. I'm asking him to put it for Resource.getContent as well. Since we already have our own list I put the following: /** * @since 2.0 */ @Override public boolean contains(Object object) { if (object instanceof InternalEObject) { InternalEObject eObject = (InternalEObject)object; return eObject.eDirectResource() == CDOResourceImpl.this; } return super.contains(object); } and it is working as fast as uniqueCOntent == false. Can we remove that options ?
(In reply to comment #2) > The bugs that Ed is fixing right now will do the job only for containments :-(. > I'm asking him to put it for Resource.getContent as well. Good idea. > Since we already have our own list I put the following: > > /** > * @since 2.0 > */ > @Override > public boolean contains(Object object) > { > if (object instanceof InternalEObject) > { > InternalEObject eObject = (InternalEObject)object; > return eObject.eDirectResource() == CDOResourceImpl.this; > } > return super.contains(object); > } > > and it is working as fast as uniqueCOntent == false. Do you really mean that with this fix all EList methods of Resource.getContents() are always optimized to perform similar to CDOView.setUniqueResourceContents(false)? > > Can we remove that options ? Are you asking if we can *now* remove the two methods from CDOView? Again, I'd say no until at least EMF 2.5 is GA. What about a deprecation like: /** * @see #setUniqueResourceContents(boolean) * @deprecated This performance tweak is not necessary with EMF 2.5 anymore and is likely to be removed when CDO * reaches 2.0. In the meantime it can be safely used. */ @Deprecated public boolean hasUniqueResourceContents(); /** * Specifies whether the contents list of resources will be unique or not. * <p> * This property is transient in that it does not stick with resources outside of the scope of this view. Especially * it will not be persisted with resources in the repository. Each new view will start with <code>true</code> as a * default value. Changing to <code>false</code> will subsequently apply to all resources being loaded or created. * <p> * <b>Note:</b> The resource contents is a containment list and as such <em>must be</em> unique. Setting this property * to <code>false</code> is only recommended for performance optimization when uniqueness is granted by other means. * Violating the uniqueness constraint will result in unpredictable behaviour and possible corruption of the * repository! * * @deprecated This performance tweak is not necessary with EMF 2.5 anymore and is likely to be removed when CDO * reaches 2.0. In the meantime it can be safely used. */ @Deprecated public void setUniqueResourceContents(boolean uniqueResourceContents);
Ed also thought it was a good idea! DOView.setUniqueResourceContents(false) removed the uniqueness. Our optimization keep it... but use the inverse lookup to determine if the item is in the list. Yes I asking if we can remove it now. We should put it deprecated :-)
Then I'm fine with it ;-)
I added deprecated and add the feature in ResourceImpl.ContentList into our list. Should we remove the testcases that use deprecated method ? Or keep it until we remove it completely ?
(In reply to comment #6) > Should we remove the testcases that use deprecated method ? Or keep it until we > remove it completely ? I'd prefer to mark the test cases with @SuppressWarnings and remove them together with the code under test.
Created attachment 112521 [details] Patch v1 - uniqueContent deprecated
Created attachment 112870 [details] Patch v2 Reviewed, minor changes applied, ready to be committed.
Committed to HEAD
Fix available in HEAD: 2.0.0.I200809260911.
Generally available.
