mailto:kabir.khan@jboss.com wrote:   > mailto:flavia.rainone@jboss.com wrote: > - why is it that calling get on the default domain returns a different version of the same class? This needs further investigation because it is clearly a bug Yes, this definitely needs fixing. But I thought we had tests for this already? e.g. this from SimpleDelegatingClassPoolTestCase. I added a few extra checks as indicated to make double sure      *public* *void* testOnePoolPerClassLoadedByA() *throws* Exception    {       ClassPoolDomain domain = createClassPoolDomain("SIMPLE", null, *false*);       ClassPool poolA = createDelegatingClassPool(domain, JAR_A_URL);       ClassPool poolB = createDelegatingClassPool(domain, JAR_B_URL);             //The first time we access the pool it will create the classes, second time will use the cache       accessOnePoolPerClassLoadedByA(poolA, poolB);       accessOnePoolPerClassLoadedByA(poolA, poolB);    }       private  *void* accessOnePoolPerClassLoadedByA(ClassPool poolA, ClassPool poolB) *throws* Exception    {       CtClass a = poolA.get(CLASS_A);       assertEquals(a, poolA.get(CLASS_A));//Added       assertEquals(a, poolB.get(CLASS_A));//Added       CtClass b = poolA.get(CLASS_B);       assertEquals(b, poolA.get(CLASS_B));//Added       assertEquals(b, poolB.get(CLASS_B));//Added       assertNotSame(a.getClassPool(), b.getClassPool());       assertEquals(poolA, a.getClassPool());       assertEquals(poolB, b.getClassPool());    }

mailto:kabir.khan@jboss.com wrote: > mailto:flavia.rainone@jboss.com wrote: >   > - why wasn't the classes cache being used by BaseClassPool.get before? Any reason for this? This has been copied from previous versions, I think that it definetly causes some overhead (the above should work, but it would be faster if the classpool finds out it has already created the class and returns the same class instead of delegating to the domain first). I can't see anything in BaseClassPool.get() or get0() regarding this? I think you mean DelegatingClassPool.get0()? I don't see the ClassPool.classes cache being used there either, so maybe I am looking in the wrong place. If I am in the right place, it might be an idea to try to load it locally in the initiating classpool first before hitting the domain if the cachedLookups == null.      //TODO KABIR was synchronized - I don't see why apart from that the standard javassist.ClassPool implementation was synchronized?    *public* *final* CtClass get0(String classname, *boolean* useCache) *throws* NotFoundException    {       //KABIR Made final just to make sure that cached lookups are only handled in one place.             *if* (cachedLookups != *null*)       {          CtClass cachedLookup = cachedLookups.get(classname, domain.getModCount());          *if* (cachedLookup != *null*)          {             logger.trace(classname + " was found in the cache of " + *this*);             *return* cachedLookup;          }       }       *else*       {          CtClass cached = getCachedLocally(classname);          *if* (cached != *null*)             *return* cached;       }            ...    }

mailto:kabir.khan@jboss.com wrote: > mailto:flavia.rainone@jboss.com wrote: > - if the cache starts being used as a first step of BaseClassPool.get, as it is in my local fix (not committed yet), is there need for a second level cache as Kabir wrote? Yes. It caches all CtClasses looked up by that classpool, and they might come from any classpool in the domain. The cacheLookups of all the classpools in the domain are invalidated when a classpool is added/removed to the domain or its parent domain. Basically, it means that if we look something up in a classpool, we don't need to do all the work in the domain.