[flavia.rainone]

Kabir Khan wrote:

I will try using RepositoryClassPoolFactory instead and modify AS startup as required.

Let me know if you find any pieces missing or non-working stuff.

[kabirkhan]

I have modified the bootstrap project as shown in the attached patch to set JavassistTypeInfoFactoryImpl's classPoolFactory to be RepositoryClassPoolFactory. A quick check in the debugger shows this to kick in.

I am now getting some exceptions on startup that I need to look into.

[kabirkhan]

Kabir Khan wrote:

I have modified the bootstrap project as shown in the attached patch to set JavassistTypeInfoFactoryImpl's classPoolFactory to be RepositoryClassPoolFactory. A quick check in the debugger shows this to kick in.

I am now getting some exceptions on startup that I need to look into.

I modified JavassistTypeInfoFactoryImpl to give some extra output. When a class can not be found it ends up in this method:

   private TypeInfo delegateToIntrospectionImplementation(ClassLoader cl, String name) throws ClassNotFoundException
   {
      System.out.println("======> " + name + " " + cl);

      //Extra code just for debugging
      ClassPool pool = poolFactory.getPoolForLoader(cl);
      try
      {
         CtClass ct = pool.get(name); 
      }
      catch(Exception alreadyHandled)
      {
         System.out.println("---> Not found in " + pool);
      }

      Class<?> clazz = cl.loadClass(name);
      System.out.println("---> Loaded real class from " + clazz.getClassLoader());

      try
      {
         CtClass ct = pool.get(name); 
      }
      catch(Exception alreadyHandled)
      {
         System.out.println("---> Not found again in " + pool);
      }

      pool = poolFactory.getPoolForLoader(clazz.getClassLoader());
      try
      {
         CtClass ct = pool.get(name); 
         System.out.println("---> Found in actual pool " + pool);
      }
      catch(Exception alreadyHandled)
      {
         System.out.println("---> Not found in actual pool " + pool);
      }


      //Extra code - END

      IntrospectionTypeInfoFactory factory = new IntrospectionTypeInfoFactory();
      return factory.getTypeInfo(name, cl);
   }

Here is the output, this is during installing the beans from conf/bootstrap

17:16:06,954 INFO  [JMXKernel] Legacy JMX core initialized
17:16:07,009 INFO  [STDOUT] ======> org.jboss.aop.deployers.AspectManagerJMXRegistrar BaseClassLoader@47ac1adf{jmx-classloader:0.0.0$MODULE}
17:16:08,927 INFO  [STDOUT] ---> Not found in [org.jboss.classpool.plugins.jbosscl.JBossClDelegatingClassPool@2002512083 [class path: BaseClassLoader@47ac1adf{jmx-classloader:0.0.0$MODULE}:] - dcl:BaseClassLoader@47ac1adf{jmx-classloader:0.0.0$MODULE} domain: [org.jboss.classpool.plugins.jbosscl.JBossClClassPoolDomain@101ebf5c name:DefaultDomain]]
17:16:08,930 INFO  [STDOUT] ---> Loaded real class from BaseClassLoader@8a85268{aop-classloader:0.0.0$MODULE}
17:16:08,931 INFO  [STDOUT] ---> Not found again in [org.jboss.classpool.plugins.jbosscl.JBossClDelegatingClassPool@2002512083 [class path: BaseClassLoader@47ac1adf{jmx-classloader:0.0.0$MODULE}:] - dcl:BaseClassLoader@47ac1adf{jmx-classloader:0.0.0$MODULE} domain: [org.jboss.classpool.plugins.jbosscl.JBossClClassPoolDomain@101ebf5c name:DefaultDomain]]
17:16:08,931 INFO  [STDOUT] ---> Found in actual pool org.jboss.classpool.spi.AbstractClassPool@697579067 [class path: BaseClassLoader@8a85268{aop-classloader:0.0.0$MODULE}:] - dcl:BaseClassLoader@8a85268{aop-classloader:0.0.0$MODULE}
17:16:09,563 INFO  [STDOUT] ======> org.jboss.aop.asintegration.jboss5.AOPAnnotationMetaDataParserDeployer BaseClassLoader@280bca{deployers-classloader:0.0.0$MODULE}
17:16:10,283 INFO  [STDOUT] ---> Not found in [org.jboss.classpool.plugins.jbosscl.JBossClDelegatingClassPool@404225673 [class path: BaseClassLoader@280bca{deployers-classloader:0.0.0$MODULE}:] - dcl:BaseClassLoader@280bca{deployers-classloader:0.0.0$MODULE} domain: [org.jboss.classpool.plugins.jbosscl.JBossClClassPoolDomain@101ebf5c name:DefaultDomain]]
17:16:10,287 INFO  [STDOUT] ---> Loaded real class from BaseClassLoader@8a85268{aop-classloader:0.0.0$MODULE}
17:16:10,289 INFO  [STDOUT] ---> Not found again in [org.jboss.classpool.plugins.jbosscl.JBossClDelegatingClassPool@404225673 [class path: BaseClassLoader@280bca{deployers-classloader:0.0.0$MODULE}:] - dcl:BaseClassLoader@280bca{deployers-classloader:0.0.0$MODULE} domain: [org.jboss.classpool.plugins.jbosscl.JBossClClassPoolDomain@101ebf5c name:DefaultDomain]]
17:16:10,289 INFO  [STDOUT] ---> Found in actual pool org.jboss.classpool.spi.AbstractClassPool@697579067 [class path: BaseClassLoader@8a85268{aop-classloader:0.0.0$MODULE}:] - dcl:BaseClassLoader@8a85268{aop-classloader:0.0.0$MODULE}

As you can see AspectManagerJMXRegistrar is loaded from the classloader used to install jmx.xml, and AOPAnnotationMetaDataParserDeployer from deployers.xml's classloader. If I use the pools for those loaders directly I get a NotFoundException. If I try to load the class from the loader directly it works, and in both cases the classes returned are loaded from the classloader used for aop.xml  (AspectManagerJMXRegistrar comes from jboss-aop-asintegration-core.jar, and AOPAnnotationMetaDataParserDeployer comes from jboss-aop-deployers.jar).

Next I get an error that brings everything to a halt (with some extra debug information compared to the error message that is in svn):

17:16:13,036 ERROR [AbstractKernelController] Error installing to PreInstall: name=AOPClassLoaderDeployer state=Real: java.lang.NoClassDefFoundError: Unable to find class org.jboss.aop.asintegration.jboss5.AOPClassLoaderDeployer from org.jboss.osgi.integration.jbossas.AOPClassLoaderDeployerJBAS7909 in pool [org.jboss.classpool.plugins.jbosscl.JBossClDelegatingClassPool@404225673 [class path: BaseClassLoader@280bca{deployers-classloader:0.0.0$MODULE}:] - dcl:BaseClassLoader@280bca{deployers-classloader:0.0.0$MODULE} domain: [org.jboss.classpool.plugins.jbosscl.JBossClClassPoolDomain@101ebf5c name:DefaultDomain]]
     at org.jboss.reflect.plugins.javassist.JavassistTypeInfoFactoryImpl.raiseClassNotFound(JavassistTypeInfoFactoryImpl.java:104) [jboss-reflect.jar:2.2.0-SNAPSHOT]
     at org.jboss.reflect.plugins.javassist.JavassistTypeInfo.getSuperclass(JavassistTypeInfo.java:235) [jboss-reflect.jar:2.2.0-SNAPSHOT]
     at org.jboss.beans.info.plugins.AbstractBeanInfoFactory.getMethods(AbstractBeanInfoFactory.java:262) [jboss-reflect.jar:2.2.0-SNAPSHOT]
     at org.jboss.beans.info.plugins.AbstractBeanInfoFactory.getBeanInfo(AbstractBeanInfoFactory.java:152) [jboss-reflect.jar:2.2.0-SNAPSHOT]
     at org.jboss.config.plugins.AbstractConfiguration.getBeanInfo(AbstractConfiguration.java:87) [jboss-reflect.jar:2.2.0-SNAPSHOT]
     at org.jboss.kernel.plugins.config.AbstractKernelConfig.getBeanInfo(AbstractKernelConfig.java:80) [jboss-kernel.jar:2.2.0.Alpha9]
     at org.jboss.kernel.plugins.config.AbstractKernelConfigurator.getBeanInfo(AbstractKernelConfigurator.java:78) [jboss-kernel.jar:2.2.0.Alpha9]
     at org.jboss.kernel.plugins.config.AbstractKernelConfigurator.getBeanInfo(AbstractKernelConfigurator.java:97) [jboss-kernel.jar:2.2.0.Alpha9]
     at org.jboss.kernel.plugins.dependency.PreInstallAction.installActionInternal(PreInstallAction.java:88) [jboss-kernel.jar:2.2.0.Alpha9]
     at org.jboss.kernel.plugins.dependency.InstallsAwareAction.installAction(InstallsAwareAction.java:54) [jboss-kernel.jar:2.2.0.Alpha9]
     at org.jboss.kernel.plugins.dependency.InstallsAwareAction.installAction(InstallsAwareAction.java:42) [jboss-kernel.jar:2.2.0.Alpha9]
     at org.jboss.dependency.plugins.action.SimpleControllerContextAction.simpleInstallAction(SimpleControllerContextAction.java:62) [jboss-dependency.jar:2.2.0.Alpha9]
     at org.jboss.dependency.plugins.action.AccessControllerContextAction.install(AccessControllerContextAction.java:71) [jboss-dependency.jar:2.2.0.Alpha9]
     at org.jboss.dependency.plugins.AbstractControllerContextActions.install(AbstractControllerContextActions.java:51) [jboss-dependency.jar:2.2.0.Alpha9]
     at org.jboss.dependency.plugins.AbstractControllerContext.install(AbstractControllerContext.java:377) [jboss-dependency.jar:2.2.0.Alpha9]
     at org.jboss.dependency.plugins.AbstractController.install(AbstractController.java:2042) [jboss-dependency.jar:2.2.0.Alpha9]
     at org.jboss.dependency.plugins.AbstractController.incrementState(AbstractController.java:1081) [jboss-dependency.jar:2.2.0.Alpha9]
     at org.jboss.dependency.plugins.AbstractController.executeOrIncrementStateDirectly(AbstractController.java:1320) [jboss-dependency.jar:2.2.0.Alpha9]
     at org.jboss.dependency.plugins.AbstractController.resolveContexts(AbstractController.java:1244) [jboss-dependency.jar:2.2.0.Alpha9]
     at org.jboss.dependency.plugins.AbstractController.resolveContexts(AbstractController.java:1137) [jboss-dependency.jar:2.2.0.Alpha9]
     at org.jboss.dependency.plugins.AbstractController.install(AbstractController.java:892) [jboss-dependency.jar:2.2.0.Alpha9]
     at org.jboss.dependency.plugins.AbstractController.install(AbstractController.java:639) [jboss-dependency.jar:2.2.0.Alpha9]
     at org.jboss.kernel.plugins.deployment.AbstractKernelDeployer.deployBean(AbstractKernelDeployer.java:319) [jboss-kernel.jar:2.2.0.Alpha9]
     at org.jboss.kernel.plugins.deployment.AbstractKernelDeployer.deployBeans(AbstractKernelDeployer.java:297) [jboss-kernel.jar:2.2.0.Alpha9]
     at org.jboss.kernel.plugins.deployment.AbstractKernelDeployer.deploy(AbstractKernelDeployer.java:130) [jboss-kernel.jar:2.2.0.Alpha9]
     at org.jboss.kernel.plugins.deployment.BasicKernelDeployer.deploy(BasicKernelDeployer.java:76) [jboss-kernel.jar:2.2.0.Alpha9]
     at org.jboss.bootstrap.impl.mc.deployer.TempBasicXMLDeployer.deploy(TempBasicXMLDeployer.java:92) [jboss-bootstrap-impl-mc.jar:2.1.0-SNAPSHOT]
     at org.jboss.bootstrap.impl.mc.deployer.TempBasicXMLDeployer.deploy(TempBasicXMLDeployer.java:193) [jboss-bootstrap-impl-mc.jar:2.1.0-SNAPSHOT]
     at org.jboss.bootstrap.impl.mc.server.AbstractMCServerBase.bootstrapMcAndDescriptors(AbstractMCServerBase.java:318) [jboss-bootstrap-impl-mc.jar:2.1.0-SNAPSHOT]
     at org.jboss.bootstrap.impl.mc.server.AbstractMCServerBase.doStart(AbstractMCServerBase.java:265) [jboss-bootstrap-impl-mc.jar:2.1.0-SNAPSHOT]
     at org.jboss.bootstrap.impl.as.server.AbstractJBossASServerBase.doStart(AbstractJBossASServerBase.java:381) [jboss-bootstrap-impl-as.jar:2.1.0-SNAPSHOT]
     at org.jboss.bootstrap.impl.base.server.AbstractServer$StartServerTask.run(AbstractServer.java:413) [jboss-bootstrap-impl-base.jar:2.1.0-SNAPSHOT]
     at java.lang.Thread.run(Thread.java:637) [:1.6.0_17]

This is caused by a call to CtClass.getSuperClass() for org.jboss.osgi.integration.jbossas.AOPClassLoaderDeployerJBAS7909 which has the deployers.xml classpool, but the superclass lives in the aop.xml classpool.

So something is going wrong during bootstrap, and it looks like the classpools can't see classes from the other pools within the domain, while the classloaders can see classes from the other loaders in the domain. What is weird is that from bootstrap.xml it looks like the RegisterModuleCallback (from aop.xml) should be deployed before jmx.xml and deployers.xml, but could there be anything else missing at this stage?

<bootstrap xmlns="urn:jboss:bootstrap:1.0">
   <url>bootstrap/vfs.xml</url>
   <url>bootstrap/classloader.xml</url>
   <url>bootstrap/stdio.xml</url>
   <url>bootstrap/kernel.xml</url>
   <url>bootstrap/aop.xml</url>
   <url>bootstrap/jmx.xml</url>
   <url>bootstrap/deployers.xml</url>
   <url>bootstrap/profile.xml</url>
</bootstrap>

[alesj]

Do you already register ClassPoolREgistry asap?

e.g. in some bootstrap callback / plugin or even Main

If not, we should definitely do that, so we don't rely on exceptions handling our type info lookup.

Another thing to fix or do properly is where and how we define these classpool jars.

e.g. in which boostrap sub xml should we define classpool roots; specially with some deployers dependency in the registry

[kabirkhan]

I put some breakpoints in RegisterModuleCallback.addModule(), JBossClDelegatingClassPool constructor, and in JavassistTypeInfoFactoryImpl.delegateToIntrospectionImplementation() (where failed ClassPool lookups end up) and it seems that there is a problem in that the aop classloader does not get registered with the classpools. The flow of these break points is:

1. RegisterModuleCallback.addModule() - bootstrap-classloader:0.0.0
2. RegisterModuleCallback.addModule() - jmx-classloader:0.0.0
3. JBossClDelegatingClassPool() - bootstrap-classloader:0.0.0
4. JBossClDelegatingClassPool() - jmx-classloader:0.0.0
5. Get TypeInfo for AspectManagerJMXRegistrar fails (aop classpool not created yet)
6. RegisterModuleCallback.addModule() - deployers-classloader:0.0.0
7. JBossClDelegatingClassPool() - deployers-classloader:0.0.0
8. Get TypeInfo for AOPAnnotationMetaDataParserDeployer fails (aop classpool not created)
9. Get TypeInfo for AOPDeploymentAopMetaDataDeployer fails (aop classpool not created)
10. Get TypeInfo for BeansDeploymentAopMetaDataDeployer fails (aop classpool not created)
11. RegisterModuleCallback.addModule() - profile-classloader:0.0.0
12. JBossClDelegatingClassPool() profile-classloader:0.0.0

Looking at the sequence of files from bootstrap.xml:

• bootstrap/vfs.xml  - No classloader
• bootstrap/classloader.xml - defines bootstap-classloader:0.0.0
• bootstrap/stdio.xml - defines stdio-classloader:0.0.0
• bootstrap/kernel.xml - defines asynch-classloader:0.0.0
• bootstrap/aop.xml - defines aop-classloader:0.0.0 and the RegisterModuleCallback
• bootstrap/jmx.xml - defines jmx-classloader:0.0.0
• bootstrap/deployers.xml - defines deployers-classloader:0.0.0
• bootstrap/profile.xml - defines deployers-classloader:0.0.0

Assuming these get deployed in order (which I will look at next) it seems strange that the RegisterModuleCallback receives bootstrap-, jmx-, deployers- and profile-classloader, but not the aop-, stdio and, asynch-classloaders

[alesj]

Assuming these get deployed in order (which I will look at next)

Yes, they do.

it seems strange that the RegisterModuleCallback receives bootstrap-, jmx-, deployers- and profile-classloader, but not the aop-, stdio and, asynch-classloaders

It's not strange if you look into aop.xml in more details. ;-)

It's simply a matter of order + a small detail. :-)

For the order this is obvious -- aop, stdio and asynch are all deployed before the registry is intalled.

And the small detail is this

<install method="addModule">
   <parameter><inject bean="bootstrap-classloader:0.0.0$MODULE"/></parameter>
</install>

which is why you also see bootstrap.

[alrubinger]

This patch looks allright.  Another option is to remove the ClassPool stuff from "doInitialize" and create a LifecycleEventHandler (registered in LifecycleState.PRE_INIT) to extract out the logic from the server itself (I recommend this approach).

This patch leads spi-test to fail with:

java.lang.NoClassDefFoundError: org/jboss/classpool/scoped/ScopedClassPoolRepository

...which is just an issue of adding the dep to the spi-test POM config to be copied into target/lib.

S,
ALR

[kabirkhan]

I've modified aop.xml as Ales suggested

   <bean name="AOPRegisterModuleCallback" class="org.jboss.classpool.plugins.jbosscl.RegisterModuleCallback">
      <!-- classloader><null/></classloader -->
     <constructor>
       <parameter><inject bean="ClassLoaderDomainRegistry"/></parameter>
     </constructor>
     <install method="addModuleRegistry" bean="ClassLoading" whenRequired="Start">
       <parameter><this/></parameter>
     </install>
     <install method="addModule">
       <parameter><inject bean="bootstrap-classloader:0.0.0$MODULE"/></parameter>
     </install>
     <install method="addModule">
       <parameter><inject bean="stdio-classloader:0.0.0$MODULE"/></parameter>
     </install>
     <install method="addModule">
       <parameter><inject bean="asynch-classloader:0.0.0$MODULE"/></parameter>
     </install>
     <install method="addModule">
       <parameter><inject bean="aop-classloader:0.0.0$MODULE"/></parameter>
     </install>
     <uninstall method="removeModuleRegistry" bean="ClassLoading" whenRequired="Start">
       <parameter><this/></parameter>
     </uninstall>
   </bean>

Now the classpools get created, but I am getting the wrong sort of classpool until aop kicks in. These get an AbstractClassPool since ClassPoolRepository still defaults to AbstractClassPoolFactory. AbstractClassPool is unaware of the class loader domain, so we get exceptions when trying to load classes from the domain using any of these, and the domain cannot see the classes from these loaders. The loaders are in the order they get installed:

• URLClassLoader
• BaseClassLoader stdio-classloader
• Launcher$AppClassLoader
• BaseClassLoader asynch-classloader
• BaseClassLoader aop-classloader

These get JBossClDelegatingClassPool since ClassPoolFactory.factory is now JBoss5Integration which delegates to JBossClDelegatingClassPoolFactory

• BaseClassLoader bootstrap-classloader
• BaseClassLoader jmx-classloader

I'll try to change what I did in bootstrap to hardwire these beans and see where that gets me:

   <bean name="AOPClassLoaderScopingPolicy" class="org.jboss.aop.asintegration.jboss5.VFSClassLoaderScopingPolicy"/>  
   <bean name="AOPClassPoolFactory" class="org.jboss.classpool.plugins.jbosscl.JBossClDelegatingClassPoolFactory">
      <constructor>
         <parameter><inject bean="AOPClassLoaderScopingPolicy" property="registry"/></parameter>
         <parameter><inject bean="AOPRegisterModuleCallback"/></parameter>
      </constructor>
   </bean>

   <bean name="AOPJBossIntegration" class="org.jboss.aop.asintegration.jboss5.JBoss5Integration">
      <property name="classPoolFactory"><inject bean="AOPClassPoolFactory"/></property>
      <property name="aopClassLoaderScopingPolicy"><inject bean="AOPClassLoaderScopingPolicy"/></property>
   </bean>

[alrubinger]

ie:

/**
 * {@link LifecycleEventHandler} implementation to set the jboss-reflect
 * {@link RepositoryClassPoolFactory}.  Must be called before MC is brought up.
 *
 * @author <a href="mailto:[e-mail]">ALR</a>
 */
public enum SetClassPoolLifecycleEventHandler implements LifecycleEventHandler {

   INSTANCE;

   //-------------------------------------------------------------------------------------||
   // Class Members ----------------------------------------------------------------------||
   //-------------------------------------------------------------------------------------||

   /**
    * Logger
    */
   private static final Logger log = Logger.getLogger(SetClassPoolLifecycleEventHandler.class);

   //-------------------------------------------------------------------------------------||
   // Required Implementations -----------------------------------------------------------||
   //-------------------------------------------------------------------------------------||

   /**
    * {@inheritDoc}
    * @see org.jboss.bootstrap.api.lifecycle.LifecycleEventHandler#handleEvent(org.jboss.bootstrap.api.lifecycle.LifecycleState)
    */
   @Override
   public void handleEvent(final LifecycleState state) throws LifecycleEventException
   {
      /*
       * Make sure we have the correct classpool  
       */

      final RepositoryClassPoolFactory factory = new RepositoryClassPoolFactory(ClassPoolRepository.getInstance());
      JavassistTypeInfoFactoryImpl.setPoolFactory(factory);
      if (log.isTraceEnabled())
      {
         log.tracef("Set %s pool factory to %s", JavassistTypeInfoFactoryImpl.class.getSimpleName(), factory);
      }
   }
}

[kabirkhan]

Kabir Khan wrote:

I'll try to change what I did in bootstrap to hardwire these beans and see where that gets me:

   <bean name="AOPClassLoaderScopingPolicy" class="org.jboss.aop.asintegration.jboss5.VFSClassLoaderScopingPolicy"/>  

   <bean name="AOPClassPoolFactory" class="org.jboss.classpool.plugins.jbosscl.JBossClDelegatingClassPoolFactory">
      <constructor>
         <parameter><inject bean="AOPClassLoaderScopingPolicy" property="registry"/></parameter>
         <parameter><inject bean="AOPRegisterModuleCallback"/></parameter>
      </constructor>
   </bean>

   <bean name="AOPJBossIntegration" class="org.jboss.aop.asintegration.jboss5.JBoss5Integration">
      <property name="classPoolFactory"><inject bean="AOPClassPoolFactory"/></property>
      <property name="aopClassLoaderScopingPolicy"><inject bean="AOPClassLoaderScopingPolicy"/></property>
   </bean>

This part of the discussion continues at http://community.jboss.org/thread/151095

[kabirkhan]

Kabir Khan wrote:

I have done some performance measurements where I compare the times taken creating the following class, using javassist.bytecode.* and asm

public class JavassistMethod1 implements JavassistMethod
{ 
  public Object invoke(Object target, Object[] args) throws Throwable
  {     
      return Integer.valueOf(((SomeClass)target).someMethod(((Integer)args[0]).intValue(), (String)args[1])).intValue();
  }
} 

Which would be used to call the method:

    int someMethod(int i, String);

The basic flow for what I do for both approaches is the same, whereby I do the following lots of times to generate lots of similar classes:

A) - Create class structure

B) - Add default constructor with body to call super

C) - Add invoke method

C1) - Add invoke method body

D) - Convert class structure from A) into byte[]

E) - Define java.lang.Class by calling ClassLoader.defineClass()

F) - Call Class.newInstance()

Chiba has done some great work on creating a new API for Javassist tailor made to create new classes. Taking the defining of the class and instantiating it out of the equation (since that is JVM stuff out of our control), so we do A-D so we have the bytes ready to create the class the times are now for creating 20000 JavassistMethod implementations

ASM	Javassist ClassFile	JavassistClassFileWriter
476	1030	356
613	1056	269
483	1076	309
464	1001	357
383	1186	315

I have attached the modified benchmark

[kabirkhan]

I forgot to mention that I have converted the JavassistMemberFactory to use the new ClassFileWriter API

https://jira.jboss.org/jira/browse/JBREFLECT-122

[kabirkhan]

Although too early to say yet, it looks like the classpools are a bottleneck. I have started caching things a bit more and that is speeding things up somewhat. For example calling CtBehavior.getParameterTypes() is slow since it does not cache the parameter types, instead for every call to this it parses the method signature and then hits the classpools for every class. I am attempting to avoid that as much as possible.

Another observation is that quite a few calls to JavassistMethodInfo.getParameterTypes() are only interested in the length of the parameters and the names of the parameter types, so it might be an idea to return an array of "lazy" type infos which only know the name and the declaring typeinfo. I can be found out easily from the raw method signature (I am already passing the SignatureKey into JavassistMethodInfo and using that in its equals() method to avoid having to load parameters there). Once something more advanced is called, then that could obtain the real typeinfo and delegate to that. I'm not 100% sure yet if this is what I want to do.

Once I am done with caching things as much as possible, we'll see if the classpools are still a bottleneck since there will be a lot less calls. If they are, I have an idea for how to change them into something simpler, but just want to jot it down so Flavia can take a look and see if she thinks it is a good idea or if I have missed something.

The idea is quite simple, rather than managing the domains and pools and essentially recreating what the classloaders do, it might make sense to delegate everything to the classloaders. If we had a method like this, as I discussed with Ales (let's say it goes in the ClassLoading class, although where is up to Ales):

ClassLoader getClassLoaderForClass(ClassLoader initiating, String classname);

And we have a map of classpools by classloader in the classpool registry.

Then I think our "dumb" class pool would not need much more than this - it has a null parent

class DumbClassPool extends ClassPool{

    WeakReference<ClassLoader> cl;
    ClassPoolRepository repo;

    DumbClassPool(ClassLoader cl, ClassPoolRepository repo){
      this.cl = new WeakReference<ClassLoader>(cl);
      this.repo = repo;
    }


    CtClass get(String classname){
        //Check cache first, if not found then do the rest

        CtClass clazz = super.get0(classname);
        if (clazz != null)
           return clazz;

        String real = adjustForArraysAndPrimitives(classname);
        ClassLoader loader = ClassLoading.getClassLoaderForClass(cl.get(), real);

        ClassPool pool = repo.registerClassLoader(loader);
        if (pool != null)
        {
           clazz = pool.getOrNull(classname); //new method from Chiba that does not throw NFE, but behaves like get()
        }

        if (clazz != null)
           //cache class
        else
           throw new NotFoundException
    }
}

I'd obviously be extending ScopedClassPool or whatever is needed in AOP and other places. It would also have some knowledge of the domain and get notified when loaders get added/removed to the domain so the caches in the pools can be invalidated. There will probably be a bit more to it than this, but this is the basic idea.

[flavia.rainone]

Kabir Khan wrote:

Once I am done with caching things as much as possible, we'll see if the classpools are still a bottleneck since there will be a lot less calls. If they are, I have an idea for how to change them into something simpler, but just want to jot it down so Flavia can take a look and see if she thinks it is a good idea or if I have missed something.

The idea is quite simple, rather than managing the domains and pools and essentially recreating what the classloaders do, it might make sense to delegate everything to the classloaders. If we had a method like this, as I discussed with Ales (let's say it goes in the ClassLoading class, although where is up to Ales):

ClassLoader getClassLoaderForClass(ClassLoader initiating, String classname);

And we have a map of classpools by classloader in the classpool registry.

Then I think our "dumb" class pool would not need much more than this - it has a null parent

class DumbClassPool extends ClassPool{
 
    WeakReference<ClassLoader> cl;
    ClassPoolRepository repo;
 
    DumbClassPool(ClassLoader cl, ClassPoolRepository repo){
      this.cl = new WeakReference<ClassLoader>(cl);
      this.repo = repo;
    }
 
 
    CtClass get(String classname){
        //Check cache first, if not found then do the rest
 
        CtClass clazz = super.get0(classname);
        if (clazz != null)
           return clazz;
 
        String real = adjustForArraysAndPrimitives(classname);
        ClassLoader loader = ClassLoading.getClassLoaderForClass(cl.get(), real);
 
        ClassPool pool = repo.registerClassLoader(loader);
        if (pool != null)
        {
           clazz = pool.getOrNull(classname); //new method from Chiba that does not throw NFE, but behaves like get()
        }
 
        if (clazz != null)
           //cache class
        else
           throw new NotFoundException
    }
}

I'd obviously be extending ScopedClassPool or whatever is needed in AOP and other places. It would also have some knowledge of the domain and get notified when loaders get added/removed to the domain so the caches in the pools can be invalidated. There will probably be a bit more to it than this, but this is the basic idea.

I only see pros at implementing the classpools this way:

- it is fast to do it. I know that there is a lot of detail that needs to be filled in, but nobody is talking about reimplementing them with the same level of complexity of before

- it is so clean that we will be able of seeing the real impact of classpools in the performance

Regarding the last sentence, as has already been discussed on our team calls, it is not a piece of cake to outperform reflection:

- it is good that we don't force classes to be loaded... but what if the class has already been loaded? Do we want to use an extra loading level in that case? We can of course try to create a clever mechanism for working around this, but this is more complexity to a code that is already complex.

- we are doing with Java and Javassist (one more layer of indirection) something that the JDK already does (apart from bytecodes manipulation, of course). After years of JDK improvements, we can expect that they do this natively and fast. The JDK can use its own internal structures (the ones it uses to load and run code) for answering reflection invocations.

Thus, if we have the cleanest possible implementation of the ClassPools, we have less overhead and we will finally be able to see if it is possible to improve the performance with Javassist/Classpools and to beat our reflection-based implementation.

This is the main question we've been trying to answer all this time.

Using the ClassLoaders SPI to find the appropriate ClassLoader/ClassPool is definitely a plus in this regard, as we are using the same ClassLoader structure that the reflection implementation uses.

The only drawback is that we won't be able of getting rid of the old structure for AS 4 (ie, erasing all classpool code for as 4), unless we use an older version of the project for JBoss AOP. Which is not really a drawback, it is more like a constraint that we have to face for being backwards compatible with an older version of AS.

[kabirkhan]

Kabir Khan wrote:

Another observation is that quite a few calls to JavassistMethodInfo.getParameterTypes() are only interested in the length of the parameters and the names of the parameter types, so it might be an idea to return an array of "lazy" type infos which only know the name and the declaring typeinfo. I can be found out easily from the raw method signature (I am already passing the SignatureKey into JavassistMethodInfo and using that in its equals() method to avoid having to load parameters there). Once something more advanced is called, then that could obtain the real typeinfo and delegate to that. I'm not 100% sure yet if this is what I want to do.

I've looked more into this and will try out "lazy" type infos. When constructing the BeanInfos, it goes through and calls MethodInfo.getReturnType() and MethodInfo.getParameterTypes(). These calls are really heavy, and at least for this use-case we don't care about anything apart from the name. Behind the scenes, what happens for each parameter is:

a) It hits the classpool to load the CtClass

b) When creating the type info it needs to check if the class is an enum or annotation, the first time you call anything apart from CtClass.getName() it needs to load the underlying ClassFile. This is slow, but initialises the CtClass for future use.

a) and b) seem to take about the same time, although b) probably varies with class size. I don't think we can do much about b) apart from trying to avoid it at all costs as much as possible.

Another observation in favour of optimizing the classpools further is needing to do this in JavassistTypeInfoFactory.get() (pseudocode)

public TypeInfo get(ClassLoader cl, String name){

    Cache cache = getCacheForLoader(cl);
    TypeInfo info = cache.get(name);
    if (info != null)
       return info;

    //The passed in classloader is not necessarily the one where the class lives,
    //so we need to hit the pool to find out where it comes from
    ClassPool pool = getPoolForLoader(cl);
    CtClass clazz = pool.get(name);

    if (clazz.getClassPool().getClassLoader() != cl)
    {
       cl = clazz.getClassPool().getClassLoader(); 
       cache = getCacheForLoader(cl);
       info = cache.get(name);
       if (info != null)
          return info;
    }

    info = instantiate(clazz);
    cache.cache(info);

   return info;
}

We need to query the pool to figure out the correct classloader