[jboss-dev-forums] [Design the new POJO MicroContainer] - Re: ClassPool for JBoss Reflection

[flavia.rainone@jboss.com]

Kabir,

I have taken a very good look on all class pools, and I'm still having a hard time to figure out a few things.

Starting from javassist.ClassPool (http://www.docjar.com/html/api/javassist/ClassPool.java.html):
ClassPool allows you to retrieve a CtClass representing already existing classes through get and through getCtClass:
- getCtClass adds support to arrays and calls get
- get delegates to get0, plus increment some CtClass getCounter
- get0 uses childFirstLookup to determine whether it should try to retrieve the class from the parent domain before or after trying to retrieve itself. When trying to retrieve itself,it checks on whether the class is cached, if not, creates a new one by calling createCtClass, plus caches it.
Note that the caching of get0 is enabled or disabled according to the cache parameters. If cache is false, the ClassPool won't cache the result of createCtClass. Given get0 is an internal method, I've taken a look at all callers to see when is cache false and when is it true. It is false only when it is called by getAndRename, which IMO makes sense, given it is just a shortcut for creating a new class, so you don't want to cache the original one with a new name. So, we can assume that get0 will cache classes whenever needed.

Now, taking a look at ScopedClassPool:
http://www.docjar.com/html/api/javassist/scopedpool/ScopedClassPool.java.html
- it extends ClassPool
- adds a very good functionality: soften. This method allows that classes be moved from the hard reference classes cache to the softcache, that keeps soft references, allowing them to be gc'ed. This functionality is used by JBoss AOP to mark all classes that were not instrumented, thus preventing memory leak of non-instrumented CtClasses (the idea is that if they need to be created again, there won't be inconsistencies as no instrumentation has taken place). We could do the same for JBoss Reflection.
- cacheCtClass: complementing the soften functionality, this CP overwrites the cacheCtClass method, delegating to the superclass only if the class is being created dynamicly (i.e., it is a new class, that was not in the classpath), and adding it to softcache otherwise. 
- adds a ScopedClassPoolRepository, that contains several pools.
- overwrites internal getCached method, so that whenever you need a cached class it will also look into the classes of the other class pools
- adds a getCachedLocally, which just looks for classes at the "classes" cache (defined at the super class ClassPool) and at the "softcache' as well.
- overwrites the toClass method, so that it uses the getClassLoader0 (there is a commentary justifying why it is that way), plus it calls lockInCache, which I didn't understand why. (1) ***
- added lockInCache, which is just a shortcut for calling  super.cacheCtClass(c.getName(), c, false);
- added a close() method, that removes the cp from the repository
- added a getLocally method. This method basically removes a class from the softcache, creates it and caches it at the classes collection. I imagine that this method removes the class from the softcache for garanteeing consistency, so that there won't be two different versions of the same ctClass: one in the softcache and another one in the classes cache.

I figured that this ScopedClassPool and its repository were created to deal with UnifiedClassLoaders (let me know if I'm wrong). But now, the one million dollar question (2): why is it that getLocally was added to ScopedClassPool? Shouldn't ScopedClassPool had overwitten get0 instead? I see that this method is almost the same as the get0, except for two differences: it removes the class from the softcache (a much needed feature whenever a ct class is created) and it doesn't use childLookupFirst to delegate to the parent class loader. The result is that, IMO, by creating getLocally instead of overwriting get0, there are two ways of retrieving a ctclass, and the "official" way doesn't even handle the softcache stuff, creating an inconsistency. A way of solving but keeping getLocally would be overriding createCtClass like this:
softcache.remove(classname); super.createCtClass(...);

For the record, JBoss AOP never uses the official methods get and getCtClass... it uses the getLocally instead, thus avoiding the inconsistency mentioned. (3)

Now, we go to AOPClassPool:
- it extends ScopedClassPool
- getLocally: this method is overriden, but the strange thing is that the body of this method is a copy of the body of the super class, excepts for a few logging calls and except for teh fact that it replaces an invocation to ClassPool.cacheCtClass with a call to lockInCache, but they are equivalent. (4)
- getCached: the AOPClassPool overrides the ScopedClassPool.getCached method, thus overriding the single point of ScopedClassPool that uses the ScopedClassPoolRepository to delegate ctclass retrieval to other CPs of the same repository.
- there are several other details that I don't think are useful for my questions.

So, why does AOPClassPool extends ScopedClassPool if it doesn't uses the ScopedClassPoolRepository for retrieval of classes? (5)

Now, going to BaseClassPool (subclass of AOPClassPool), I see that it overrites createCtClass. It adds a call to lockInCache before calling super.createCtClass method. This generates another inconsistency for the get methods of ClassPool. Take a look at a part of get0 implementation:
  491         clazz = createCtClass(classname, useCache);
  |   492           if (clazz != null) {
  |   493               // clazz.getName() != classname if classname is "[L<name>;".
  |   494               if (useCache)
  |   495                   cacheCtClass(clazz.getName(), clazz, false);
  |   496   
  |   497               return clazz;
  |   498           }
As you can see, get0 adds the class to the cache by calling cacheCtClass after calling createCtClass. As I mentioned previously, cacheCtClass is overriden by ScopedClassPool, and adds the class to classes cache only if it is not generated dynamic. By adding the lockInCache call to the createCtClass method at BaseClassPool, you are generating a (6):
- memory leak if the class was generated dynamically. The class will be added to both softcache (by Scoped.cacheCtClass) and to classes (by BaseClassPool.cacheCtClass), thus avoiding it from being gc'ed.
- or a redundancy if the class was not generated dynamically, as the Scoped.cacheCtClass will already add the class to the classes cache.


Now, taking a look at NonDelegatingClassPool (subclass of BaseClassPool), I see that it overrides createCtClass, recreating the behaviour we had originally in the ClassPool. So, why is it a subclass of ScopedClassPool in the first place? (7)

(8) Plus, taking a look at DelegatingClassPool (subclass of BaseClassPool), I see it overrides the get0 method, but this method is never invoked by JBoss AOP if it uses getLocally. So, are the ClassPool.get/ClassPool.getCtClass being used from inside AS? If they are not, I don't see how are the new functionalities of the new ClassPools being used inside AS. If they are, then I see a point in overriding get0, but there is an inconsistency given that JBoss AOP uses the getLocally method, defined at AOPClass superclass.

So, Kabir, I'm obviously missing several links that will help me understand the class pools. So, please, comment on the numbered remarks and questions, and let me know what is it I'm missing here :)

*** I mean, when you do toClass, the class is already in the cache by now. Plus, what if somebody is doing a toClass with a class that is in the softcache? The lockInCache call will force the class being added as a hard reference to the "classes" cache, thus ruining the soften feature.

View the original post : http://www.jboss.org/index.html?module=bb&op=viewtopic&p=4238816#4238816

Reply to the post : http://www.jboss.org/index.html?module=bb&op=posting&mode=reply&p=4238816