Return-Path: Delivered-To: apmail-jackrabbit-commits-archive@www.apache.org Received: (qmail 52458 invoked from network); 4 Oct 2010 10:13:04 -0000 Received: from unknown (HELO mail.apache.org) (140.211.11.3) by 140.211.11.9 with SMTP; 4 Oct 2010 10:13:04 -0000 Received: (qmail 89829 invoked by uid 500); 4 Oct 2010 10:13:04 -0000 Delivered-To: apmail-jackrabbit-commits-archive@jackrabbit.apache.org Received: (qmail 89703 invoked by uid 500); 4 Oct 2010 10:13:00 -0000 Mailing-List: contact commits-help@jackrabbit.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@jackrabbit.apache.org Delivered-To: mailing list commits@jackrabbit.apache.org Received: (qmail 89696 invoked by uid 99); 4 Oct 2010 10:13:00 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 04 Oct 2010 10:13:00 +0000 X-ASF-Spam-Status: No, hits=-2000.0 required=10.0 tests=ALL_TRUSTED X-Spam-Check-By: apache.org Received: from [140.211.11.4] (HELO eris.apache.org) (140.211.11.4) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 04 Oct 2010 10:12:57 +0000 Received: by eris.apache.org (Postfix, from userid 65534) id 5A81D23888FE; Mon, 4 Oct 2010 10:12:36 +0000 (UTC) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: 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... Date: Mon, 04 Oct 2010 10:12:36 -0000 To: commits@jackrabbit.apache.org From: jukka@apache.org X-Mailer: svnmailer-1.0.8 Message-Id: <20101004101236.5A81D23888FE@eris.apache.org> 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. + *

+ * 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 true. + */ +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 accessListener = + new AtomicReference(); + + /** + * Checks whether the current estimate of the amount of memory used + * by this cache exceeds the allocated maximum amount of memory. + * + * @return true if the cache size is too big, + * false 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 extends AbstractCache { + + /** Logger instance */ + private static Logger log = LoggerFactory.getLogger(ConcurrentCache.class); + + private static class E { + + private final V value; + + private final long size; + + public E(V value, long size) { + this.value = value; + this.size = size; + } + + } + + private final Map>[] 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>(1024, 0.75f, true) { + @Override + protected boolean removeEldestEntry(Map.Entry> 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> 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 true if the entry is cached, + * false otherwise + */ + public boolean containsKey(K key) { + Map> segment = getSegment(key); + synchronized (segment) { + return segment.containsKey(key); + } + } + + /** + * Returns the identified cache entry. + * + * @param key entry key + * @return entry value, or null if not found + */ + public V get(K key) { + recordCacheAccess(); + + Map> segment = getSegment(key); + synchronized (segment) { + E 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 values = new ArrayList(); + for (int i = 0; i < segments.length; i++) { + synchronized (segments[i]) { + for (E 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> segment = getSegment(key); + synchronized (segment) { + recordSizeChange(size); + E previous = segment.put(key, new E(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 null if not found + */ + public V remove(K key) { + Map> segment = getSegment(key); + synchronized (segment) { + E 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 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>> iterator = + segments[i].entrySet().iterator(); + if (iterator.hasNext()) { + Map.Entry> 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. *

* 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 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(); + 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. *

* 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 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(); + 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 ItemState instances - */ - private final Map cache; - - /** - * Constructs a new, empty ItemStateCache with a maximum amount - * of memory of {@link #DEFAULT_MAX_MEM}. - */ - public MLRUItemStateCache() { - this(DEFAULT_MAX_MEM); - } + private final ConcurrentCache cache = + new ConcurrentCache(); - /** - * Constructs a new, empty ItemStateCache 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( - maxMem / 1024, 0.75f, true /* access-ordered */) { - @Override - protected boolean removeEldestEntry(Map.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> list = - new ArrayList>(cache.entrySet()); - for (int i = list.size() - 1; totalMem > maxMem && i >= 0; i--) { - Map.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 cache = + new ConcurrentCache(); + 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