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 15:04:45 UTC
svn commit: r1004223 - in
/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core:
cache/ persistence/bundle/ persistence/pool/ state/
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;
}