User development,

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

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

Author  : Flavia Rainone
Profile : http://community.jboss.org/people/[email protected]

Message:
--------------------------------------------------------------
> mailto:[email protected] wrote:
>  
> > mailto:[email protected] 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());
>    }
>  
> 
> 
> 
    For some reason yet to be found, those tests do not replicate what is 
happening in the tests in AS. I'll have to do more debugging to find out 
exactly what is going on and then add any tests to jboss-classpool if needed.
 
 
> mailto:[email protected] wrote:
> > mailto:[email protected] 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;
>       }
>       
>      ...
>    }
> 
That's the point  I'm saying that, IMO, the first thing that BaseClassPool.get 
should do is looking in the cache. Currently, it doesn't. That has been done 
only locally in my machine, and fixed the failing tests. I haven't comitted 
anything because none of this is 100% validated yet.
 
> mailto:[email protected] wrote:
> > mailto:[email protected] 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.
  I see now. This looks nice . Testing the cache in AS is next in my list.


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

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


_______________________________________________
jboss-user mailing list
[email protected]
https://lists.jboss.org/mailman/listinfo/jboss-user

Reply via email to