[473804] Undo of massive deletion very long

https://bugs.eclipse.org/bugs/show_bug.cgi?id=473804

7 files changed, 247 insertions, 46 deletions

diff --git a/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/performance/DeletionUndoPerformanceTests.java b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/performance/DeletionUndoPerformanceTests.java
new file mode 100644
index 0000000000..447236431b
--- /dev/null
+++ b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/performance/DeletionUndoPerformanceTests.java
@@ -0,0 +1,131 @@
+/*
+ * Copyright (c) 2016 Eike Stepper (Berlin, Germany) and others.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ *    Eike Stepper - initial API and implementation
+ */
+package org.eclipse.emf.cdo.tests.performance;
+
+import org.eclipse.emf.cdo.eresource.CDOResource;
+import org.eclipse.emf.cdo.session.CDOSession;
+import org.eclipse.emf.cdo.tests.AbstractCDOTest;
+import org.eclipse.emf.cdo.tests.config.impl.ConfigTest.CleanRepositoriesBefore;
+import org.eclipse.emf.cdo.tests.model3.NodeA;
+import org.eclipse.emf.cdo.transaction.CDOTransaction;
+
+import org.eclipse.emf.common.command.Command;
+import org.eclipse.emf.common.command.CommandStack;
+import org.eclipse.emf.common.util.URI;
+import org.eclipse.emf.ecore.resource.Resource;
+import org.eclipse.emf.ecore.resource.impl.ResourceSetImpl;
+import org.eclipse.emf.ecore.xmi.impl.XMIResourceFactoryImpl;
+import org.eclipse.emf.edit.command.DeleteCommand;
+import org.eclipse.emf.edit.domain.AdapterFactoryEditingDomain;
+import org.eclipse.emf.edit.provider.ComposedAdapterFactory;
+import org.eclipse.emf.edit.provider.ReflectiveItemProviderAdapterFactory;
+import org.eclipse.emf.transaction.TransactionalEditingDomain;
+
+import java.util.Collections;
+
+/**
+ * Tests performance difference on deletion undo between a XMI and CDO.
+ *
+ * @author Eike Stepper
+ */
+@CleanRepositoriesBefore(reason = "To not be disturbed by other tests")
+public class DeletionUndoPerformanceTests extends AbstractCDOTest
+{
+  private static final int CHILDREN_COUNT = 3;
+
+  private static final int DEPTH = 9;
+
+  public void testDeletionUndoWithXMI() throws Exception
+  {
+    ResourceSetImpl resourceSet = new ResourceSetImpl();
+    resourceSet.getResourceFactoryRegistry().getExtensionToFactoryMap().put("model1", new XMIResourceFactoryImpl());
+
+    URI uri = URI.createFileURI(createTempFile("test1", ".model1").getCanonicalPath());
+    Resource resource = resourceSet.createResource(uri);
+
+    populate(resource, CHILDREN_COUNT, DEPTH);
+    testDeletionUndo(resource);
+  }
+
+  public void testDeletionUndoWithCDO() throws Exception
+  {
+    CDOSession session = openSession();
+    CDOTransaction transaction = session.openTransaction();
+    CDOResource resource = transaction.createResource(getResourcePath("test1.model1"));
+
+    populate(resource, CHILDREN_COUNT, DEPTH);
+    testDeletionUndo(resource);
+  }
+
+  private void populate(Resource resource, long childrenCount, long depth) throws Exception
+  {
+    NodeA root = getModel3Factory().createNodeA();
+    root.setName("" + 1);
+    resource.getContents().add(root);
+
+    populate(root, childrenCount, depth);
+    resource.save(Collections.emptyMap());
+
+    long elementCount = (long)((1L - Math.pow(childrenCount, depth + 1)) / (1L - childrenCount));
+    System.out.println("Populated " + elementCount + " elements");
+  }
+
+  private void populate(NodeA root, long childrenCount, long depth) throws Exception
+  {
+    if (depth > 0)
+    {
+      for (long i = 0; i < childrenCount; i++)
+      {
+        NodeA child = getModel3Factory().createNodeA();
+        child.setName("" + (Integer.valueOf(root.getName()) + 1 + i));
+        populate(child, childrenCount, depth - 1);
+        root.getChildren().add(child);
+      }
+    }
+  }
+
+  private void testDeletionUndo(Resource resource)
+  {
+    NodeA rootNodeA = (NodeA)resource.getContents().get(0);
+
+    TransactionalEditingDomain domain = TransactionalEditingDomain.Factory.INSTANCE
+        .createEditingDomain(resource.getResourceSet());
+
+    ((ComposedAdapterFactory)((AdapterFactoryEditingDomain)domain).getAdapterFactory())
+        .addAdapterFactory(new ReflectiveItemProviderAdapterFactory());
+
+    CommandStack commandStack = domain.getCommandStack();
+
+    for (int i = 0; i < 5; i++)
+    {
+      Command cmd = null;
+      for (NodeA nodeA : rootNodeA.getChildren())
+      {
+        Command deleteCmd = DeleteCommand.create(domain, nodeA);
+        if (cmd != null)
+        {
+          cmd = cmd.chain(deleteCmd);
+        }
+        else
+        {
+          cmd = deleteCmd;
+        }
+      }
+
+      commandStack.execute(cmd);
+
+      long start = System.currentTimeMillis();
+      commandStack.undo();
+      long duration = System.currentTimeMillis() - start;
+      System.err.println("Duration: " + duration);
+    }
+  }
+}
diff --git a/plugins/org.eclipse.emf.cdo.workspace/META-INF/MANIFEST.MF b/plugins/org.eclipse.emf.cdo.workspace/META-INF/MANIFEST.MF
index c4a1c956ee..ffe6c887f0 100644
--- a/plugins/org.eclipse.emf.cdo.workspace/META-INF/MANIFEST.MF
+++ b/plugins/org.eclipse.emf.cdo.workspace/META-INF/MANIFEST.MF
@@ -1,7 +1,7 @@
 Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-SymbolicName: org.eclipse.emf.cdo.workspace;singleton:=true
-Bundle-Version: 4.2.100.qualifier
+Bundle-Version: 4.2.200.qualifier
 Bundle-Name: %pluginName
 Bundle-Vendor: %providerName
 Bundle-Localization: plugin
@@ -13,7 +13,7 @@ Require-Bundle: org.eclipse.core.runtime;bundle-version="[3.5.0,4.0.0)";resoluti
  org.eclipse.emf.cdo.server.net4j;bundle-version="[4.0.0,5.0.0)",
  org.eclipse.emf.cdo.net4j;bundle-version="[4.0.0,5.0.0)",
  org.eclipse.net4j.jvm;bundle-version="[4.0.0,5.0.0)"
-Export-Package: org.eclipse.emf.cdo.internal.workspace;version="4.2.100";x-friends:="org.eclipse.emf.cdo.tests,org.eclipse.emf.cdo.tests.db,org.eclipse.emf.cdo.ui.workspace",
- org.eclipse.emf.cdo.internal.workspace.bundle;version="4.2.100";x-internal:=true,
- org.eclipse.emf.cdo.spi.workspace;version="4.2.100",
- org.eclipse.emf.cdo.workspace;version="4.2.100"
+Export-Package: org.eclipse.emf.cdo.internal.workspace;version="4.2.200";x-friends:="org.eclipse.emf.cdo.tests,org.eclipse.emf.cdo.tests.db,org.eclipse.emf.cdo.ui.workspace",
+ org.eclipse.emf.cdo.internal.workspace.bundle;version="4.2.200";x-internal:=true,
+ org.eclipse.emf.cdo.spi.workspace;version="4.2.200",
+ org.eclipse.emf.cdo.workspace;version="4.2.200"
diff --git a/plugins/org.eclipse.emf.cdo.workspace/src/org/eclipse/emf/cdo/internal/workspace/CDOWorkspaceImpl.java b/plugins/org.eclipse.emf.cdo.workspace/src/org/eclipse/emf/cdo/internal/workspace/CDOWorkspaceImpl.java
index c62293d35f..4742f804a4 100644
--- a/plugins/org.eclipse.emf.cdo.workspace/src/org/eclipse/emf/cdo/internal/workspace/CDOWorkspaceImpl.java
+++ b/plugins/org.eclipse.emf.cdo.workspace/src/org/eclipse/emf/cdo/internal/workspace/CDOWorkspaceImpl.java
@@ -101,6 +101,7 @@ import org.eclipse.emf.ecore.EClass;
 import org.eclipse.emf.ecore.resource.ResourceSet;
 import org.eclipse.emf.spi.cdo.CDOSessionProtocol.RefreshSessionResult;
 import org.eclipse.emf.spi.cdo.DefaultCDOMerger;
+import org.eclipse.emf.spi.cdo.InternalCDOSavepoint;
 import org.eclipse.emf.spi.cdo.InternalCDOSession;
 import org.eclipse.emf.spi.cdo.InternalCDOSessionConfiguration;
 import org.eclipse.emf.spi.cdo.InternalCDOTransaction;
@@ -454,16 +455,14 @@ public class CDOWorkspaceImpl extends Notifier implements InternalCDOWorkspace
         Set<CDOID> changedIDs = new HashSet<CDOID>();
 
         InternalCDOTransaction tx = (InternalCDOTransaction)transaction;
-        Set<CDOID> dirtyObjects = tx.getDirtyObjects().keySet();
-        Set<CDOID> detachedObjects = tx.getDetachedObjects().keySet();
+        InternalCDOSavepoint lastSavepoint = tx.getLastSavepoint();
 
         for (InternalCDORevision revision : tx.getCleanRevisions().values())
         {
           CDOID id = revision.getID();
           changedIDs.add(id);
 
-          boolean isDetached = detachedObjects.contains(id);
-          if (isDetached)
+          if (lastSavepoint.getDetachedObject(id) != null)
           {
             if (base.isAddedObject(id))
             {
@@ -474,7 +473,7 @@ public class CDOWorkspaceImpl extends Notifier implements InternalCDOWorkspace
               base.registerChangedOrDetachedObject(revision);
             }
           }
-          else if (dirtyObjects.contains(id))
+          else if (lastSavepoint.getDirtyObject(id) != null)
           {
             base.registerChangedOrDetachedObject(revision);
           }
diff --git a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/query/CDOQueryResultIteratorImpl.java b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/query/CDOQueryResultIteratorImpl.java
index 5fb85f0998..1d1f65c55a 100644
--- a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/query/CDOQueryResultIteratorImpl.java
+++ b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/query/CDOQueryResultIteratorImpl.java
@@ -14,39 +14,29 @@ package org.eclipse.emf.internal.cdo.query;
 import org.eclipse.emf.cdo.CDOObject;
 import org.eclipse.emf.cdo.common.id.CDOID;
 import org.eclipse.emf.cdo.common.util.CDOQueryInfo;
-import org.eclipse.emf.cdo.transaction.CDOTransaction;
 import org.eclipse.emf.cdo.util.CDOUtil;
 import org.eclipse.emf.cdo.util.ObjectNotFoundException;
 import org.eclipse.emf.cdo.view.CDOView;
 
 import org.eclipse.emf.common.util.EList;
 import org.eclipse.emf.spi.cdo.AbstractQueryIterator;
+import org.eclipse.emf.spi.cdo.InternalCDOTransaction;
 
 import java.util.AbstractList;
 import java.util.ArrayList;
 import java.util.List;
-import java.util.Map;
 
 /**
  * @author Simon McDuff
  */
 public class CDOQueryResultIteratorImpl<T> extends AbstractQueryIterator<T>
 {
-  private Map<CDOID, CDOObject> detachedObjects;
-
   public CDOQueryResultIteratorImpl(CDOView view, CDOQueryInfo queryInfo)
   {
     super(view, queryInfo);
   }
 
   @Override
-  public void close()
-  {
-    detachedObjects = null;
-    super.close();
-  }
-
-  @Override
   public T next()
   {
     return adapt(super.next());
@@ -72,15 +62,10 @@ public class CDOQueryResultIteratorImpl<T> extends AbstractQueryIterator<T>
       }
       catch (ObjectNotFoundException ex)
       {
-        if (view instanceof CDOTransaction)
+        if (view instanceof InternalCDOTransaction)
         {
-          if (detachedObjects == null)
-          {
-            CDOTransaction transaction = (CDOTransaction)view;
-            detachedObjects = transaction.getDetachedObjects();
-          }
-
-          CDOObject cdoObject = detachedObjects.get(id);
+          InternalCDOTransaction transaction = (InternalCDOTransaction)view;
+          CDOObject cdoObject = transaction.getLastSavepoint().getDetachedObject(id);
           return (T)CDOUtil.getEObject(cdoObject);
         }
 
@@ -137,7 +122,7 @@ public class CDOQueryResultIteratorImpl<T> extends AbstractQueryIterator<T>
   /**
    * @author Simon McDuff
    */
-  private class QueryResultList extends AbstractList<T>implements EList<T>
+  private class QueryResultList extends AbstractList<T> implements EList<T>
   {
     private List<Object> objects;
 
diff --git a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/transaction/CDOSavepointImpl.java b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/transaction/CDOSavepointImpl.java
index 199b396eed..31b60e514a 100644
--- a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/transaction/CDOSavepointImpl.java
+++ b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/transaction/CDOSavepointImpl.java
@@ -569,7 +569,7 @@ public class CDOSavepointImpl extends CDOUserSavepointImpl implements InternalCD
 
       try
       {
-        if (getPreviousSavepoint() == null && reattachedObjects.isEmpty())
+        if (getPreviousSavepoint() == null && getReattachedObjects().isEmpty())
         {
           return Collections.unmodifiableMap(getDetachedObjects());
         }
@@ -588,7 +588,8 @@ public class CDOSavepointImpl extends CDOUserSavepointImpl implements InternalCD
             }
           }
 
-          for (CDOID reattachedID : savepoint.getReattachedObjects().keySet())
+          Map<CDOID, CDOObject> reattachedObjects = savepoint.getReattachedObjects();
+          for (CDOID reattachedID : reattachedObjects.keySet())
           {
             detachedObjects.remove(reattachedID);
           }
@@ -668,6 +669,76 @@ public class CDOSavepointImpl extends CDOUserSavepointImpl implements InternalCD
   // return isNew;
   // }
 
+  public CDOObject getDetachedObject(CDOID id)
+  {
+    synchronized (transaction.getViewMonitor())
+    {
+      transaction.lockView();
+
+      try
+      {
+        for (InternalCDOSavepoint savepoint = this; savepoint != null; savepoint = savepoint.getPreviousSavepoint())
+        {
+          Map<CDOID, CDOObject> reattachedObjects = savepoint.getReattachedObjects();
+          if (!reattachedObjects.isEmpty())
+          {
+            CDOObject object = reattachedObjects.get(id);
+            if (object != null)
+            {
+              return null;
+            }
+          }
+
+          Map<CDOID, CDOObject> detachedObjects = savepoint.getDetachedObjects();
+          if (!detachedObjects.isEmpty())
+          {
+            CDOObject object = detachedObjects.get(id);
+            if (object != null)
+            {
+              return object;
+            }
+          }
+        }
+      }
+      finally
+      {
+        transaction.unlockView();
+      }
+    }
+
+    return null;
+  }
+
+  public CDOObject getDirtyObject(CDOID id)
+  {
+    synchronized (transaction.getViewMonitor())
+    {
+      transaction.lockView();
+
+      try
+      {
+        for (InternalCDOSavepoint savepoint = this; savepoint != null; savepoint = savepoint.getPreviousSavepoint())
+        {
+          Map<CDOID, CDOObject> dirtyObjects = savepoint.getDirtyObjects();
+          if (!dirtyObjects.isEmpty())
+          {
+            CDOObject object = dirtyObjects.get(id);
+            if (object != null)
+            {
+              return object;
+            }
+          }
+        }
+      }
+      finally
+      {
+        transaction.unlockView();
+      }
+    }
+
+    return null;
+  }
+
   public void rollback()
   {
     synchronized (transaction.getViewMonitor())
diff --git a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/transaction/CDOTransactionImpl.java b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/transaction/CDOTransactionImpl.java
index 3d9ddd3f46..c6719a057a 100644
--- a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/transaction/CDOTransactionImpl.java
+++ b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/transaction/CDOTransactionImpl.java
@@ -1444,7 +1444,9 @@ public class CDOTransactionImpl extends CDOViewImpl implements InternalCDOTransa
         }
 
         CDOID id = super.getRootOrTopLevelResourceNodeID(name);
-        if (getLastSavepoint().getAllDetachedObjects().containsKey(id) || getDirtyObjects().containsKey(id))
+
+        InternalCDOSavepoint lastSavepoint = getLastSavepoint();
+        if (lastSavepoint.getDetachedObject(id) != null || lastSavepoint.getDirtyObject(id) != null)
         {
           throw new CDOException(MessageFormat.format(Messages.getString("CDOTransactionImpl.1"), name)); //$NON-NLS-1$
         }
@@ -2495,7 +2497,7 @@ public class CDOTransactionImpl extends CDOViewImpl implements InternalCDOTransa
   private static List<EPackage> analyzeNewPackages(Collection<EPackage> usedTopLevelPackages,
       CDOPackageRegistry packageRegistry)
   {
-    // Determine which of the corresdonding EPackages are new
+    // Determine which of the corresponding EPackages are new
     List<EPackage> newPackages = new ArrayList<EPackage>();
 
     IPackageClosure closure = new CompletePackageClosure();
@@ -2984,8 +2986,6 @@ public class CDOTransactionImpl extends CDOViewImpl implements InternalCDOTransa
       try
       {
         CDOID id = super.getID(object, onlyPersistedID);
-
-        // If super returned a good result, return immediately
         if (id != null)
         {
           return id;
@@ -2995,21 +2995,25 @@ public class CDOTransactionImpl extends CDOViewImpl implements InternalCDOTransa
         // indirectly through provideCDOID. This occurs when deltas or revisions are
         // being written out to a stream; in which case null must be returned (for transients) so that
         // the caller will detect a dangling reference
-        if (providingCDOID.get())
+        if (providingCDOID.get() == Boolean.TRUE)
         {
           return null;
         }
 
-        // The super implementation will return null for a transient (unattached) object;
-        // but in a tx, an transient object may previously have been attached. So we consult
-        // the cleanRevisions if that's the case.
-        CDORevisionKey revKey = cleanRevisions.get(object);
-        if (revKey != null && getDetachedObjects().containsValue(object))
+        // The super implementation returns null for a transient (unattached) object;
+        // but in a transaction, a transient object may have been attached previously.
+        // So we consult the cleanRevisions if that's the case.
+        CDORevisionKey revisionKey = cleanRevisions.get(object);
+        if (revisionKey != null)
         {
-          id = revKey.getID();
+          CDOID revisionID = revisionKey.getID();
+          if (lastSavepoint.getDetachedObject(revisionID) != null)
+          {
+            return revisionID;
+          }
         }
 
-        return id;
+        return null;
       }
       finally
       {
@@ -3029,12 +3033,12 @@ public class CDOTransactionImpl extends CDOViewImpl implements InternalCDOTransa
       {
         try
         {
-          providingCDOID.set(true);
+          providingCDOID.set(Boolean.TRUE);
           return super.provideCDOID(idOrObject);
         }
         finally
         {
-          providingCDOID.set(false);
+          providingCDOID.remove();
         }
       }
       finally
diff --git a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/spi/cdo/InternalCDOSavepoint.java b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/spi/cdo/InternalCDOSavepoint.java
index 3e1469c605..64002bd50e 100644
--- a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/spi/cdo/InternalCDOSavepoint.java
+++ b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/spi/cdo/InternalCDOSavepoint.java
@@ -10,6 +10,7 @@
  */
 package org.eclipse.emf.spi.cdo;
 
+import org.eclipse.emf.cdo.CDOObject;
 import org.eclipse.emf.cdo.common.id.CDOID;
 import org.eclipse.emf.cdo.transaction.CDOSavepoint;
 
@@ -45,4 +46,14 @@ public interface InternalCDOSavepoint extends CDOSavepoint, InternalCDOUserSavep
    * @since 4.1
    */
   public boolean isNewObject(CDOID id);
+
+  /**
+   * @since 4.6
+   */
+  public CDOObject getDirtyObject(CDOID id);
+
+  /**
+   * @since 4.6
+   */
+  public CDOObject getDetachedObject(CDOID id);
 }