Author: manik.surtani(a)jboss.com Date: 2008-02-05 20:49:23 -0500 (Tue, 05 Feb 2008) New Revision: 5308 Modified: core/trunk/src/main/java/org/jboss/cache/NodeSPI.java core/trunk/src/main/java/org/jboss/cache/UnversionedNode.java core/trunk/src/main/java/org/jboss/cache/interceptors/PessimisticLockInterceptor.java core/trunk/src/main/java/org/jboss/cache/invocation/NodeInvocationDelegate.java Log: Used indexes rather than Fqn comparison to determine when the lock loop completes. Modified: core/trunk/src/main/java/org/jboss/cache/NodeSPI.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/NodeSPI.java 2008-02-06 01:48:37 UTC (rev 5307) +++ core/trunk/src/main/java/org/jboss/cache/NodeSPI.java 2008-02-06 01:49:23 UTC (rev 5308) @@ -259,6 +259,18 @@ NodeSPI<K, V> addChildDirect(Fqn f, boolean notify); /** + * Same as {@link #addChildDirect(Fqn, boolean)} except that it just takes a child name + * + * @param childName name of child + * @param notify if true, notification events are sent; if false, they are not + * @return child node + * @throws org.jboss.cache.lock.LockingException + * if locking was not obtained + * @see #addChild(Fqn) + */ + NodeSPI<K, V> addChildDirect(Object childName, boolean notify); + + /** * Directly adds the node passed in to the children map of the current node. Will throw a CacheException if * <tt>child.getFqn().getParent().equals(getFqn())</tt> returns false. * Modified: core/trunk/src/main/java/org/jboss/cache/UnversionedNode.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/UnversionedNode.java 2008-02-06 01:48:37 UTC (rev 5307) +++ core/trunk/src/main/java/org/jboss/cache/UnversionedNode.java 2008-02-06 01:49:23 UTC (rev 5308) @@ -452,6 +452,12 @@ } + public NodeSPI addChildDirect(Object childName, boolean notify) + { + GlobalTransaction gtx = cache.getInvocationContext().getGlobalTransaction(); + return getOrCreateChild(childName, gtx, true, notify); + } + public void clearDataDirect() { if (data != null) data.clear(); Modified: core/trunk/src/main/java/org/jboss/cache/interceptors/PessimisticLockInterceptor.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/interceptors/PessimisticLockInterceptor.java 2008-02-06 01:48:37 UTC (rev 5307) +++ core/trunk/src/main/java/org/jboss/cache/interceptors/PessimisticLockInterceptor.java 2008-02-06 01:49:23 UTC (rev 5308) @@ -14,7 +14,10 @@ import org.jboss.cache.config.Configuration; import org.jboss.cache.factories.annotations.ComponentName; import org.jboss.cache.factories.annotations.Inject; -import org.jboss.cache.lock.*; +import org.jboss.cache.lock.IsolationLevel; +import org.jboss.cache.lock.LockingException; +import org.jboss.cache.lock.NodeLock; +import org.jboss.cache.lock.TimeoutException; import org.jboss.cache.marshall.MethodDeclarations; import org.jboss.cache.transaction.GlobalTransaction; import org.jboss.cache.transaction.TransactionEntry; @@ -80,14 +83,16 @@ try { return super.invoke(ctx); - } catch (LockingException le) + } + catch (LockingException le) { - if (trace) log.trace("Locking exception occured, cleaning up locks." , le); + if (trace) log.trace("Locking exception occured, cleaning up locks.", le); releaseLocks(ctx); throw le; - } catch (TimeoutException te) + } + catch (TimeoutException te) { - if (trace) log.trace("Locking exception occured, cleaning up locks." , te); + if (trace) log.trace("Locking exception occured, cleaning up locks.", te); releaseLocks(ctx); throw te; } @@ -103,9 +108,10 @@ if (trace) log.trace("Releasing existing locks. Global tx?" + gtx); if (gtx != null) { - TransactionEntry te = cache.getTransactionTable().get(gtx); + TransactionEntry te = cache.getTransactionTable().get(gtx); te.releaseAllLocksFIFO(gtx); - } else + } + else { Thread currentThread = Thread.currentThread(); List<NodeLock> locks = getLocks(currentThread); @@ -113,7 +119,7 @@ { aLock.release(currentThread); } - } + } } @@ -284,7 +290,8 @@ { if (trace) log.trace("There were new nodes created, skiping notification on delete"); Object[] args = ctx.getMethodCall().getArgs(); - if (trace) log.trace("Changing 'skipNotification' for method '_remove' from " + args[args.length - 1] + " to true"); + if (trace) + log.trace("Changing 'skipNotification' for method '_remove' from " + args[args.length - 1] + " to true"); args[args.length - 1] = Boolean.TRUE; } @@ -403,10 +410,10 @@ * 2) acquireWriteLockOnParent is true. If so AND {@link org.jboss.cache.Node#isLockForChildInsertRemove()} then a read * lock will be aquired for the parent of the node. * - * @param createIfNotExists if true, then missing nodes will be cretaed on the fly. If false, method returns if we - * reach a node that does not exists - * @param reverseRemoveCheck see {@link #manageReverseRemove(org.jboss.cache.transaction.GlobalTransaction, org.jboss.cache.NodeSPI, boolean)} - * @param createdNodes a list to which any nodes created can register their Fqns so that calling code is aware of which nodes have been newly created. + * @param createIfNotExists if true, then missing nodes will be cretaed on the fly. If false, method returns if we + * reach a node that does not exists + * @param reverseRemoveCheck see {@link #manageReverseRemove(org.jboss.cache.transaction.GlobalTransaction, org.jboss.cache.NodeSPI, boolean)} + * @param createdNodes a list to which any nodes created can register their Fqns so that calling code is aware of which nodes have been newly created. * @param skipNotification */ private boolean lock(InvocationContext ctx, Fqn fqn, NodeLock.LockType lockType, boolean createIfNotExists, long timeout, @@ -429,6 +436,8 @@ NodeSPI parent = null; Object childName = null; int currentIndex = -1; + int targetFqnSize = fqn.size(); + do { if (currentNode == null) @@ -436,7 +445,7 @@ if (createIfNotExists) { // if the new node is to be marked as deleted, do not notify! - currentNode = parent.addChildDirect(new Fqn(childName), !skipNotification); + currentNode = parent.addChildDirect(childName, !skipNotification); created = true; if (trace) log.trace("Child node was null, so created child node " + childName); if (createdNodes != null) createdNodes.add(currentNode); @@ -452,6 +461,7 @@ { if (!currentNode.isValid() && createIfNotExists) currentNode.setValid(true, false); } + NodeLock.LockType lockTypeRequired = NodeLock.LockType.READ; if (created || writeLockNeeded(ctx, lockType, currentIndex, acquireWriteLockOnParent, createIfNotExists, fqn, currentNode)) { @@ -479,8 +489,10 @@ // crap! if (trace) log.trace("Parent has been deleted again. Go through the lock method all over again."); currentNode = rootNode; + currentIndex = -1; parent = null; - } else + } + else { currentNode = parent; currentIndex--; @@ -491,13 +503,19 @@ } if (trace) log.trace("Moving one level up, current node is :" + currentNode); } - } else + } + else { - if (currentNodeFqn.equals(fqn))//we've just processed the last child + // we have succeeded in acquiring this lock. Increment the current index since we have gained one level of depth in the tree. + currentIndex++; + + // now test if this is the final level and if we can quit the loop: + //if (currentNodeFqn.equals(fqn))//we've just processed the last child + if (currentIndex == targetFqnSize) { break; } - if (!fqn.isChildOrEquals(currentNode.getFqn())) + if (!fqn.isChildOrEquals(currentNode.getFqn())) // Does this ever happen? Perhaps with a move(), I suppose? - MS { String message = new StringBuffer("currentNode instance changed the FQN(").append(currentNode.getFqn()) .append(") and do not match the FQN on which we want to acquire lock(").append(fqn).append(")").toString(); @@ -505,9 +523,9 @@ throw new LockingException(message); } parent = currentNode; - currentIndex = currentNodeFqn.size(); - currentNode = currentNode.getChildDirect(fqn.get(currentIndex)); + childName = fqn.get(currentIndex); + currentNode = currentNode.getChildDirect(childName); } } while (true); return created; @@ -616,7 +634,7 @@ */ private void manageReverseRemove(GlobalTransaction gtx, NodeSPI childNode, boolean reverseRemoveCheck) { - boolean needToReverseRemove = reverseRemoveCheck && childNode.isDeleted() && tx_table.isNodeRemovedInTx(gtx,childNode.getFqn()); + boolean needToReverseRemove = reverseRemoveCheck && childNode.isDeleted() && tx_table.isNodeRemovedInTx(gtx, childNode.getFqn()); if (gtx != null && needToReverseRemove) { childNode.markAsDeleted(false); Modified: core/trunk/src/main/java/org/jboss/cache/invocation/NodeInvocationDelegate.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/invocation/NodeInvocationDelegate.java 2008-02-06 01:48:37 UTC (rev 5307) +++ core/trunk/src/main/java/org/jboss/cache/invocation/NodeInvocationDelegate.java 2008-02-06 01:49:23 UTC (rev 5308) @@ -162,6 +162,11 @@ return node.addChildDirect(f, notify); } + public NodeSPI<K, V> addChildDirect(Object childName, boolean notify) + { + return node.addChildDirect(childName, notify); + } + public void addChildDirect(NodeSPI<K, V> child) { node.addChildDirect(child);