Author: manik.surtani(a)jboss.com Date: 2008-11-26 15:16:12 -0500 (Wed, 26 Nov 2008) New Revision: 7199 Modified: core/trunk/src/main/java/org/jboss/cache/InvocationContext.java core/trunk/src/main/java/org/jboss/cache/interceptors/ActivationInterceptor.java core/trunk/src/main/java/org/jboss/cache/interceptors/LegacyActivationInterceptor.java core/trunk/src/test/java/org/jboss/cache/passivation/PassivationTestsBase.java Log: Optimize activations by minimizing calls to cacheloader.exists() Modified: core/trunk/src/main/java/org/jboss/cache/InvocationContext.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/InvocationContext.java 2008-11-25 12:26:47 UTC (rev 7198) +++ core/trunk/src/main/java/org/jboss/cache/InvocationContext.java 2008-11-26 20:16:12 UTC (rev 7199) @@ -69,6 +69,9 @@ @Deprecated private VisitableCommand command; + /** + * Set of Fqns loaded by the cache loader interceptor. Only recorded if needed, such as by the ActivationInterceptor + */ private Set<Fqn> fqnsLoaded; /** @@ -575,12 +578,20 @@ copy.transactionContext = transactionContext; } + /** + * Adds an Fqn to the set of Fqns loaded by the cache loader interceptor. Instantiates the set lazily if needed. + * + * @param fqn fqn to add + */ public void addFqnLoaded(Fqn fqn) { if (fqnsLoaded == null) fqnsLoaded = new HashSet<Fqn>(); fqnsLoaded.add(fqn); } + /** + * @return Set of Fqns loaded by the cache loader interceptor. Only recorded if needed, such as by the ActivationInterceptor + */ public Set<Fqn> getFqnsLoaded() { return fqnsLoaded; Modified: core/trunk/src/main/java/org/jboss/cache/interceptors/ActivationInterceptor.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/interceptors/ActivationInterceptor.java 2008-11-25 12:26:47 UTC (rev 7198) +++ core/trunk/src/main/java/org/jboss/cache/interceptors/ActivationInterceptor.java 2008-11-26 20:16:12 UTC (rev 7199) @@ -24,14 +24,10 @@ import org.jboss.cache.Fqn; import org.jboss.cache.InternalNode; import org.jboss.cache.InvocationContext; -import org.jboss.cache.Modification; -import org.jboss.cache.commands.AbstractVisitor; import org.jboss.cache.commands.read.GetChildrenNamesCommand; import org.jboss.cache.commands.read.GetKeyValueCommand; import org.jboss.cache.commands.read.GetKeysCommand; import org.jboss.cache.commands.read.GetNodeCommand; -import org.jboss.cache.commands.tx.OptimisticPrepareCommand; -import org.jboss.cache.commands.tx.PrepareCommand; import org.jboss.cache.commands.write.ClearDataCommand; import org.jboss.cache.commands.write.MoveCommand; import org.jboss.cache.commands.write.PutDataMapCommand; @@ -40,17 +36,12 @@ import org.jboss.cache.commands.write.RemoveKeyCommand; import org.jboss.cache.commands.write.RemoveNodeCommand; import org.jboss.cache.factories.annotations.Inject; -import org.jboss.cache.factories.annotations.Start; import org.jboss.cache.jmx.annotations.ManagedAttribute; import org.jboss.cache.jmx.annotations.ManagedOperation; -import org.jboss.cache.transaction.GlobalTransaction; -import org.jboss.cache.transaction.TransactionContext; +import org.jboss.cache.mvcc.ReadCommittedNode; -import javax.transaction.SystemException; import javax.transaction.TransactionManager; -import java.util.ArrayList; import java.util.HashMap; -import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; @@ -68,7 +59,6 @@ protected TransactionManager txMgr = null; private long activations = 0; - ActivationModificationsBuilder builder; /** * List<Transaction> that we have registered for @@ -88,12 +78,6 @@ this.txMgr = txMgr; } - @Start - public void createModificationsBuilder() - { - builder = new ActivationModificationsBuilder(); - } - @Override public Object visitClearDataCommand(InvocationContext ctx, ClearDataCommand command) throws Throwable { @@ -210,7 +194,7 @@ if (fqn != null && wasLoadedIntoMemory(ctx, fqn)) { InternalNode n; - if (((n = dataContainer.peekInternalNode(fqn, true)) != null) && n.isDataLoaded() && loader.exists(fqn)) + if (((n = findNode(ctx, fqn)) != null) && n.isDataLoaded() && loader.exists(fqn)) { // node not null and attributes have been loaded? if (n.hasChildren()) @@ -231,6 +215,19 @@ } } + private InternalNode findNode(InvocationContext ctx, Fqn fqn) + { + ReadCommittedNode n = (ReadCommittedNode) ctx.lookUpNode(fqn); + if (n == null || n.isNullNode()) + { + return dataContainer.peekInternalNode(fqn, true); + } + else + { + return n.getDelegationTarget(); + } + } + private boolean childrenLoaded(InternalNode<?, ?> node) { if (!node.isChildrenLoaded()) @@ -260,33 +257,6 @@ } - @Override - public Object visitOptimisticPrepareCommand(InvocationContext ctx, OptimisticPrepareCommand command) throws Throwable - { - Object retval = invokeNextInterceptor(ctx, command); - if (inTransaction()) - { - prepareCacheLoader(ctx); - } - return retval; - } - - private boolean inTransaction() throws SystemException - { - return txMgr != null && txMgr.getTransaction() != null; - } - - @Override - public Object visitPrepareCommand(InvocationContext ctx, PrepareCommand command) throws Throwable - { - Object retval = invokeNextInterceptor(ctx, command); - if (inTransaction()) - { - prepareCacheLoader(ctx); - } - return retval; - } - private void remove(Fqn fqn) throws Exception { loader.remove(fqn); @@ -311,110 +281,6 @@ } } - private void prepareCacheLoader(InvocationContext ctx) throws Throwable - { - GlobalTransaction gtx = ctx.getGlobalTransaction(); - TransactionContext tCtx = ctx.getTransactionContext(); - if (tCtx == null) - { - throw new Exception("tCtx for transaction " + gtx + " not found in transaction table"); - } - List<Modification> cacheLoaderModifications = new ArrayList<Modification>(); - - builder.visitCollection(ctx, tCtx.getModifications()); - if (cacheLoaderModifications.size() > 0) - { - loader.prepare(gtx, cacheLoaderModifications, false); - } - } - - public class ActivationModificationsBuilder extends AbstractVisitor - { - - private List<Modification> cacheLoaderModifications = new ArrayList<Modification>(); - - private int txActs = 0; - - @Override - public Object visitRemoveNodeCommand(InvocationContext ctx, RemoveNodeCommand removeNodeCommand) throws Throwable - { - Modification mod = new Modification(Modification.ModificationType.REMOVE_NODE, removeNodeCommand.getFqn()); - cacheLoaderModifications.add(mod); - return null; - } - - @Override - public Object visitPutDataMapCommand(InvocationContext ctx, PutDataMapCommand command) throws Throwable - { - Fqn fqn = command.getFqn(); - handlePutCommand(ctx, fqn); - return null; - } - - @Override - public Object visitPutKeyValueCommand(InvocationContext ctx, PutKeyValueCommand command) throws Throwable - { - Fqn fqn = command.getFqn(); - handlePutCommand(ctx, fqn); - return null; - } - - // On the way out, remove the node from the cache loader. - // Only remove the node if it exists in memory, its attributes have - // been initialized, its children have been loaded - // AND it was found in the cache loader (nodeLoaded = true). - // Then notify the listeners that the node has been activated. - private void handlePutCommand(InvocationContext ctx, Fqn fqn) - throws Exception - { - if (fqn != null && dataContainer.exists(fqn) && loader.exists(fqn)) - { - InternalNode n = dataContainer.peekInternalNode(fqn, true);// don't load - // node not null and attributes have been loaded? - if (n != null && n.isDataLoaded()) - { - // has children? - boolean result = childrenLoaded(n); - if (n.hasChildren() && result) - { - // children have been loaded, remove the node - addRemoveMod(ctx, cacheLoaderModifications, fqn, n.getData()); - txActs++; - } - // doesn't have children, check the cache loader - else if (loaderNoChildren(fqn)) - { - addRemoveMod(ctx, cacheLoaderModifications, fqn, n.getData()); - txActs++; - } - } - } - } - - private boolean loaderNoChildren(Fqn fqn) throws Exception - { - return loader.getChildrenNames(fqn) != null; - } - - private void addRemoveMod(InvocationContext ctx, List<Modification> l, Fqn fqn, Map data) - { - Modification mod = new Modification(Modification.ModificationType.REMOVE_NODE, fqn); - l.add(mod); - notifier.notifyNodeActivated(fqn, false, data, ctx); - } - - public List<Modification> getCacheLoaderModifications() - { - return cacheLoaderModifications; - } - - public int getTxActs() - { - return txActs; - } - - } - @ManagedAttribute(description = "number of cache node activations") public long getActivations() { Modified: core/trunk/src/main/java/org/jboss/cache/interceptors/LegacyActivationInterceptor.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/interceptors/LegacyActivationInterceptor.java 2008-11-25 12:26:47 UTC (rev 7198) +++ core/trunk/src/main/java/org/jboss/cache/interceptors/LegacyActivationInterceptor.java 2008-11-26 20:16:12 UTC (rev 7199) @@ -23,15 +23,11 @@ import org.jboss.cache.Fqn; import org.jboss.cache.InvocationContext; -import org.jboss.cache.Modification; import org.jboss.cache.NodeSPI; -import org.jboss.cache.commands.AbstractVisitor; import org.jboss.cache.commands.read.GetChildrenNamesCommand; import org.jboss.cache.commands.read.GetKeyValueCommand; import org.jboss.cache.commands.read.GetKeysCommand; import org.jboss.cache.commands.read.GetNodeCommand; -import org.jboss.cache.commands.tx.OptimisticPrepareCommand; -import org.jboss.cache.commands.tx.PrepareCommand; import org.jboss.cache.commands.write.ClearDataCommand; import org.jboss.cache.commands.write.MoveCommand; import org.jboss.cache.commands.write.PutDataMapCommand; @@ -40,16 +36,10 @@ import org.jboss.cache.commands.write.RemoveKeyCommand; import org.jboss.cache.commands.write.RemoveNodeCommand; import org.jboss.cache.factories.annotations.Inject; -import org.jboss.cache.factories.annotations.Start; import org.jboss.cache.jmx.annotations.ManagedOperation; -import org.jboss.cache.transaction.GlobalTransaction; -import org.jboss.cache.transaction.TransactionContext; -import javax.transaction.SystemException; import javax.transaction.TransactionManager; -import java.util.ArrayList; import java.util.HashMap; -import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; @@ -69,7 +59,6 @@ protected TransactionManager txMgr = null; private long activations = 0; - ActivationModificationsBuilder builder; /** * List<Transaction> that we have registered for @@ -89,12 +78,6 @@ this.txMgr = txMgr; } - @Start - public void createModificationsBuilder() - { - builder = new ActivationModificationsBuilder(); - } - @Override public Object visitClearDataCommand(InvocationContext ctx, ClearDataCommand command) throws Throwable { @@ -262,33 +245,6 @@ } - @Override - public Object visitOptimisticPrepareCommand(InvocationContext ctx, OptimisticPrepareCommand command) throws Throwable - { - Object retval = invokeNextInterceptor(ctx, command); - if (inTransaction()) - { - prepareCacheLoader(ctx); - } - return retval; - } - - private boolean inTransaction() throws SystemException - { - return txMgr != null && txMgr.getTransaction() != null; - } - - @Override - public Object visitPrepareCommand(InvocationContext ctx, PrepareCommand command) throws Throwable - { - Object retval = invokeNextInterceptor(ctx, command); - if (inTransaction()) - { - prepareCacheLoader(ctx); - } - return retval; - } - private void remove(Fqn fqn) throws Exception { loader.remove(fqn); @@ -318,110 +274,6 @@ return activations; } - private void prepareCacheLoader(InvocationContext ctx) throws Throwable - { - GlobalTransaction gtx = ctx.getGlobalTransaction(); - TransactionContext tCtx = ctx.getTransactionContext(); - if (tCtx == null) - { - throw new Exception("tCtx for transaction " + gtx + " not found in transaction table"); - } - List<Modification> cacheLoaderModifications = new ArrayList<Modification>(); - - builder.visitCollection(ctx, tCtx.getModifications()); - if (cacheLoaderModifications.size() > 0) - { - loader.prepare(gtx, cacheLoaderModifications, false); - } - } - - public class ActivationModificationsBuilder extends AbstractVisitor - { - - private List<Modification> cacheLoaderModifications = new ArrayList<Modification>(); - - private int txActs = 0; - - @Override - public Object visitRemoveNodeCommand(InvocationContext ctx, RemoveNodeCommand removeNodeCommand) throws Throwable - { - Modification mod = new Modification(Modification.ModificationType.REMOVE_NODE, removeNodeCommand.getFqn()); - cacheLoaderModifications.add(mod); - return null; - } - - @Override - public Object visitPutDataMapCommand(InvocationContext ctx, PutDataMapCommand command) throws Throwable - { - Fqn fqn = command.getFqn(); - handlePutCommand(ctx, fqn); - return null; - } - - @Override - public Object visitPutKeyValueCommand(InvocationContext ctx, PutKeyValueCommand command) throws Throwable - { - Fqn fqn = command.getFqn(); - handlePutCommand(ctx, fqn); - return null; - } - - // On the way out, remove the node from the cache loader. - // Only remove the node if it exists in memory, its attributes have - // been initialized, its children have been loaded - // AND it was found in the cache loader (nodeLoaded = true). - // Then notify the listeners that the node has been activated. - private void handlePutCommand(InvocationContext ctx, Fqn fqn) - throws Exception - { - if (fqn != null && dataContainer.peek(fqn, false, false) != null && loader.exists(fqn)) - { - NodeSPI n = dataContainer.peek(fqn, true, false);// don't load - // node not null and attributes have been loaded? - if (n != null && n.isDataLoaded()) - { - // has children? - boolean result = childrenLoaded(n); - if (!n.getChildrenDirect().isEmpty() && result) - { - // children have been loaded, remove the node - addRemoveMod(ctx, cacheLoaderModifications, fqn, n.getDataDirect()); - txActs++; - } - // doesn't have children, check the cache loader - else if (loaderNoChildren(fqn)) - { - addRemoveMod(ctx, cacheLoaderModifications, fqn, n.getDataDirect()); - txActs++; - } - } - } - } - - private boolean loaderNoChildren(Fqn fqn) throws Exception - { - return loader.getChildrenNames(fqn) != null; - } - - private void addRemoveMod(InvocationContext ctx, List<Modification> l, Fqn fqn, Map data) - { - Modification mod = new Modification(Modification.ModificationType.REMOVE_NODE, fqn); - l.add(mod); - notifier.notifyNodeActivated(fqn, false, data, ctx); - } - - public List<Modification> getCacheLoaderModifications() - { - return cacheLoaderModifications; - } - - public int getTxActs() - { - return txActs; - } - - } - @ManagedOperation public void resetStatistics() { Modified: core/trunk/src/test/java/org/jboss/cache/passivation/PassivationTestsBase.java =================================================================== --- core/trunk/src/test/java/org/jboss/cache/passivation/PassivationTestsBase.java 2008-11-25 12:26:47 UTC (rev 7198) +++ core/trunk/src/test/java/org/jboss/cache/passivation/PassivationTestsBase.java 2008-11-26 20:16:12 UTC (rev 7199) @@ -8,6 +8,7 @@ import org.jboss.cache.Modification; import org.jboss.cache.Node; import org.jboss.cache.NodeSPI; +import org.jboss.cache.UnitTestCacheFactory; import org.jboss.cache.loader.AbstractCacheLoaderTestBase; import org.jboss.cache.loader.CacheLoader; import org.jboss.cache.loader.SamplePojo; @@ -30,7 +31,6 @@ import java.util.List; import java.util.Map; import java.util.Set; -import org.jboss.cache.UnitTestCacheFactory; /** * Base tests for passivation using any of the cache loaders @@ -54,17 +54,17 @@ { CacheSPI<Object, Object> cache = (CacheSPI<Object, Object>) new UnitTestCacheFactory<Object, Object>().createCache(false); cache.getConfiguration().setCacheMode("local"); - + cacheTL.set(cache); - + configureCache(); cache.getConfiguration().setTransactionManagerLookupClass("org.jboss.cache.transaction.DummyTransactionManagerLookup"); cache.getConfiguration().setIsolationLevel(IsolationLevel.SERIALIZABLE); cache.create(); cache.start(); - CacheLoader loader = cache.getCacheLoaderManager().getCacheLoader(); - - loaderTL.set(loader); + CacheLoader loader = cache.getCacheLoaderManager().getCacheLoader(); + + loaderTL.set(loader); } abstract protected void configureCache() throws Exception; @@ -75,7 +75,7 @@ { CacheSPI<Object, Object> cache = cacheTL.get(); CacheLoader loader = loaderTL.get(); - + log.info("**** TEARING DOWN ****"); if (loader != null) loader.remove(Fqn.ROOT); TestingUtil.killCaches(cache); @@ -264,7 +264,7 @@ public void testCacheLoading2() throws Exception { - CacheSPI<Object, Object> cache = cacheTL.get(); + CacheSPI<Object, Object> cache = cacheTL.get(); Set<Object> keys = null; cache.put("/a/b/c", "key", "val"); @@ -276,7 +276,7 @@ public void testExists() throws Exception { - CacheSPI<Object, Object> cache = cacheTL.get(); + CacheSPI<Object, Object> cache = cacheTL.get(); cache.put("/eins/zwei/drei", "key1", "val1"); assert (exists("/eins/zwei/drei")); @@ -805,8 +805,8 @@ assertTrue(children.contains("3")); mgr.commit(); } - - + + public void testPutDataMapAfterPassivation() throws Exception { CacheSPI<Object, Object> cache = cacheTL.get(); @@ -814,24 +814,24 @@ Fqn f = Fqn.fromString("/a"); assert !cache.exists(f); - assert !loader.exists(f); - + assert !loader.exists(f); + Map<Object, Object> input = new HashMap(); input.put("one", "one"); input.put("two", "two"); cache.put(f, input); - + cache.evict(f); - + input = new HashMap(); - input.put("one", "oneA"); + input.put("one", "oneA"); cache.put(f, input); - + Map data = cache.getRoot().getChild(f).getData(); assertEquals("incorrect # of entries", 2, data.size()); assertEquals("Has key 'one", "oneA", data.get("one")); assertEquals("Has key 'two", "two", data.get("two")); - + } @@ -864,7 +864,41 @@ cache.removeNode(Fqn.ROOT); } + public void testTxReadCommit() throws Exception + { + CacheSPI<Object, Object> cache = cacheTL.get(); + CacheLoader loader = loaderTL.get(); + DummyTransactionManager mgr = DummyTransactionManager.getInstance(); + mgr.begin(); + + cache.put("/one/two/three", "key1", "val1"); + cache.put("/one/two/three/four", "key2", "val2"); + + mgr.commit(); + + assertNotNull(cache.getNode("/one/two/three").getKeys()); + assertEquals("val1", cache.get(Fqn.fromString("/one/two/three"), "key1")); + mgr.begin(); + + cache.evict(Fqn.fromString("/one/two/three")); + cache.evict(Fqn.fromString("/one/two/three/four")); + + mgr.commit(); + assertTrue(loader.exists(Fqn.fromString("/one/two/three"))); + assertTrue(loader.exists(Fqn.fromString("/one/two/three/four"))); + + // now do a READ in a TX + mgr.begin(); + assert cache.get("/one/two/three", "key1").equals("val1"); + assert cache.get("/one/two/three/four", "key2").equals("val2"); + mgr.commit(); + + // these should NOT exist in the CL anymore! + assert !loader.exists(Fqn.fromString("/one/two/three/four")); + } + + public void testTxPutRollback() throws Exception { CacheSPI<Object, Object> cache = cacheTL.get();