[391320] Weakly cache and intern() client-side CDOCommitInfos

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

4 files changed, 126 insertions, 20 deletions

diff --git a/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/internal/common/commit/CDOCommitInfoImpl.java b/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/internal/common/commit/CDOCommitInfoImpl.java
index 86bc97bc45..b3c73bf8ac 100644
--- a/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/internal/common/commit/CDOCommitInfoImpl.java
+++ b/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/internal/common/commit/CDOCommitInfoImpl.java
@@ -130,6 +130,45 @@ public class CDOCommitInfoImpl extends CDOBranchPointImpl implements CDOCommitIn
   }
 
   @Override
+  public int hashCode()
+  {
+    long timeStamp = getTimeStamp();
+
+    final int prime = 31;
+    int result = 0;
+    result = prime * result + (commitInfoManager == null ? 0 : commitInfoManager.hashCode());
+    result = prime * result + (int)(timeStamp ^ timeStamp >>> 32);
+    return result;
+  }
+
+  @Override
+  public boolean equals(Object obj)
+  {
+    if (this == obj)
+    {
+      return true;
+    }
+
+    if (!(obj instanceof CDOCommitInfoImpl))
+    {
+      return false;
+    }
+
+    CDOCommitInfoImpl other = (CDOCommitInfoImpl)obj;
+    if (commitInfoManager != other.commitInfoManager)
+    {
+      return false;
+    }
+
+    if (getTimeStamp() != other.getTimeStamp())
+    {
+      return false;
+    }
+
+    return true;
+  }
+
+  @Override
   public String toString()
   {
     String data = null;
@@ -144,7 +183,7 @@ public class CDOCommitInfoImpl extends CDOBranchPointImpl implements CDOCommitIn
             "CommitInfo[{0}, {1}, {2}, {3}, {4}, {5}]", getPreviousTimeStamp(), getBranch(), timeStamp, getUserID(), getComment(), data); //$NON-NLS-1$
   }
 
-  private void loadCommitDataIfNeeded()
+  private synchronized void loadCommitDataIfNeeded()
   {
     if (commitData == null)
     {
diff --git a/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/internal/common/commit/CDOCommitInfoManagerImpl.java b/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/internal/common/commit/CDOCommitInfoManagerImpl.java
index 4002bd7bb0..9b703b3d5a 100644
--- a/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/internal/common/commit/CDOCommitInfoManagerImpl.java
+++ b/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/internal/common/commit/CDOCommitInfoManagerImpl.java
@@ -22,6 +22,8 @@ import org.eclipse.emf.cdo.spi.common.commit.InternalCDOCommitInfoManager;
 
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Map;
+import java.util.WeakHashMap;
 
 /**
  * @author Andre Dietisheim
@@ -29,30 +31,42 @@ import java.util.List;
 public class CDOCommitInfoManagerImpl extends CDOCommitHistoryProviderImpl<CDOBranch, CDOCommitHistory> implements
     InternalCDOCommitInfoManager
 {
-  private CommitInfoLoader commitInfoLoader;
+  private final Map<CDOCommitInfo, CDOCommitInfo> cache;
 
-  private List<CDOCommitInfoHandler> commitInfoHandlers = new ArrayList<CDOCommitInfoHandler>();
+  private final Object cacheLock = new Object();
 
-  public CDOCommitInfoManagerImpl()
+  private CommitInfoLoader loader;
+
+  private List<CDOCommitInfoHandler> handlers = new ArrayList<CDOCommitInfoHandler>();
+
+  public CDOCommitInfoManagerImpl(boolean caching)
   {
+    if (caching)
+    {
+      cache = new WeakHashMap<CDOCommitInfo, CDOCommitInfo>();
+    }
+    else
+    {
+      cache = null;
+    }
   }
 
   public CommitInfoLoader getCommitInfoLoader()
   {
-    return commitInfoLoader;
+    return loader;
   }
 
   public void setCommitInfoLoader(CommitInfoLoader commitInfoLoader)
   {
     checkInactive();
-    this.commitInfoLoader = commitInfoLoader;
+    loader = commitInfoLoader;
   }
 
   public CDOCommitInfoHandler[] getCommitInfoHandlers()
   {
-    synchronized (commitInfoHandlers)
+    synchronized (handlers)
     {
-      return commitInfoHandlers.toArray(new CDOCommitInfoHandler[commitInfoHandlers.size()]);
+      return handlers.toArray(new CDOCommitInfoHandler[handlers.size()]);
     }
   }
 
@@ -61,9 +75,9 @@ public class CDOCommitInfoManagerImpl extends CDOCommitHistoryProviderImpl<CDOBr
    */
   public void addCommitInfoHandler(CDOCommitInfoHandler handler)
   {
-    synchronized (commitInfoHandlers)
+    synchronized (handlers)
     {
-      commitInfoHandlers.add(handler);
+      handlers.add(handler);
     }
   }
 
@@ -72,9 +86,9 @@ public class CDOCommitInfoManagerImpl extends CDOCommitHistoryProviderImpl<CDOBr
    */
   public void removeCommitInfoHandler(CDOCommitInfoHandler handler)
   {
-    synchronized (commitInfoHandlers)
+    synchronized (handlers)
     {
-      commitInfoHandlers.remove(handler);
+      handlers.remove(handler);
     }
   }
 
@@ -97,7 +111,9 @@ public class CDOCommitInfoManagerImpl extends CDOCommitHistoryProviderImpl<CDOBr
       String comment, CDOCommitData commitData)
   {
     checkActive();
-    return new CDOCommitInfoImpl(this, branch, timeStamp, previousTimeStamp, userID, comment, commitData);
+    CDOCommitInfo commitInfo = new CDOCommitInfoImpl(this, branch, timeStamp, previousTimeStamp, userID, comment,
+        commitData);
+    return intern(commitInfo);
   }
 
   public CDOCommitInfo getCommitInfo(long timeStamp)
@@ -118,28 +134,52 @@ public class CDOCommitInfoManagerImpl extends CDOCommitHistoryProviderImpl<CDOBr
   public void getCommitInfos(CDOBranch branch, long startTime, long endTime, CDOCommitInfoHandler handler)
   {
     checkActive();
-    commitInfoLoader.loadCommitInfos(branch, startTime, endTime, handler);
+    if (cache != null)
+    {
+      final CDOCommitInfoHandler delegate = handler;
+      handler = new CDOCommitInfoHandler()
+      {
+        public void handleCommitInfo(CDOCommitInfo commitInfo)
+        {
+          delegate.handleCommitInfo(intern(commitInfo));
+        }
+      };
+    }
+
+    loader.loadCommitInfos(branch, startTime, endTime, handler);
   }
 
   public void getCommitInfos(CDOBranch branch, long startTime, String userID, String comment, int count,
       CDOCommitInfoHandler handler)
   {
-    checkActive();
-
     if (userID != null || comment != null)
     {
       throw new IllegalArgumentException("The parameters userID and comment are not supported");
     }
 
     long endTime = CDOCommitInfoUtil.encodeCount(count);
-    commitInfoLoader.loadCommitInfos(branch, startTime, endTime, handler);
+    getCommitInfos(branch, startTime, endTime, handler);
   }
 
   @Override
   protected void doBeforeActivate() throws Exception
   {
     super.doBeforeActivate();
-    checkState(commitInfoLoader, "commitInfoLoader"); //$NON-NLS-1$
+    checkState(loader, "commitInfoLoader"); //$NON-NLS-1$
+  }
+
+  @Override
+  protected void doDeactivate() throws Exception
+  {
+    if (cache != null)
+    {
+      synchronized (cacheLock)
+      {
+        cache.clear();
+      }
+    }
+
+    super.doDeactivate();
   }
 
   @Override
@@ -147,4 +187,23 @@ public class CDOCommitInfoManagerImpl extends CDOCommitHistoryProviderImpl<CDOBr
   {
     return new CDOCommitHistoryImpl(this, key);
   }
+
+  private CDOCommitInfo intern(CDOCommitInfo commitInfo)
+  {
+    if (cache != null && commitInfo != null)
+    {
+      synchronized (cacheLock)
+      {
+        CDOCommitInfo cachedCommitInfo = cache.get(commitInfo);
+        if (cachedCommitInfo != null)
+        {
+          return cachedCommitInfo;
+        }
+
+        cache.put(commitInfo, commitInfo);
+      }
+    }
+
+    return commitInfo;
+  }
 }
diff --git a/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/spi/common/commit/CDOCommitInfoUtil.java b/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/spi/common/commit/CDOCommitInfoUtil.java
index b78f4891f1..20a0b7f489 100644
--- a/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/spi/common/commit/CDOCommitInfoUtil.java
+++ b/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/spi/common/commit/CDOCommitInfoUtil.java
@@ -39,9 +39,17 @@ public final class CDOCommitInfoUtil
   {
   }
 
+  /**
+   * @since 4.2
+   */
+  public static InternalCDOCommitInfoManager createCommitInfoManager(boolean caching)
+  {
+    return new CDOCommitInfoManagerImpl(caching);
+  }
+
   public static InternalCDOCommitInfoManager createCommitInfoManager()
   {
-    return new CDOCommitInfoManagerImpl();
+    return createCommitInfoManager(false);
   }
 
   /**
diff --git a/plugins/org.eclipse.emf.cdo.net4j/src/org/eclipse/emf/cdo/internal/net4j/CDONet4jSessionImpl.java b/plugins/org.eclipse.emf.cdo.net4j/src/org/eclipse/emf/cdo/internal/net4j/CDONet4jSessionImpl.java
index aaeaee8f78..127a0aad46 100644
--- a/plugins/org.eclipse.emf.cdo.net4j/src/org/eclipse/emf/cdo/internal/net4j/CDONet4jSessionImpl.java
+++ b/plugins/org.eclipse.emf.cdo.net4j/src/org/eclipse/emf/cdo/internal/net4j/CDONet4jSessionImpl.java
@@ -169,7 +169,7 @@ public class CDONet4jSessionImpl extends CDOSessionImpl implements org.eclipse.e
     InternalCDOCommitInfoManager commitInfoManager = getCommitInfoManager();
     if (commitInfoManager == null)
     {
-      commitInfoManager = CDOCommitInfoUtil.createCommitInfoManager();
+      commitInfoManager = CDOCommitInfoUtil.createCommitInfoManager(true);
       setCommitInfoManager(commitInfoManager);
     }