1:51 p.m.
Author: manik.surtani(a)jboss.com
Date: 2008-12-10 07:51:42 -0500 (Wed, 10 Dec 2008)
New Revision: 7274
Modified:
core/branches/flat/src/main/java/org/jboss/starobrno/context/EntryLookup.java
core/branches/flat/src/main/java/org/jboss/starobrno/context/InvocationContextImpl.java
core/branches/flat/src/main/java/org/jboss/starobrno/context/TransactionContextImpl.java
core/branches/flat/src/main/java/org/jboss/starobrno/interceptors/LockingInterceptor.java
core/branches/flat/src/test/java/org/jboss/starobrno/api/mvcc/read_committed/ReadCommittedLockTest.java
core/branches/flat/src/test/java/org/jboss/starobrno/profiling/TreeProfileTest.java
Log:
Perf improvements + added tests
Modified: core/branches/flat/src/main/java/org/jboss/starobrno/context/EntryLookup.java
===================================================================
---
core/branches/flat/src/main/java/org/jboss/starobrno/context/EntryLookup.java 2008-12-10
11:04:37 UTC (rev 7273)
+++
core/branches/flat/src/main/java/org/jboss/starobrno/context/EntryLookup.java 2008-12-10
12:51:42 UTC (rev 7274)
@@ -41,5 +41,7 @@
void putLookedUpEntries(Map<Object, MVCCEntry> lookedUpEntries);
+ void removeLookedUpEntry(Object key);
+
void clearLookedUpEntries();
}
Modified:
core/branches/flat/src/main/java/org/jboss/starobrno/context/InvocationContextImpl.java
===================================================================
---
core/branches/flat/src/main/java/org/jboss/starobrno/context/InvocationContextImpl.java 2008-12-10
11:04:37 UTC (rev 7273)
+++
core/branches/flat/src/main/java/org/jboss/starobrno/context/InvocationContextImpl.java 2008-12-10
12:51:42 UTC (rev 7274)
@@ -28,9 +28,13 @@
import org.jboss.starobrno.container.MVCCEntry;
import org.jboss.starobrno.transaction.GlobalTransaction;
import org.jboss.starobrno.transaction.TransactionTable;
+import org.jboss.starobrno.util.FastCopyHashMap;
import javax.transaction.Transaction;
-import java.util.*;
+import java.util.Collections;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Map;
/**
* // TODO: MANIK: Document this
@@ -59,15 +63,15 @@
* once we drop support for opt/pess locks we can genericise this to contain Fqns. -
Manik Surtani, June 2008
*/
protected LinkedHashSet<Object> invocationLocks;
- private HashMap<Object, MVCCEntry> lookedUpEntries = null;
+ private FastCopyHashMap<Object, MVCCEntry> lookedUpEntries = null;
/**
* Retrieves a node from the registry of looked up nodes in the current scope.
* <p/>
- * If a transaction is in progress, implementations should delegate to {@link
org.jboss.cache.transaction.MVCCTransactionContext#lookUpNode(Fqn)}
+ * If a transaction is in progress, implementations should delegate to {@link
org.jboss.starobrno.context.TransactionContext#lookupEntry(Object)}
* <p/>
*
- * @param fqn fqn to look up
+ * @param k key to look up
* @return a node, or null if it cannot be found.
*/
public MVCCEntry lookupEntry(Object k)
@@ -82,10 +86,22 @@
}
}
+ public void removeLookedUpEntry(Object key)
+ {
+ if (transactionContext != null)
+ {
+ transactionContext.removeLookedUpEntry(key);
+ }
+ else
+ {
+ if (lookedUpEntries != null) lookedUpEntries.remove(key);
+ }
+ }
+
/**
* Puts an entry in the registry of looked up nodes in the current scope.
* <p/>
- * If a transaction is in progress, implementations should delegate to {@link
org.jboss.cache.transaction.MVCCTransactionContext#putLookedUpNode(Fqn, NodeSPI)}
+ * If a transaction is in progress, implementations should delegate to {@link
TransactionContext#putLookedUpEntry(Object, org.jboss.starobrno.container.MVCCEntry)}
* <p/>
*
* @param key
@@ -96,7 +112,7 @@
transactionContext.putLookedUpEntry(key, e);
else
{
- if (lookedUpEntries == null) lookedUpEntries = new HashMap<Object,
MVCCEntry>(4);
+ if (lookedUpEntries == null) lookedUpEntries = new FastCopyHashMap<Object,
MVCCEntry>(4);
lookedUpEntries.put(key, e);
}
}
@@ -108,7 +124,7 @@
else
{
if (this.lookedUpEntries == null)
- this.lookedUpEntries = new HashMap<Object, MVCCEntry>();
+ this.lookedUpEntries = new FastCopyHashMap<Object, MVCCEntry>();
this.lookedUpEntries.putAll(lookedUpEntries);
}
@@ -142,7 +158,7 @@
{
InvocationContextImpl copy = new InvocationContextImpl();
doCopy(copy);
- if (lookedUpEntries != null) copy.lookedUpEntries = (HashMap<Object,
MVCCEntry>) lookedUpEntries.clone();
+ if (lookedUpEntries != null) copy.lookedUpEntries = (FastCopyHashMap<Object,
MVCCEntry>) lookedUpEntries.clone();
return copy;
}
@@ -314,9 +330,6 @@
* Note that if a transaction is in scope, implementations should record this lock on
the {@link org.jboss.cache.transaction.TransactionContext}.
* Using this method should always ensure that the appropriate scope is used.
* <p/>
- * Note that currently (as of 3.0.0) this lock is weakly typed. This is to allow
support for both MVCC (which uses {@link Fqn}s as locks)
- * as well as legacy Optimistic and Pessimistic Locking schemes (which use {@link
org.jboss.cache.lock.NodeLock} as locks). Once support for
- * legacy node locking schemes are dropped, this method will be more strongly typed to
accept {@link Fqn}.
*
* @param lock lock to add
*/
@@ -342,9 +355,6 @@
* Note that if a transaction is in scope, implementations should remove this lock
from the {@link org.jboss.cache.transaction.TransactionContext}.
* Using this method should always ensure that the lock is removed from the
appropriate scope.
* <p/>
- * Note that currently (as of 3.0.0) this lock is weakly typed. This is to allow
support for both MVCC (which uses {@link Fqn}s as locks)
- * as well as legacy Optimistic and Pessimistic Locking schemes (which use {@link
org.jboss.cache.lock.NodeLock} as locks). Once support for
- * legacy node locking schemes are dropped, this method will be more strongly typed to
accept {@link Fqn}.
*
* @param lock lock to remove
*/
@@ -369,9 +379,6 @@
* Note that if a transaction is in scope, implementations should clear locks from the
{@link org.jboss.cache.transaction.TransactionContext}.
* Using this method should always ensure locks are cleared in the appropriate scope.
* <p/>
- * Note that currently (as of 3.0.0) this lock is weakly typed. This is to allow
support for both MVCC (which uses {@link Fqn}s as locks)
- * as well as legacy Optimistic and Pessimistic Locking schemes (which use {@link
org.jboss.cache.lock.NodeLock} as locks). Once support for
- * legacy node locking schemes are dropped, this method will be more strongly typed to
accept {@link Fqn}.
*/
public void clearKeysLocked()
{
@@ -408,7 +415,7 @@
}
/**
- * @return true if options exist to suppress locking - false otherwise. Note that
this is only used by the {@link org.jboss.cache.interceptors.PessimisticLockInterceptor}.
+ * @return true if options exist to suppress locking - false otherwise.
*/
public boolean isLockingSuppressed()
{
Modified:
core/branches/flat/src/main/java/org/jboss/starobrno/context/TransactionContextImpl.java
===================================================================
---
core/branches/flat/src/main/java/org/jboss/starobrno/context/TransactionContextImpl.java 2008-12-10
11:04:37 UTC (rev 7273)
+++
core/branches/flat/src/main/java/org/jboss/starobrno/context/TransactionContextImpl.java 2008-12-10
12:51:42 UTC (rev 7274)
@@ -26,6 +26,7 @@
import org.jboss.starobrno.config.Option;
import org.jboss.starobrno.container.MVCCEntry;
import org.jboss.starobrno.transaction.GlobalTransaction;
+import org.jboss.starobrno.util.FastCopyHashMap;
import javax.transaction.RollbackException;
import javax.transaction.SystemException;
@@ -77,7 +78,7 @@
*/
private List<Object> removedNodes = null;
- private final Map<Object, MVCCEntry> lookedUpEntries = new HashMap<Object,
MVCCEntry>(8);
+ private final FastCopyHashMap<Object, MVCCEntry> lookedUpEntries = new
FastCopyHashMap<Object, MVCCEntry>(8);
private GlobalTransaction gtx;
public TransactionContextImpl(Transaction tx) throws SystemException,
RollbackException
@@ -100,6 +101,11 @@
return lookedUpEntries.get(key);
}
+ public void removeLookedUpEntry(Object key)
+ {
+ lookedUpEntries.remove(key);
+ }
+
/**
* Puts an entry in the registry of looked up nodes in the current scope.
* <p/>
@@ -107,7 +113,7 @@
* would delegate to this method if a transaction is in scope.
* <p/>
*
- * @param key
+ * @param key key to put
*/
public void putLookedUpEntry(Object key, MVCCEntry entry)
{
@@ -290,7 +296,7 @@
public String toString()
{
StringBuilder sb = new StringBuilder();
- sb.append("TransactionContext (" + System.identityHashCode(this) +
") nmodificationList: ").append(modificationList);
+ sb.append("TransactionContext
(").append(System.identityHashCode(this)).append(") nmodificationList:
").append(modificationList);
return sb.toString();
}
Modified:
core/branches/flat/src/main/java/org/jboss/starobrno/interceptors/LockingInterceptor.java
===================================================================
---
core/branches/flat/src/main/java/org/jboss/starobrno/interceptors/LockingInterceptor.java 2008-12-10
11:04:37 UTC (rev 7273)
+++
core/branches/flat/src/main/java/org/jboss/starobrno/interceptors/LockingInterceptor.java 2008-12-10
12:51:42 UTC (rev 7274)
@@ -22,6 +22,7 @@
package org.jboss.starobrno.interceptors;
import org.jboss.cache.lock.IsolationLevel;
+import org.jboss.starobrno.commands.TransactionBoundaryCommand;
import org.jboss.starobrno.commands.VisitableCommand;
import org.jboss.starobrno.commands.read.GetKeyValueCommand;
import org.jboss.starobrno.commands.read.GravitateDataCommand;
@@ -39,10 +40,10 @@
import org.jboss.starobrno.interceptors.base.PrePostProcessingCommandInterceptor;
import org.jboss.starobrno.lock.LockManager;
-import java.util.HashSet;
+import java.util.HashMap;
import java.util.List;
import java.util.ListIterator;
-import java.util.Set;
+import java.util.Map;
/**
* Interceptor to implement <a
href="http://wiki.jboss.org/wiki/JBossCacheMVCC">MVCC</a>
functionality.
@@ -213,23 +214,18 @@
}
else
{
- if (useReadCommitted)
+ if (useReadCommitted && !(command instanceof
TransactionBoundaryCommand))
{
- if (trace) log.trace("Wiping unnecessary entries from context");
- // wipe all unchanged entries from context, to force subsequent reads to go
to the container,
- // hence providing R_C semantics.
- Set<Object> keysToRemove = new HashSet<Object>();
- for (MVCCEntry e : ctx.getLookedUpEntries().values())
+ Map<Object, MVCCEntry> original = ctx.getLookedUpEntries();
+ if (!original.isEmpty())
{
- if (!e.isChanged()) keysToRemove.add(e.getKey());
+ Map<Object, MVCCEntry> defensiveCopy = new HashMap<Object,
MVCCEntry>(original);
+ for (Map.Entry<Object, MVCCEntry> e : defensiveCopy.entrySet())
+ {
+ if (!e.getValue().isChanged()) ctx.removeLookedUpEntry(e.getKey());
+ }
}
- for (Object k : keysToRemove) ctx.removeKeyLocked(k);
}
- else
- {
- if (trace) log.trace("Nothing to do since there is a transaction in
scope.");
- }
-
}
}
Modified:
core/branches/flat/src/test/java/org/jboss/starobrno/api/mvcc/read_committed/ReadCommittedLockTest.java
===================================================================
---
core/branches/flat/src/test/java/org/jboss/starobrno/api/mvcc/read_committed/ReadCommittedLockTest.java 2008-12-10
11:04:37 UTC (rev 7273)
+++
core/branches/flat/src/test/java/org/jboss/starobrno/api/mvcc/read_committed/ReadCommittedLockTest.java 2008-12-10
12:51:42 UTC (rev 7274)
@@ -1,8 +1,11 @@
package org.jboss.starobrno.api.mvcc.read_committed;
+import org.jboss.starobrno.Cache;
import org.jboss.starobrno.api.mvcc.LockTestBase;
import org.testng.annotations.Test;
+import javax.transaction.Transaction;
+
@Test(groups = {"functional", "mvcc"})
public class ReadCommittedLockTest extends LockTestBase
{
@@ -10,4 +13,33 @@
{
repeatableRead = false;
}
+
+ public void testVisibilityOfCommittedData() throws Exception
+ {
+ Cache c = threadLocal.get().cache;
+ c.put("k", "v");
+
+ assert "v".equals(c.get("k"));
+
+ // start a tx and read K
+ threadLocal.get().tm.begin();
+ assert "v".equals(c.get("k"));
+ assert "v".equals(c.get("k"));
+ Transaction reader = threadLocal.get().tm.suspend();
+
+ threadLocal.get().tm.begin();
+ c.put("k", "v2");
+ Transaction writer = threadLocal.get().tm.suspend();
+
+ threadLocal.get().tm.resume(reader);
+ assert "v".equals(c.get("k")) : "Should not read
uncommitted data";
+ reader = threadLocal.get().tm.suspend();
+
+ threadLocal.get().tm.resume(writer);
+ threadLocal.get().tm.commit();
+
+ threadLocal.get().tm.resume(reader);
+ assert "v2".equals(c.get("k")) : "Should read committed
data";
+ threadLocal.get().tm.commit();
+ }
}
Modified:
core/branches/flat/src/test/java/org/jboss/starobrno/profiling/TreeProfileTest.java
===================================================================
---
core/branches/flat/src/test/java/org/jboss/starobrno/profiling/TreeProfileTest.java 2008-12-10
11:04:37 UTC (rev 7273)
+++
core/branches/flat/src/test/java/org/jboss/starobrno/profiling/TreeProfileTest.java 2008-12-10
12:51:42 UTC (rev 7274)
@@ -42,10 +42,10 @@
Test configuration options
*/
protected static final long NUM_OPERATIONS = 1000000; // DURATION is replaced with a
fixed number of operations instead.
- protected static final int NUM_THREADS = 50;
+ protected static final int NUM_THREADS = 25;
protected static final int MAX_RANDOM_SLEEP_MILLIS = 1;
- protected static final int MAX_OVERALL_FQNS = 2000;
- protected static final int TREE_DEPTH = 2;
+ protected static final int MAX_DEPTH = 3;
+ protected static final int MAX_OVERALL_NODES = 2000;
protected static final int WARMUP_LOOPS = 20000;
protected static final boolean USE_SLEEP = false; // throttle generation a bit
@@ -60,7 +60,7 @@
cfg.setConcurrencyLevel(2000);
cfg.setLockAcquisitionTimeout(120000);
cfg.setLockParentForChildInsertRemove(true);
- cfg.setIsolationLevel(IsolationLevel.REPEATABLE_READ);
+ cfg.setIsolationLevel(IsolationLevel.READ_COMMITTED);
Cache c = new UnitTestCacheFactory().createCache(cfg);
cache = new TreeCacheImpl(c);
}
@@ -72,7 +72,7 @@
cache = null;
}
- private List<Fqn> fqns = new ArrayList<Fqn>(MAX_OVERALL_FQNS);
+ private List<Fqn> fqns = new ArrayList<Fqn>(MAX_OVERALL_NODES);
Log log = LogFactory.getLog(TreeProfileTest.class);
@@ -102,12 +102,12 @@
long startTime = System.currentTimeMillis();
log.warn("Starting init() phase");
fqns.clear();
- for (int i = 0; i < MAX_OVERALL_FQNS; i++)
+ for (int i = 0; i < MAX_OVERALL_NODES; i++)
{
Fqn fqn;
do
{
- fqn = Generator.createRandomFqn(TREE_DEPTH);
+ fqn = Generator.createRandomFqn(MAX_DEPTH);
}
while (fqns.contains(fqn));