jackrabbit-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ju...@apache.org
Subject svn commit: r1004223 - in /jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core: cache/ persistence/bundle/ persistence/pool/ state/
Date Mon, 04 Oct 2010 13:04:45 GMT
Author: jukka
Date: Mon Oct  4 13:04:45 2010
New Revision: 1004223

URL: http://svn.apache.org/viewvc?rev=1004223&view=rev
Log:
JCR-2699: Improve read/write concurrency

It's better to allow the occasional cache overwrite than to block concurrent threads. The
locking in SessionState and SISM already prevent concurrent read/write operations that could
possibly lead to inconsistencies.

Modified:
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/cache/ConcurrentCache.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/bundle/AbstractBundlePersistenceManager.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/pool/AbstractBundlePersistenceManager.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/LocalItemStateManager.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/SharedItemStateManager.java

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/cache/ConcurrentCache.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/cache/ConcurrentCache.java?rev=1004223&r1=1004222&r2=1004223&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/cache/ConcurrentCache.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/cache/ConcurrentCache.java
Mon Oct  4 13:04:45 2010
@@ -22,9 +22,6 @@ import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
 /**
  * Concurrent cache implementation that uses cache segments to minimize
  * the chance of lock contention. The LRU algorithm is used to evict excess
@@ -34,9 +31,6 @@ import org.slf4j.LoggerFactory;
  */
 public class ConcurrentCache<K, V> extends AbstractCache {
 
-    /** Logger instance */
-    private static Logger log = LoggerFactory.getLogger(ConcurrentCache.class);
-
     private static class E<V> {
 
         private final V value;
@@ -150,16 +144,18 @@ public class ConcurrentCache<K, V> exten
      * @param key entry key
      * @param value entry value
      * @param size entry size
+     * @return the previous value, or <code>null</code>
      */
-    public void put(K key, V value, long size) {
+    public V put(K key, V value, long size) {
         Map<K, E<V>> segment = getSegment(key);
         synchronized (segment) {
             recordSizeChange(size);
             E<V> previous = segment.put(key, new E<V>(value, size));
             if (previous != null) {
-                log.warn("Overwriting cached entry {} from {} to {}",
-                        new Object[] { key, previous.value, value });
                 recordSizeChange(-previous.size);
+                return previous.value;
+            } else {
+                return null;
             }
         }
     }

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/bundle/AbstractBundlePersistenceManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/bundle/AbstractBundlePersistenceManager.java?rev=1004223&r1=1004222&r2=1004223&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/bundle/AbstractBundlePersistenceManager.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/bundle/AbstractBundlePersistenceManager.java
Mon Oct  4 13:04:45 2010
@@ -657,7 +657,6 @@ public abstract class AbstractBundlePers
     private void deleteBundle(NodePropBundle bundle) throws ItemStateException {
         destroyBundle(bundle);
         bundle.removeAllProperties(getBlobStore());
-        bundles.remove(bundle.getId());
         bundles.put(bundle.getId(), MISSING, 16);
     }
 
@@ -675,7 +674,6 @@ public abstract class AbstractBundlePers
         // only put to cache if already exists. this is to ensure proper overwrite
         // and not creating big contention during bulk loads
         if (bundles.containsKey(bundle.getId())) {
-            bundles.remove(bundle.getId());
             bundles.put(bundle.getId(), bundle, bundle.getSize());
         }
     }

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/pool/AbstractBundlePersistenceManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/pool/AbstractBundlePersistenceManager.java?rev=1004223&r1=1004222&r2=1004223&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/pool/AbstractBundlePersistenceManager.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/pool/AbstractBundlePersistenceManager.java
Mon Oct  4 13:04:45 2010
@@ -655,7 +655,6 @@ public abstract class AbstractBundlePers
     private void deleteBundle(NodePropBundle bundle) throws ItemStateException {
         destroyBundle(bundle);
         bundle.removeAllProperties(getBlobStore());
-        bundles.remove(bundle.getId());
         bundles.put(bundle.getId(), MISSING, 16);
     }
 
@@ -673,7 +672,6 @@ public abstract class AbstractBundlePers
         // only put to cache if already exists. this is to ensure proper
         // overwrite and not creating big contention during bulk loads
         if (bundles.containsKey(bundle.getId())) {
-            bundles.remove(bundle.getId());
             bundles.put(bundle.getId(), bundle, bundle.getSize());
         }
     }

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/LocalItemStateManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/LocalItemStateManager.java?rev=1004223&r1=1004222&r2=1004223&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/LocalItemStateManager.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/LocalItemStateManager.java
Mon Oct  4 13:04:45 2010
@@ -163,19 +163,17 @@ public class LocalItemStateManager
             return state;
         }
 
-        // check cache. synchronized to ensure an entry is not created twice.
-        synchronized (this) {
-            state = cache.retrieve(id);
-            if (state == null) {
-                // regular behaviour
-                if (id.denotesNode()) {
-                    state = getNodeState((NodeId) id);
-                } else {
-                    state = getPropertyState((PropertyId) id);
-                }
+        // check cache
+        state = cache.retrieve(id);
+        if (state == null) {
+            // regular behaviour
+            if (id.denotesNode()) {
+                state = getNodeState((NodeId) id);
+            } else {
+                state = getPropertyState((PropertyId) id);
             }
-            return state;
         }
+        return state;
     }
 
     /**

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/SharedItemStateManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/SharedItemStateManager.java?rev=1004223&r1=1004222&r2=1004223&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/SharedItemStateManager.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/SharedItemStateManager.java
Mon Oct  4 13:04:45 2010
@@ -1702,21 +1702,8 @@ public class SharedItemStateManager
             // not found in cache, load from persistent storage
             state = loadItemState(id);
             state.setStatus(ItemState.STATUS_EXISTING);
-            synchronized (this) {
-                // Use a double check to ensure that the cache entry is
-                // not created twice. We don't synchronize the entire
-                // method to allow the first cache retrieval to proceed
-                // even when another thread is loading a new item state.
-                ItemState cachedState = cache.retrieve(id);
-                if (cachedState == null) {
-                    // put it in cache
-                    cache.cache(state);
-                    // set parent container
-                    state.setContainer(this);
-                } else {
-                    state = cachedState;
-                }
-            }
+            state.setContainer(this);
+            cache.cache(state);
         }
         return state;
     }



Mime
View raw message