[jbosscache-commits] JBoss Cache SVN: r6378 - in core/trunk/src: main/java/org/jboss/cache/invocation and 5 other directories.
jbosscache-commits at lists.jboss.org
jbosscache-commits at lists.jboss.org
Wed Jul 23 11:00:08 EDT 2008
Author: manik.surtani at jboss.com
Date: 2008-07-23 11:00:08 -0400 (Wed, 23 Jul 2008)
New Revision: 6378
Added:
core/trunk/src/test/java/org/jboss/cache/util/ImmutableListCopyTest.java
Modified:
core/trunk/src/main/java/org/jboss/cache/interceptors/MVCCLockingInterceptor.java
core/trunk/src/main/java/org/jboss/cache/invocation/AbstractInvocationContext.java
core/trunk/src/main/java/org/jboss/cache/invocation/InvocationContext.java
core/trunk/src/main/java/org/jboss/cache/lock/MVCCLockManager.java
core/trunk/src/main/java/org/jboss/cache/lock/NodeBasedLockManager.java
core/trunk/src/main/java/org/jboss/cache/mvcc/MVCCNodeHelper.java
core/trunk/src/main/java/org/jboss/cache/transaction/PessimisticTransactionContext.java
core/trunk/src/main/java/org/jboss/cache/transaction/TransactionContext.java
core/trunk/src/main/java/org/jboss/cache/util/ImmutableListCopy.java
Log:
Implemented efficient custom collections in place of immutable and defensively copied collections
Modified: core/trunk/src/main/java/org/jboss/cache/interceptors/MVCCLockingInterceptor.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/interceptors/MVCCLockingInterceptor.java 2008-07-23 14:23:32 UTC (rev 6377)
+++ core/trunk/src/main/java/org/jboss/cache/interceptors/MVCCLockingInterceptor.java 2008-07-23 15:00:08 UTC (rev 6378)
@@ -35,6 +35,7 @@
import java.util.Collections;
import java.util.LinkedList;
import java.util.List;
+import java.util.ListIterator;
import java.util.Map;
/**
@@ -321,18 +322,18 @@
if (!(locks = ctx.getLocks()).isEmpty())
{
// clean up.
- Fqn[] fqnsToUnlock = new Fqn[locks.size()];
- fqnsToUnlock = locks.toArray(fqnsToUnlock);
+ // unlocking needs to be done in reverse order.
+ ListIterator<Fqn> it = locks.listIterator(locks.size());
Object owner = Thread.currentThread();
-
- for (int i = fqnsToUnlock.length - 1; i > -1; i--)
+ while (it.hasPrevious())
{
+ Fqn f = it.previous();
// for each of these, swap refs
- ReadCommittedNode rcn = (ReadCommittedNode) ctx.lookUpNode(fqnsToUnlock[i]);
+ ReadCommittedNode rcn = (ReadCommittedNode) ctx.lookUpNode(f);
if (rcn != null) rcn.commitUpdate(ctx, dataContainer, nodeFactory); // could be null with read-committed
// and then unlock
- if (trace) log.trace("Releasing lock on [" + fqnsToUnlock[i] + "] for thread " + owner);
- lockManager.unlock(fqnsToUnlock[i], owner);
+ if (trace) log.trace("Releasing lock on [" + f + "] for thread " + owner);
+ lockManager.unlock(f, owner);
}
ctx.clearLocks();
}
@@ -356,13 +357,13 @@
if (!(locks = ctx.getTransactionContext().getLocks()).isEmpty())
{
// clean up.
- Fqn[] fqnsToUnlock = new Fqn[locks.size()];
- fqnsToUnlock = locks.toArray(fqnsToUnlock);
+ // unlocking needs to be done in reverse order.
+ ListIterator<Fqn> it = locks.listIterator(locks.size());
Object owner = ctx.getGlobalTransaction();
-
- for (int i = fqnsToUnlock.length - 1; i > -1; i--)
+ while (it.hasPrevious())
{
- ReadCommittedNode rcn = (ReadCommittedNode) ctx.lookUpNode(fqnsToUnlock[i]);
+ Fqn f = it.previous();
+ ReadCommittedNode rcn = (ReadCommittedNode) ctx.lookUpNode(f);
if (rcn != null) // could be null with read-committed
{
if (commit)
@@ -376,8 +377,8 @@
}
}
// and then unlock
- if (trace) log.trace("Releasing lock on [" + fqnsToUnlock[i] + "] for transaction " + owner);
- lockManager.unlock(fqnsToUnlock[i], owner);
+ if (trace) log.trace("Releasing lock on [" + f + "] for transaction " + owner);
+ lockManager.unlock(f, owner);
}
ctx.clearLocks();
}
Modified: core/trunk/src/main/java/org/jboss/cache/invocation/AbstractInvocationContext.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/invocation/AbstractInvocationContext.java 2008-07-23 14:23:32 UTC (rev 6377)
+++ core/trunk/src/main/java/org/jboss/cache/invocation/AbstractInvocationContext.java 2008-07-23 15:00:08 UTC (rev 6378)
@@ -182,6 +182,19 @@
}
}
+ public boolean hasLock(Object lock)
+ {
+ // first check transactional scope
+ if (transactionContext != null)
+ {
+ return transactionContext.hasLock(lock);
+ }
+ else
+ {
+ return invocationLocks != null && invocationLocks.contains(lock);
+ }
+ }
+
public boolean isLockingSuppressed()
{
return getOptionOverrides() != null && getOptionOverrides().isSuppressLocking();
Modified: core/trunk/src/main/java/org/jboss/cache/invocation/InvocationContext.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/invocation/InvocationContext.java 2008-07-23 14:23:32 UTC (rev 6377)
+++ core/trunk/src/main/java/org/jboss/cache/invocation/InvocationContext.java 2008-07-23 15:00:08 UTC (rev 6378)
@@ -205,7 +205,17 @@
*/
void clearLocks();
+
/**
+ * Note that if a transaction is in scope, implementations should test this lock from on {@link org.jboss.cache.transaction.TransactionContext}.
+ * Using this method should always ensure locks checked in the appropriate scope.
+ *
+ * @param lock lock to test
+ * @return true if the lock being tested is already held in the current scope, false otherwise.
+ */
+ boolean hasLock(Object lock);
+
+ /**
* @return true if options exist to suppress locking - false otherwise. Note that this is only used by the {@link org.jboss.cache.interceptors.PessimisticLockInterceptor}.
*/
boolean isLockingSuppressed();
Modified: core/trunk/src/main/java/org/jboss/cache/lock/MVCCLockManager.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/lock/MVCCLockManager.java 2008-07-23 14:23:32 UTC (rev 6377)
+++ core/trunk/src/main/java/org/jboss/cache/lock/MVCCLockManager.java 2008-07-23 15:00:08 UTC (rev 6378)
@@ -20,6 +20,7 @@
import java.util.Collection;
import java.util.Collections;
import java.util.List;
+import java.util.ListIterator;
import java.util.Set;
import static java.util.concurrent.TimeUnit.MILLISECONDS;
import java.util.concurrent.locks.Lock;
@@ -123,12 +124,12 @@
if (!locks.isEmpty())
{
// unlocking needs to be done in reverse order.
- Fqn[] fqns = new Fqn[locks.size()];
- fqns = locks.toArray(fqns);
- for (int i = fqns.length - 1; i > -1; i--)
+ ListIterator<Fqn> it = locks.listIterator(locks.size());
+ while (it.hasPrevious())
{
- if (trace) log.trace("Attempting to unlock " + fqns[i]);
- lockContainer.getLock(fqns[i]).unlock();
+ Fqn f = it.previous();
+ if (trace) log.trace("Attempting to unlock " + f);
+ lockContainer.getLock(f).unlock();
}
}
}
Modified: core/trunk/src/main/java/org/jboss/cache/lock/NodeBasedLockManager.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/lock/NodeBasedLockManager.java 2008-07-23 14:23:32 UTC (rev 6377)
+++ core/trunk/src/main/java/org/jboss/cache/lock/NodeBasedLockManager.java 2008-07-23 15:00:08 UTC (rev 6378)
@@ -12,6 +12,7 @@
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
+import java.util.ListIterator;
/**
* @author Mircea.Markus at jboss.com
@@ -119,14 +120,16 @@
if (locks.isEmpty()) return;
Object owner = getLockOwner(ctx);
- // Copying out to an array is faster than creating an ArrayList and iterating,
- // since list creation will just copy out to an array internally
- NodeLock[] lockArray = locks.toArray(new NodeLock[locks.size()]);
- for (int i = lockArray.length - 1; i >= 0; i--)
+
+ // unlocking needs to be done in reverse order.
+ ListIterator<NodeLock> it = locks.listIterator(locks.size());
+ while (it.hasPrevious())
{
+ NodeLock l = it.previous();
+
if (trace)
- log.trace("releasing lock for " + lockArray[i].getFqn() + " (" + lockArray[i] + "), owner " + owner);
- lockArray[i].release(owner);
+ log.trace("releasing lock for " + l.getFqn() + " (" + l + "), owner " + owner);
+ l.release(owner);
}
ctx.clearLocks();
Modified: core/trunk/src/main/java/org/jboss/cache/mvcc/MVCCNodeHelper.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/mvcc/MVCCNodeHelper.java 2008-07-23 14:23:32 UTC (rev 6377)
+++ core/trunk/src/main/java/org/jboss/cache/mvcc/MVCCNodeHelper.java 2008-07-23 15:00:08 UTC (rev 6378)
@@ -139,7 +139,7 @@
// lock which may be shared with another Fqn that we have a lock for already.
// nothing wrong, just means that we fail to record the lock. And that is a problem.
// Better to check our records and lock again if necessary.
- if (!ctx.getLocks().contains(fqn))
+ if (!ctx.hasLock(fqn))
{
if (!lockManager.lockAndRecord(fqn, WRITE, ctx))
{
Modified: core/trunk/src/main/java/org/jboss/cache/transaction/PessimisticTransactionContext.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/transaction/PessimisticTransactionContext.java 2008-07-23 14:23:32 UTC (rev 6377)
+++ core/trunk/src/main/java/org/jboss/cache/transaction/PessimisticTransactionContext.java 2008-07-23 15:00:08 UTC (rev 6378)
@@ -159,6 +159,10 @@
if (transactionLocks != null) transactionLocks.clear();
}
+ public boolean hasLock(Object lock)
+ {
+ return transactionLocks != null && transactionLocks.contains(lock);
+ }
@SuppressWarnings("unchecked")
public void addAllLocks(List newLocks)
Modified: core/trunk/src/main/java/org/jboss/cache/transaction/TransactionContext.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/transaction/TransactionContext.java 2008-07-23 14:23:32 UTC (rev 6377)
+++ core/trunk/src/main/java/org/jboss/cache/transaction/TransactionContext.java 2008-07-23 15:00:08 UTC (rev 6378)
@@ -156,6 +156,15 @@
List getLocks();
/**
+ * Most code could not use this method directly, but use {@link org.jboss.cache.invocation.InvocationContext#hasLock(Object)} ()} instead,
+ * which would delegate to this method if a transaction is in scope or otherwise use invocation-specific locks.
+ *
+ * @param lock lock to test
+ * @return true if the lock being tested is already held in the current scope, false otherwise.
+ */
+ boolean hasLock(Object lock);
+
+ /**
* Gets the value of the forceAsyncReplication flag. Used by ReplicationInterceptor and OptimisticReplicationInterceptor
* when dealing with {@link org.jboss.cache.Cache#putForExternalRead(org.jboss.cache.Fqn,Object,Object)} within
* a transactional context.
Modified: core/trunk/src/main/java/org/jboss/cache/util/ImmutableListCopy.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/util/ImmutableListCopy.java 2008-07-23 14:23:32 UTC (rev 6377)
+++ core/trunk/src/main/java/org/jboss/cache/util/ImmutableListCopy.java 2008-07-23 15:00:08 UTC (rev 6378)
@@ -204,13 +204,12 @@
@Override
public final ListIterator<E> listIterator()
{
- return listIterator(0);
+ return new ImmutableIterator();
}
@Override
public final ListIterator<E> listIterator(int index)
{
- assertIndexInRange(index);
return new ImmutableIterator(index);
}
@@ -231,12 +230,12 @@
ImmutableIterator(int index)
{
+ if (index < 0 || index > size()) throw new IndexOutOfBoundsException("Index: " + index);
cursor = index;
}
ImmutableIterator()
{
-
}
public boolean hasNext()
Added: core/trunk/src/test/java/org/jboss/cache/util/ImmutableListCopyTest.java
===================================================================
--- core/trunk/src/test/java/org/jboss/cache/util/ImmutableListCopyTest.java (rev 0)
+++ core/trunk/src/test/java/org/jboss/cache/util/ImmutableListCopyTest.java 2008-07-23 15:00:08 UTC (rev 6378)
@@ -0,0 +1,144 @@
+package org.jboss.cache.util;
+
+import org.testng.annotations.Test;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import java.util.ListIterator;
+
+ at Test(groups = "unit")
+public class ImmutableListCopyTest
+{
+ public void testImmutability()
+ {
+ List<String> l = new ImmutableListCopy<String>(Collections.singletonList("one"));
+ try
+ {
+ l.add("two");
+ assert false;
+ }
+ catch (UnsupportedOperationException good)
+ {
+
+ }
+
+ try
+ {
+ l.remove(0);
+ assert false;
+ }
+ catch (UnsupportedOperationException good)
+ {
+
+ }
+
+ try
+ {
+ l.clear();
+ assert false;
+ }
+ catch (UnsupportedOperationException good)
+ {
+
+ }
+
+ try
+ {
+ l.add(0, "x");
+ assert false;
+ }
+ catch (UnsupportedOperationException good)
+ {
+
+ }
+
+ try
+ {
+ l.set(0, "i");
+ assert false;
+ }
+ catch (UnsupportedOperationException good)
+ {
+
+ }
+
+ try
+ {
+ l.addAll(Collections.singletonList("l"));
+ assert false;
+ }
+ catch (UnsupportedOperationException good)
+ {
+
+ }
+
+ try
+ {
+ l.addAll(0, Collections.singletonList("l"));
+ assert false;
+ }
+ catch (UnsupportedOperationException good)
+ {
+
+ }
+
+ try
+ {
+ l.removeAll(Collections.singletonList("l"));
+ assert false;
+ }
+ catch (UnsupportedOperationException good)
+ {
+
+ }
+
+ try
+ {
+ l.retainAll(Collections.singletonList("l"));
+ assert false;
+ }
+ catch (UnsupportedOperationException good)
+ {
+
+ }
+
+ try
+ {
+ l.iterator().remove();
+ assert false;
+ }
+ catch (UnsupportedOperationException good)
+ {
+
+ }
+
+ try
+ {
+ l.listIterator().set("w");
+ assert false;
+ }
+ catch (UnsupportedOperationException good)
+ {
+
+ }
+ }
+
+ public void testListIterator()
+ {
+ List<Integer> ints = Arrays.asList(1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
+
+ List<Integer> list = new ImmutableListCopy<Integer>(ints);
+
+ ListIterator<Integer> li = list.listIterator();
+
+ int number = 1;
+ while (li.hasNext()) assert li.next() == number++;
+ assert number == 11;
+
+ number = 10;
+ li = list.listIterator(list.size());
+ while (li.hasPrevious()) assert li.previous() == number--;
+ assert number == 0;
+ }
+}
More information about the jbosscache-commits
mailing list