[jbosscache-issues] [JBoss JIRA] (JBCACHE-394) Performance Improvements in CachedSetImpl

Performance Improvements in CachedSetImpl ----------------------------------------- Key: JBCACHE-394 URL: https://issues.jboss.org/browse/JBCACHE-394 Project: JBoss Cache Issue Type: Patch Security Level: Public(Everyone can see) Components: Legacy POJO Cache Reporter: Brian Stansberry Assignee: Scott Marlow Fix For: 1.3.0.GA Attachments: CachedSetImpl.diff, CachedSetImpl.java Jussi Pyörre posted a revised version of CachedSetImpl on the user forum. I haven't evaluated it, but am moving it to JIRA so we don't lose track of it. From the forum post: As I was running into severe performance problems with CachedSetImpl in TreeCacheAop, I wrote a new version with increased performance (can't remember the exact amount of improvement, but it was substantial anyway). This version has two main improvements: 1. The older version used size() method several times in next() method of the IteratorImpl. Since size() is a quite heavy operation in CachedSetImpl, the performance was poor. New version doesn't use size() for iterations at all. 2. The old version used ArrayList style indexing for node children names. This was only slowing things down. The new version uses the first available integer as a new child's name. For example, if first there were three children (0, 1 and 2) and '1' was destroyed, '0' and '2' would not get renamed. The next element addition would use '1' as its child name. As I am currently really busy, I unfortunately don't have time to get familiar with your ways of contributing to source code, so I'll just publish the new version here. So, here's the new CachedSetImpl: Code: /* * JBoss, the OpenSource J2EE webOS * * Distributable under LGPL license. * See terms of license at gnu.org. */ package org.jboss.cache.aop.collection; import org.jboss.cache.CacheException; import org.jboss.cache.Fqn; import org.jboss.cache.DataNode; import org.jboss.cache.aop.TreeCacheAop; import org.jboss.cache.aop.util.AopUtil; import org.jboss.util.NestedRuntimeException; import java.util.AbstractSet; import java.util.Collection; import java.util.Collections; import java.util.Iterator; import java.util.Map; import java.util.NoSuchElementException; import java.util.Set; /** * Set that uses cache as a underlying backend store * * @author Ben Wang, Jussi Pyörre */ public class CachedSetImpl extends AbstractSet { protected TreeCacheAop cache; protected AbstractCollectionInterceptor interceptor; public CachedSetImpl(TreeCacheAop cache, AbstractCollectionInterceptor interceptor) { this.cache = cache; this.interceptor = interceptor; } private Collection keySet() { DataNode node = getNode(); if (node != null) { Map children = node.getChildren(); if (children != null) { return children.keySet(); } } return Collections.EMPTY_SET; } private class IteratorImpl implements Iterator { private Iterator iterator; private Object key; private IteratorImpl(Collection keys) { iterator = keys.iterator(); } private IteratorImpl() { iterator = CachedSetImpl.this.keySet().iterator(); } public boolean hasNext() { return iterator.hasNext(); } public Object next() throws NoSuchElementException { if (!this.hasNext()) { throw new NoSuchElementException(); } this.key = iterator.next(); try { return cache .getObject(AopUtil.constructFqn(getFqn(), this.key)); } catch (CacheException e) { throw new RuntimeException(e); } } public void remove() throws IllegalStateException { if (this.key == null) { throw new IllegalStateException(); } try { cache.removeObject(AopUtil.constructFqn(getFqn(), this.key)); } catch (CacheException e) { throw new RuntimeException(e); } } } // protected static final Log log_=LogFactory.getLog(CachedSetImpl.class); protected DataNode getNode() { try { return cache.get(getFqn()); } catch (Exception e) { throw new NestedRuntimeException(e); } } protected Fqn getFqn() { // Need this since fqn can be reset. return interceptor.getFqn(); } public boolean add(Object o) { Collection keys = keySet(); // This could be done with 'contains(o)' but it would invoke 'keySet()' // twice for (Iterator iter = new IteratorImpl(keys); iter.hasNext();) if (iter.next() == o) return false; // Search first available key. This is a fast operation as the key set // is already available int key = 0; while (keys.contains(Integer.toString(key))) key++; try { cache.putObject(AopUtil.constructFqn(getFqn(), Integer .toString(key)), o); } catch (CacheException e) { throw new RuntimeException(e); } return true; } public Iterator iterator() { return new IteratorImpl(); } public int size() { return keySet().size(); } public boolean equals(Object o) { if (o == null) return false; if (o == this) return true; try { Set set = (Set) o; return (set.size() == keySet().size() && this.containsAll(set)); } catch (ClassCastException e) { return false; } } } It might need modification with older JDKs with rethrowing of catched exceptions (I use RuntimeException as you seem to throw NestedRuntimeException...). Anyway, that is a minor modification, if needed.