[jboss-user] [JBoss AOP Development] New message: "Re: ClassPool Refactoring"

[Kabir Khan]

User development,

A new message was posted in the thread "ClassPool Refactoring":

http://community.jboss.org/message/522340#522340

Author  : Kabir Khan
Profile : http://community.jboss.org/people/kabir.khan@jboss.com

Message:
--------------------------------------------------------------
> mailto:flavia.rainone at 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));
      assertEquals(a, poolB.get(CLASS_A));
      CtClass b = poolA.get(CLASS_B);
      assertEquals(b, poolA.get(CLASS_B));
      assertEquals(b, poolB.get(CLASS_B));
 
      assertNotSame(a.getClassPool(), b.getClassPool());
      assertEquals(poolA, a.getClassPool());
      assertEquals(poolB, b.getClassPool());
   }

> mailto:flavia.rainone at 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:flavia.rainone at 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.

--------------------------------------------------------------

To reply to this message visit the message page: http://community.jboss.org/message/522340#522340