You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jackrabbit.apache.org by ju...@apache.org on 2010/10/04 12:12:36 UTC

svn commit: r1004182 - in /jackrabbit/trunk/jackrabbit-core/src: main/java/org/apache/jackrabbit/core/cache/ main/java/org/apache/jackrabbit/core/persistence/bundle/ main/java/org/apache/jackrabbit/core/persistence/pool/ main/java/org/apache/jackrabbit...

Author: jukka
Date: Mon Oct  4 10:12:35 2010
New Revision: 1004182

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

Even the tiny synchronized block in the LRU cache becomes a source of lock contention, so replace it with a segmented cache that has now single synchronization block over the entire cache. The downside is a slight deviation from the LRU eviction policy.

Replaced the BundleCache and LRUNodeIdCache classes with the new ConcurrentCache implementation. Instead of using a separate data structure, a special MISSING sentinel bundle is used to to mark non-existent bundles.

Added:
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/cache/AbstractCache.java   (with props)
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/cache/ConcurrentCache.java   (with props)
    jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/cache/
    jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/cache/ConcurrentCacheTest.java   (with props)
Removed:
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/util/BundleCache.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/util/LRUNodeIdCache.java
Modified:
    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/ItemStateCache.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/ItemStateReferenceCache.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/MLRUItemStateCache.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/ManagedMLRUItemStateCacheFactory.java

Added: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/cache/AbstractCache.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/cache/AbstractCache.java?rev=1004182&view=auto
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/cache/AbstractCache.java (added)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/cache/AbstractCache.java Mon Oct  4 10:12:35 2010
@@ -0,0 +1,145 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.jackrabbit.core.cache;
+
+import static org.apache.jackrabbit.core.cache.CacheAccessListener.ACCESS_INTERVAL;
+
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.atomic.AtomicLong;
+import java.util.concurrent.atomic.AtomicReference;
+
+/**
+ * Abstract base class for managed {@link Cache}s. This class uses atomic
+ * variables to track the current and maximum size of the cache, the cache
+ * access count and a possible {@link CacheAccessListener} instance.
+ * <p>
+ * A subclass should call the protected {@link #recordCacheAccess()} method
+ * whenever the cache is accessed (even cache misses should be reported).
+ * The subclass should also use the {@link #recordSizeChange(long)} method
+ * to record all changes in the cache size, and automatically evict excess
+ * items when the {@link #isTooBig()} method returns <code>true</code>.
+ */
+public abstract class AbstractCache implements Cache {
+
+    /**
+     * The estimated amount of memory currently used by this cache. The
+     * current value is returned by the {@link #getMemoryUsed()} method
+     * and can be updated by a subclass using the protected
+     * {@link #recordSizeChange(long)} method.
+     */
+    private final AtomicLong memoryUsed = new AtomicLong();
+
+    /**
+     * The allocated maximum size of this cache. A {@link CacheManager} uses
+     * the {@link #getMaxMemorySize()} and {@link #setMaxMemorySize(long)}
+     * methods to control the target size of a cache. Subclasses can use the
+     * protected {@link #isTooBig()} method to determine whether the current
+     * size of the cache exceeds this size limit.
+     */
+    private final AtomicLong maxMemorySize = new AtomicLong();
+
+    /**
+     * Cache access counter. Used to fire periodic
+     * {@link CacheAccessListener#cacheAccessed()} events once every
+     * {@link CacheAccessListener#ACCESS_INTERVAL} calls to the protected
+     * {@link #recordCacheAccess()} method.
+     */
+    private final AtomicInteger accessCount = new AtomicInteger();
+
+    /**
+     * Cache access listener. Set in the
+     * {@link #setAccessListener(CacheAccessListener)} method and accessed
+     * by periodically by the {@link #recordCacheAccess()} method.
+     */
+    private final AtomicReference<CacheAccessListener> accessListener =
+        new AtomicReference<CacheAccessListener>();
+
+    /**
+     * Checks whether the current estimate of the amount of memory used
+     * by this cache exceeds the allocated maximum amount of memory.
+     *
+     * @return <code>true</code> if the cache size is too big,
+     *         <code>false</code> otherwise
+     */
+    protected boolean isTooBig() {
+        return memoryUsed.get() > maxMemorySize.get();
+    }
+
+    /**
+     * Updates the current memory use estimate of this cache.
+     *
+     * @param delta number of bytes added or removed
+     */
+    protected void recordSizeChange(long delta) {
+        memoryUsed.addAndGet(delta); // ignore the return value
+    }
+
+    /**
+     * Records a single cache access and calls the configured
+     * {@link CacheAccessListener} (if any) whenever the constant access
+     * interval has passed since the previous listener call.
+     */
+    protected void recordCacheAccess() {
+        int count = accessCount.incrementAndGet();
+        if (count % ACCESS_INTERVAL == 0) {
+            CacheAccessListener listener = accessListener.get();
+            if (listener != null) {
+                listener.cacheAccessed();
+            }
+        }
+    }
+
+    public long getAccessCount() {
+        return accessCount.get();
+    }
+
+    public void resetAccessCount() {
+        accessCount.set(0);
+    }
+
+    public long getMemoryUsed() {
+        return memoryUsed.get();
+    }
+
+    public long getMaxMemorySize() {
+        return maxMemorySize.get();
+    }
+
+    public void setMaxMemorySize(long size) {
+        maxMemorySize.set(size);
+    }
+
+    /**
+     * Set the cache access listener. Only one listener per cache is supported.
+     *
+     * @param listener the new listener
+     */
+    public void setAccessListener(CacheAccessListener listener) {
+        accessListener.set(listener);
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    public void dispose() {
+        CacheAccessListener listener = accessListener.get();
+        if (listener != null) {
+            listener.disposeCache(this);
+        }
+    }
+
+}

Propchange: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/cache/AbstractCache.java
------------------------------------------------------------------------------
    svn:eol-style = native

Added: 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=1004182&view=auto
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/cache/ConcurrentCache.java (added)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/cache/ConcurrentCache.java Mon Oct  4 10:12:35 2010
@@ -0,0 +1,236 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.jackrabbit.core.cache;
+
+import java.util.ArrayList;
+import java.util.Iterator;
+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
+ * entries from each cache segment separately, which makes the combined
+ * eviction algorithm similar but not exactly the same as LRU. None of the
+ * methods of this class are synchronized, but they are all thread-safe.
+ */
+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;
+
+        private final long size;
+
+        public E(V value, long size) {
+            this.value = value;
+            this.size = size;
+        }
+
+    }
+
+    private final Map<K, E<V>>[] segments;
+
+    @SuppressWarnings({ "unchecked", "serial" })
+    public ConcurrentCache(int numberOfSegments) {
+        this.segments = new Map[numberOfSegments];
+        for (int i = 0; i < segments.length; i++) {
+            segments[i] = new LinkedHashMap<K, E<V>>(1024, 0.75f, true) {
+                @Override
+                protected boolean removeEldestEntry(Map.Entry<K, E<V>> eldest) {
+                    if (isTooBig()) {
+                        recordSizeChange(-eldest.getValue().size);
+                        return true;
+                    } else {
+                        return false;
+                    }
+                }
+            };
+        }
+    }
+
+    public ConcurrentCache() {
+        this(Runtime.getRuntime().availableProcessors());
+    }
+
+    /**
+     * Returns the cache segment for the given entry key. The segment is
+     * selected based on the hash code of the key, after a transformation
+     * to prevent interfering with the optimal performance of the segment
+     * hash map.
+     *
+     * @param key entry key
+     * @return cache segment
+     */
+    private Map<K, E<V>> getSegment(K key) {
+        // Unsigned shift right to prevent negative indexes and to
+        // prevent too similar keys to all get stored in the same segment
+        return segments[(key.hashCode() >>> 1) % segments.length];
+    }
+
+    /**
+     * Checks if the identified entry is cached.
+     *
+     * @param key entry key
+     * @return <code>true</code> if the entry is cached,
+     *         <code>false</code> otherwise
+     */
+    public boolean containsKey(K key) {
+        Map<K, E<V>> segment = getSegment(key);
+        synchronized (segment) {
+            return segment.containsKey(key);
+        }
+    }
+
+    /**
+     * Returns the identified cache entry.
+     *
+     * @param key entry key
+     * @return entry value, or <code>null</code> if not found
+     */
+    public V get(K key) {
+        recordCacheAccess();
+
+        Map<K, E<V>> segment = getSegment(key);
+        synchronized (segment) {
+            E<V> entry = segment.get(key);
+            if (entry != null) {
+                return entry.value;
+            } else {
+                return null;
+            }
+        }
+    }
+
+    /**
+     * Returns all values in the cache. Note that this method is not
+     * synchronized over the entire cache, so it is only guaranteed to
+     * return accurate results when there are no concurrent threads modifying
+     * the cache.
+     *
+     * @return cached values
+     */
+    @SuppressWarnings("unchecked")
+    public V[] values() {
+        List<V> values = new ArrayList<V>();
+        for (int i = 0; i < segments.length; i++) {
+            synchronized (segments[i]) {
+                for (E<V> entry : segments[i].values()) {
+                    values.add(entry.value);
+                }
+            }
+        }
+        return (V[]) values.toArray();
+    }
+
+    /**
+     * Adds the given entry to the cache.
+     *
+     * @param key entry key
+     * @param value entry value
+     * @param size entry size
+     */
+    public void 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);
+            }
+        }
+    }
+
+    /**
+     * Removes the identified entry from the cache.
+     *
+     * @param key entry key
+     * @return removed entry, or <code>null</code> if not found
+     */
+    public V remove(K key) {
+        Map<K, E<V>> segment = getSegment(key);
+        synchronized (segment) {
+            E<V> entry = segment.remove(key);
+            if (entry != null) {
+                recordSizeChange(-entry.size);
+                return entry.value;
+            } else {
+                return null;
+            }
+        }
+    }
+
+    /**
+     * Clears all segments of the cache. Note that even this method is not
+     * synchronized over the entire cache, so it needs to explicitly count
+     * the cache size changes and may return with a non-empty cache if
+     * other threads have concurrently been adding new entries.
+     */
+    public void clear() {
+        for (int i = 0; i < segments.length; i++) {
+            synchronized (segments[i]) {
+                for (E<V> entry : segments[i].values()) {
+                    recordSizeChange(-entry.size);
+                }
+                segments[i].clear();
+            }
+        }
+    }
+
+    /**
+     * Checks if the cache size is zore.
+     */
+    public boolean isEmpty() {
+        return getMemoryUsed() == 0;
+    }
+
+    /**
+     * Sets the maximum size of the cache and evicts any excess items until
+     * the current size falls within the given limit.
+     */
+    @Override
+    public void setMaxMemorySize(long size) {
+        super.setMaxMemorySize(size);
+
+        // Semi-random start index to prevent bias against the first segments
+        int start = (int) getAccessCount() % segments.length;
+        for (int i = start; isTooBig(); i = (i + 1) % segments.length) {
+            synchronized (segments[i]) {
+                Iterator<Map.Entry<K, E<V>>> iterator =
+                    segments[i].entrySet().iterator();
+                if (iterator.hasNext()) {
+                    Map.Entry<K, E<V>> entry = iterator.next();
+                    // Removing and re-adding the first entry will
+                    // automatically the last entry if the cache is
+                    // too big
+                    segments[i].remove(entry.getKey());
+                    segments[i].put(entry.getKey(), entry.getValue());
+                }
+            }
+        }
+    }
+
+}

Propchange: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/cache/ConcurrentCache.java
------------------------------------------------------------------------------
    svn:eol-style = native

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=1004182&r1=1004181&r2=1004182&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 10:12:35 2010
@@ -18,6 +18,7 @@ package org.apache.jackrabbit.core.persi
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.apache.jackrabbit.core.cache.ConcurrentCache;
 import org.apache.jackrabbit.core.fs.FileSystemResource;
 import org.apache.jackrabbit.core.fs.FileSystem;
 import org.apache.jackrabbit.core.state.ItemState;
@@ -37,9 +38,7 @@ import org.apache.jackrabbit.core.persis
 import org.apache.jackrabbit.core.persistence.PersistenceManager;
 import org.apache.jackrabbit.core.util.StringIndex;
 import org.apache.jackrabbit.core.persistence.util.BLOBStore;
-import org.apache.jackrabbit.core.persistence.util.BundleCache;
 import org.apache.jackrabbit.core.persistence.util.FileBasedIndex;
-import org.apache.jackrabbit.core.persistence.util.LRUNodeIdCache;
 import org.apache.jackrabbit.core.persistence.util.NodePropBundle;
 import org.apache.jackrabbit.spi.Name;
 import org.apache.jackrabbit.spi.commons.name.NameConstants;
@@ -68,7 +67,7 @@ import javax.jcr.PropertyType;
  * included in the bundle but generated when required.
  * <p/>
  * In order to increase performance, there are 2 caches maintained. One is the
- * {@link BundleCache} that caches already loaded bundles. The other is the
+ * bundle cache that caches already loaded bundles. The other is the
  * {@link LRUNodeIdCache} that caches non-existent bundles. This is useful
  * because a lot of {@link #exists(NodeId)} calls are issued that would result
  * in a useless SQL execution if the desired bundle does not exist.
@@ -96,6 +95,10 @@ public abstract class AbstractBundlePers
     /** the name of the namespace-index resource */
     protected static final String RES_NS_INDEX = "/namespaces.properties";
 
+    /** Sentinel instance used to mark a non-existent bundle in the cache */
+    private static final NodePropBundle MISSING =
+        new NodePropBundle(new NodeId());
+
     /** the index for namespaces */
     private StringIndex nsIndex;
 
@@ -103,10 +106,7 @@ public abstract class AbstractBundlePers
     private StringIndex nameIndex;
 
     /** the cache of loaded bundles */
-    private BundleCache bundles;
-
-    /** the cache of non-existent bundles */
-    private LRUNodeIdCache missing;
+    private ConcurrentCache<NodeId, NodePropBundle> bundles;
 
     /** the persistence manager context */
     protected PMContext context;
@@ -287,25 +287,22 @@ public abstract class AbstractBundlePers
      */
     public synchronized void onExternalUpdate(ChangeLog changes) {
         for (ItemState state : changes.modifiedStates()) {
-            if (state.isNode()) {
-                bundles.remove((NodeId) state.getId());
-            } else {
-                bundles.remove(state.getParentId());
-            }
+            bundles.remove(getBundleId(state));
         }
         for (ItemState state : changes.deletedStates()) {
-            if (state.isNode()) {
-                bundles.remove((NodeId) state.getId());
-            } else {
-                bundles.remove(state.getParentId());
-            }
+            bundles.remove(getBundleId(state));
         }
         for (ItemState state : changes.addedStates()) {
-            if (state.isNode()) {
-                missing.remove((NodeId) state.getId());
-            } else {
-                missing.remove(state.getParentId());
-            }
+            // There may have been a cache miss entry
+            bundles.remove(getBundleId(state));
+        }
+    }
+
+    private NodeId getBundleId(ItemState state) {
+        if (state.isNode()) {
+            return (NodeId) state.getId();
+        } else {
+            return state.getParentId();
         }
     }
 
@@ -387,8 +384,8 @@ public abstract class AbstractBundlePers
     public void init(PMContext context) throws Exception {
         this.context = context;
         // init bundle cache
-        bundles = new BundleCache(bundleCacheSize);
-        missing = new LRUNodeIdCache();
+        bundles = new ConcurrentCache<NodeId, NodePropBundle>();
+        bundles.setMaxMemorySize(bundleCacheSize);
     }
     
     /**
@@ -399,7 +396,6 @@ public abstract class AbstractBundlePers
     public void close() throws Exception {
         // clear caches
         bundles.clear();
-        missing.clear();
     }
 
     /**
@@ -506,7 +502,6 @@ public abstract class AbstractBundlePers
         } finally {
             if (!success) {
                 bundles.clear();
-                missing.clear();
             }
         }
     }
@@ -647,18 +642,17 @@ public abstract class AbstractBundlePers
      * @throws ItemStateException if an error occurs.
      */
     private NodePropBundle getBundle(NodeId id) throws ItemStateException {
-        if (missing.contains(id)) {
-            return null;
-        }
         NodePropBundle bundle = bundles.get(id);
-        if (bundle == null) {
+        if (bundle == MISSING) {
+            return null;
+        } else if (bundle == null) {
             synchronized (this) {
                 bundle = loadBundle(id);
                 if (bundle != null) {
                     bundle.markOld();
-                    bundles.put(bundle);
+                    bundles.put(id, bundle, bundle.getSize());
                 } else {
-                    missing.put(id);
+                    bundles.put(id, MISSING, 16);
                 }
             }
         }
@@ -675,7 +669,7 @@ public abstract class AbstractBundlePers
         destroyBundle(bundle);
         bundle.removeAllProperties(getBlobStore());
         bundles.remove(bundle.getId());
-        missing.put(bundle.getId());
+        bundles.put(bundle.getId(), MISSING, 16);
     }
 
     /**
@@ -689,11 +683,11 @@ public abstract class AbstractBundlePers
         bundle.markOld();
         log.debug("stored bundle {}", bundle.getId());
 
-        missing.remove(bundle.getId());
         // only put to cache if already exists. this is to ensure proper overwrite
         // and not creating big contention during bulk loads
-        if (bundles.contains(bundle.getId())) {
-            bundles.put(bundle);
+        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=1004182&r1=1004181&r2=1004182&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 10:12:35 2010
@@ -24,6 +24,7 @@ import javax.jcr.PropertyType;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.apache.jackrabbit.core.cache.ConcurrentCache;
 import org.apache.jackrabbit.core.fs.FileSystemResource;
 import org.apache.jackrabbit.core.fs.FileSystem;
 import org.apache.jackrabbit.core.id.ItemId;
@@ -34,9 +35,7 @@ import org.apache.jackrabbit.core.persis
 import org.apache.jackrabbit.core.persistence.PMContext;
 import org.apache.jackrabbit.core.persistence.PersistenceManager;
 import org.apache.jackrabbit.core.persistence.util.BLOBStore;
-import org.apache.jackrabbit.core.persistence.util.BundleCache;
 import org.apache.jackrabbit.core.persistence.util.FileBasedIndex;
-import org.apache.jackrabbit.core.persistence.util.LRUNodeIdCache;
 import org.apache.jackrabbit.core.persistence.util.NodePropBundle;
 import org.apache.jackrabbit.core.state.ItemState;
 import org.apache.jackrabbit.core.state.ChangeLog;
@@ -68,7 +67,7 @@ import org.apache.jackrabbit.spi.commons
  * included in the bundle but generated when required.
  * <p/>
  * In order to increase performance, there are 2 caches maintained. One is the
- * {@link BundleCache} that caches already loaded bundles. The other is the
+ * bundle cache that caches already loaded bundles. The other is the
  * {@link LRUNodeIdCache} that caches non-existent bundles. This is useful
  * because a lot of {@link #exists(NodeId)} calls are issued that would result
  * in a useless SQL execution if the desired bundle does not exist.
@@ -96,6 +95,10 @@ public abstract class AbstractBundlePers
     /** the name of the namespace-index resource */
     protected static final String RES_NS_INDEX = "/namespaces.properties";
 
+    /** Sentinel instance used to mark a non-existent bundle in the cache */
+    private static final NodePropBundle MISSING =
+        new NodePropBundle(new NodeId());
+
     /** the index for namespaces */
     private StringIndex nsIndex;
 
@@ -103,10 +106,7 @@ public abstract class AbstractBundlePers
     private StringIndex nameIndex;
 
     /** the cache of loaded bundles */
-    private BundleCache bundles;
-
-    /** the cache of non-existent bundles */
-    private LRUNodeIdCache missing;
+    private ConcurrentCache<NodeId, NodePropBundle> bundles;
 
     /** the persistence manager context */
     protected PMContext context;
@@ -287,25 +287,22 @@ public abstract class AbstractBundlePers
      */
     public synchronized void onExternalUpdate(ChangeLog changes) {
         for (ItemState state : changes.modifiedStates()) {
-            if (state.isNode()) {
-                bundles.remove((NodeId) state.getId());
-            } else {
-                bundles.remove(state.getParentId());
-            }
+            bundles.remove(getBundleId(state));
         }
         for (ItemState state : changes.deletedStates()) {
-            if (state.isNode()) {
-                bundles.remove((NodeId) state.getId());
-            } else {
-                bundles.remove(state.getParentId());
-            }
+            bundles.remove(getBundleId(state));
         }
         for (ItemState state : changes.addedStates()) {
-            if (state.isNode()) {
-                missing.remove((NodeId) state.getId());
-            } else {
-                missing.remove(state.getParentId());
-            }
+            // There may have been a cache miss entry
+            bundles.remove(getBundleId(state));
+        }
+    }
+
+    private NodeId getBundleId(ItemState state) {
+        if (state.isNode()) {
+            return (NodeId) state.getId();
+        } else {
+            return state.getParentId();
         }
     }
 
@@ -387,19 +384,18 @@ public abstract class AbstractBundlePers
     public void init(PMContext context) throws Exception {
         this.context = context;
         // init bundle cache
-        bundles = new BundleCache(bundleCacheSize);
-        missing = new LRUNodeIdCache();
+        bundles = new ConcurrentCache<NodeId, NodePropBundle>();
+        bundles.setMaxMemorySize(bundleCacheSize);
     }
 
     /**
      * {@inheritDoc}
      *
-     *  Closes the persistence manager, release acquired resourecs.
+     *  Closes the persistence manager, release acquired resources.
      */
     public void close() throws Exception {
         // clear caches
         bundles.clear();
-        missing.clear();
     }
 
     /**
@@ -504,7 +500,6 @@ public abstract class AbstractBundlePers
         } finally {
             if (!success) {
                 bundles.clear();
-                missing.clear();
             }
         }
     }
@@ -645,18 +640,17 @@ public abstract class AbstractBundlePers
      * @throws ItemStateException if an error occurs.
      */
     private NodePropBundle getBundle(NodeId id) throws ItemStateException {
-        if (missing.contains(id)) {
-            return null;
-        }
         NodePropBundle bundle = bundles.get(id);
-        if (bundle == null) {
+        if (bundle == MISSING) {
+            return null;
+        } else if (bundle == null) {
             synchronized (this) {
                 bundle = loadBundle(id);
                 if (bundle != null) {
                     bundle.markOld();
-                    bundles.put(bundle);
+                    bundles.put(id, bundle, bundle.getSize());
                 } else {
-                    missing.put(id);
+                    bundles.put(id, MISSING, 16);
                 }
             }
         }
@@ -673,7 +667,7 @@ public abstract class AbstractBundlePers
         destroyBundle(bundle);
         bundle.removeAllProperties(getBlobStore());
         bundles.remove(bundle.getId());
-        missing.put(bundle.getId());
+        bundles.put(bundle.getId(), MISSING, 16);
     }
 
     /**
@@ -687,11 +681,11 @@ public abstract class AbstractBundlePers
         bundle.markOld();
         log.debug("stored bundle {}", bundle.getId());
 
-        missing.remove(bundle.getId());
-        // only put to cache if already exists. this is to ensure proper overwrite
-        // and not creating big contention during bulk loads
-        if (bundles.contains(bundle.getId())) {
-            bundles.put(bundle);
+        // 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/ItemStateCache.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/ItemStateCache.java?rev=1004182&r1=1004181&r2=1004182&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/ItemStateCache.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/ItemStateCache.java Mon Oct  4 10:12:35 2010
@@ -83,14 +83,6 @@ public interface ItemStateCache {
     boolean isEmpty();
 
     /**
-     * Informs the cache that the item was modified and the cache might need to
-     * recalculate the items caching weight.
-     *
-     * @param id the id of the item that was modified.
-     */
-    void update(ItemId id);
-
-    /**
      * Informs the cache that it is no longer in use.
      */
     void dispose();

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/ItemStateReferenceCache.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/ItemStateReferenceCache.java?rev=1004182&r1=1004181&r2=1004182&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/ItemStateReferenceCache.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/ItemStateReferenceCache.java Mon Oct  4 10:12:35 2010
@@ -162,14 +162,6 @@ public class ItemStateReferenceCache imp
     /**
      * {@inheritDoc}
      */
-    public synchronized void update(ItemId id) {
-        // delegate
-        cache.update(id);
-    }
-
-    /**
-     * {@inheritDoc}
-     */
     public synchronized boolean isEmpty() {
         // check primary cache
         return refs.isEmpty();

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/MLRUItemStateCache.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/MLRUItemStateCache.java?rev=1004182&r1=1004181&r2=1004182&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/MLRUItemStateCache.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/MLRUItemStateCache.java Mon Oct  4 10:12:35 2010
@@ -16,14 +16,9 @@
  */
 package org.apache.jackrabbit.core.state;
 
-import java.util.ArrayList;
-import java.util.LinkedHashMap;
-import java.util.List;
-import java.util.Map;
-
 import org.apache.commons.collections.map.LinkedMap;
-import org.apache.jackrabbit.core.cache.Cache;
-import org.apache.jackrabbit.core.cache.CacheAccessListener;
+import org.apache.jackrabbit.core.cache.CacheManager;
+import org.apache.jackrabbit.core.cache.ConcurrentCache;
 import org.apache.jackrabbit.core.id.ItemId;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -38,139 +33,59 @@ import org.slf4j.LoggerFactory;
  * TODO rename class to something more appropriate, e.g. FIFOItemSateCache since
  * it doesn't use a LRU eviction policy anymore.
  */
-public class MLRUItemStateCache implements ItemStateCache, Cache {
+public class MLRUItemStateCache implements ItemStateCache {
+
     /** Logger instance */
     private static Logger log = LoggerFactory.getLogger(MLRUItemStateCache.class);
 
     /** default maximum memory to use */
     public static final int DEFAULT_MAX_MEM = 4 * 1024 * 1024;
 
-    /** the amount of memory the entries use */
-    private volatile long totalMem;
-
-    /** the maximum of memory the cache may use */
-    private volatile long maxMem;
-
     /** the number of writes */
-    private volatile long numWrites;
-
-    /** the access count */
-    private volatile long accessCount;
+    private volatile long numWrites = 0;
 
-    /** the cache access listeners */
-    private CacheAccessListener accessListener;
-
-    /**
-     * A cache for <code>ItemState</code> instances
-     */
-    private final Map<ItemId, Entry> cache;
-
-    /**
-     * Constructs a new, empty <code>ItemStateCache</code> with a maximum amount
-     * of memory of {@link #DEFAULT_MAX_MEM}.
-     */
-    public MLRUItemStateCache() {
-        this(DEFAULT_MAX_MEM);
-    }
+    private final ConcurrentCache<ItemId, ItemState> cache =
+        new ConcurrentCache<ItemId, ItemState>();
 
-    /**
-     * Constructs a new, empty <code>ItemStateCache</code> with the specified
-     * maximum memory.
-     *
-     * @param maxMem the maximum amount of memory this cache may use.
-     */
-    @SuppressWarnings("serial")
-    private MLRUItemStateCache(int maxMem) {
-        this.maxMem = maxMem;
-        this.cache = new LinkedHashMap<ItemId, MLRUItemStateCache.Entry>(
-                maxMem / 1024, 0.75f, true /* access-ordered */) {
-            @Override
-            protected boolean removeEldestEntry(Map.Entry<ItemId, Entry> e) {
-                long maxMem = MLRUItemStateCache.this.maxMem;
-                if (totalMem <= maxMem) {
-                    return false;
-                } else if (totalMem - e.getValue().size <= maxMem) {
-                    totalMem -= e.getValue().size;
-                    return true;
-                } else {
-                    shrink();
-                    return false;
-                }
-            }
-        };
+    public MLRUItemStateCache(CacheManager cacheMgr) {
+        cache.setMaxMemorySize(DEFAULT_MAX_MEM);
+        cache.setAccessListener(cacheMgr);
+        cacheMgr.add(cache);
     }
 
     //-------------------------------------------------------< ItemStateCache >
+
     /**
      * {@inheritDoc}
      */
     public boolean isCached(ItemId id) {
-        synchronized (cache) {
-            return cache.containsKey(id);
-        }
+        return cache.containsKey(id);
     }
 
     /**
      * {@inheritDoc}
      */
     public ItemState retrieve(ItemId id) {
-        touch();
-        synchronized (cache) {
-            Entry entry = cache.get(id);
-            if (entry != null) {
-                return entry.state;
-            } else {
-                return null;
-            }
-        }
+        return cache.get(id);
     }
 
     /**
      * {@inheritDoc}
      */
     public ItemState[] retrieveAll() {
-        synchronized (cache) {
-            ItemState[] states = new ItemState[cache.size()];
-            int i = 0;
-            for (Entry entry : cache.values()) {
-                states[i++] = entry.state;
-            }
-            return states;
-        }
-    }
-
-    /**
-     * {@inheritDoc}
-     */
-    public void update(ItemId id) {
-        touch();
-        synchronized (cache) {
-            Entry entry = cache.get(id);
-            if (entry != null) {
-                totalMem -= entry.size;
-                entry.recalc();
-                totalMem += entry.size;
-            }
-        }
+        return cache.values();
     }
 
     /**
      * {@inheritDoc}
      */
     public void cache(ItemState state) {
-        touch();
-        synchronized (cache) {
-            ItemId id = state.getId();
-            if (cache.containsKey(id)) {
-                log.warn("overwriting cached entry " + id);
-                evict(id);
-            }
-            Entry entry = new Entry(state);
-            totalMem += entry.size;
-            cache.put(id, entry);
-            if (numWrites++ % 10000 == 0 && log.isDebugEnabled()) {
-                log.debug(this + " size=" + cache.size() + ", " + totalMem + "/" + maxMem);
-            }
+        cache.put(state.getId(), state, state.calculateMemoryFootprint());
+
+        if (numWrites++ % 10000 == 0 && log.isDebugEnabled()) {
+            log.debug("Item state cache size: {}% of {} bytes",
+                    cache.getMemoryUsed() * 100 / cache.getMaxMemorySize(),
+                    cache.getMaxMemorySize());
         }
     }
 
@@ -178,135 +93,28 @@ public class MLRUItemStateCache implemen
      * {@inheritDoc}
      */
     public void evict(ItemId id) {
-        touch();
-        synchronized (cache) {
-            Entry entry = cache.remove(id);
-            if (entry != null) {
-                totalMem -= entry.size;
-            }
-        }
+        cache.remove(id);
     }
 
     /**
      * {@inheritDoc}
      */
     public void evictAll() {
-        synchronized (cache) {
-            cache.clear();
-            totalMem = 0;
-        }
+        cache.clear();
     }
 
     /**
      * {@inheritDoc}
      */
     public boolean isEmpty() {
-        synchronized (cache) {
-            return cache.isEmpty();
-        }
-    }
-
-    private void touch() {
-        accessCount++;
-        if ((accessCount % CacheAccessListener.ACCESS_INTERVAL) == 0) {
-            if (accessListener != null) {
-                accessListener.cacheAccessed();
-            }
-        }
-    }
-
-    /**
-     * {@inheritDoc}
-     */
-    public long getAccessCount() {
-        return accessCount;
-    }
-
-    /**
-     * {@inheritDoc}
-     */
-    public long getMaxMemorySize() {
-        return maxMem;
-    }
-
-    /**
-     * {@inheritDoc}
-     */
-    public long getMemoryUsed() {
-        return totalMem;
-    }
-
-    /**
-     * {@inheritDoc}
-     */
-    public void resetAccessCount() {
-        synchronized (cache) {
-            accessCount = 0;
-        }
-    }
-
-    /**
-     * {@inheritDoc}
-     */
-    public void setMaxMemorySize(long size) {
-        synchronized (cache) {
-            this.maxMem = size;
-
-            // remove items, if too many
-            if (totalMem > maxMem) {
-                shrink();
-            }
-        }
-    }
-
-    private void shrink() {
-        List<Map.Entry<ItemId, Entry>> list =
-            new ArrayList<Map.Entry<ItemId, Entry>>(cache.entrySet());
-        for (int i = list.size() - 1; totalMem > maxMem && i >= 0; i--) {
-            Map.Entry<ItemId, Entry> last = list.get(i);
-            totalMem -= last.getValue().size;
-            cache.remove(last.getKey());
-        }
-    }
-
-    /**
-     * Set the cache access listener. Only one listener per cache is supported.
-     *
-     * @param listener the new listener
-     */
-    public void setAccessListener(CacheAccessListener listener) {
-        this.accessListener = listener;
+        return cache.isEmpty();
     }
 
     /**
      * {@inheritDoc}
      */
     public void dispose() {
-        synchronized (cache) {
-            if (accessListener != null) {
-                accessListener.disposeCache(this);
-            }
-        }
-    }
-
-
-    /**
-     * Internal cache entry.
-     */
-    private static class Entry {
-
-        private final ItemState state;
-
-        private long size;
-
-        public Entry(ItemState state) {
-            this.state = state;
-            this.size = 64 + state.calculateMemoryFootprint();
-        }
-
-        public void recalc() {
-            size = 64 + state.calculateMemoryFootprint();
-        }
+        cache.dispose();
     }
 
 }

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/ManagedMLRUItemStateCacheFactory.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/ManagedMLRUItemStateCacheFactory.java?rev=1004182&r1=1004181&r2=1004182&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/ManagedMLRUItemStateCacheFactory.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/ManagedMLRUItemStateCacheFactory.java Mon Oct  4 10:12:35 2010
@@ -41,10 +41,7 @@ public class ManagedMLRUItemStateCacheFa
      * Create a new cache instance and link it to the cache manager.
      */
     public ItemStateCache newItemStateCache() {
-        MLRUItemStateCache cache = new MLRUItemStateCache();
-        cacheMgr.add(cache);
-        cache.setAccessListener(cacheMgr);
-        return cache;
+        return new MLRUItemStateCache(cacheMgr);
     }
 
 }

Added: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/cache/ConcurrentCacheTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/cache/ConcurrentCacheTest.java?rev=1004182&view=auto
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/cache/ConcurrentCacheTest.java (added)
+++ jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/cache/ConcurrentCacheTest.java Mon Oct  4 10:12:35 2010
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.jackrabbit.core.cache;
+
+import org.apache.jackrabbit.core.id.NodeId;
+
+import junit.framework.TestCase;
+
+/**
+ * Test cases for the {@link ConcurrentCache} class.
+ */
+public class ConcurrentCacheTest extends TestCase {
+
+    /**
+     * Tests a concurrent cache by adding lots of random items to it
+     * and checking that the excess items have automatically been evicted
+     * while frequently accessed items are still present.
+     */
+    public void testConcurrentCache() {
+        NodeId[] ids = new NodeId[1000];
+        for (int i = 0; i < ids.length; i++) {
+            ids[i] = new NodeId();
+        }
+
+        ConcurrentCache<NodeId, NodeId> cache =
+            new ConcurrentCache<NodeId, NodeId>();
+        cache.setMaxMemorySize(ids.length / 2);
+
+        for (int i = 0; i < ids.length; i++) {
+            for (int j = 0; j < i; j += 3) {
+                cache.get(ids[j]);
+            }
+            cache.put(ids[i], ids[i], 1);
+        }
+
+        assertTrue(cache.getMemoryUsed() <= ids.length / 2);
+
+        int n = 0;
+        for (int i = 0; i < ids.length; i++) {
+            if (cache.containsKey(ids[i])) {
+                n++;
+            }
+        }
+
+        assertTrue(n <= ids.length / 2);
+
+        n = 0;
+        for (int i = 0; i < ids.length; i += 3) {
+            if (cache.containsKey(ids[i])) {
+                n++;
+            }
+        }
+
+        assertTrue(cache.getMemoryUsed() > ids.length / 4);
+    }
+
+}

Propchange: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/cache/ConcurrentCacheTest.java
------------------------------------------------------------------------------
    svn:eol-style = native