12 Replies Latest reply on Jan 9, 2009 6:26 AM by kabirkhan

    Using ClassInfo in JBoss AOP

    kabirkhan

      We would like to start using ClassInfo in JBoss AOP, since that will get rid of the duplicated code we have in our pointcut matchers. Currently we have code doing the same thing for javassist (during weaving) and for java.lang.reflect (to populate the advisors of woven classes at runtime).

      For our use the current implementation of JavassistTypeInfoFactoryImpl has a few problems:

      1) From what I can see, it does not allow the specification of a javassist ClassPool to use, instead it falls back on ClassPool.getDefault(), which just gives you the stuff on the bootstrap classpath, and so will not work in AS for application classes. It should be possible to pass in the ClassPool to use, or if working out the ClassPool from the classloader, to pass in some kind of ClassPool factory so that it uses the same underlying ClassPool as AOP does.

      2) We would want to use the javassist version of ClassInfo during weaving, i.e. before the classes are loaded. However, the WeakClassCache used by JavassistTypeInfoFactoryImpl seems to need to load the class:

      Thread [main] (Suspended (breakpoint at line 126 in JavassistTypeInfoFactoryImpl))
       JavassistTypeInfoFactoryImpl.instantiate(Class) line: 126
       JavassistTypeInfoFactoryImpl(WeakClassCache).get(Class) line: 67
       JavassistTypeInfoFactoryImpl.getTypeInfo(Class<?>) line: 280
       JavassistTypeInfoFactoryImpl.getTypeInfo(String, ClassLoader) line: 312
       JavassistTypeInfoFactory.getTypeInfo(String, ClassLoader) line: 54
       JavassistArrayUnitTestCase(AbstractClassInfoTest).testBasics(Class<?>, TypeInfo) line: 72
       JavassistArrayUnitTestCase(ClassInfoArrayTest).testArray(Object) line: 139
       JavassistArrayUnitTestCase(ClassInfoArrayTest).testSimpleArray() line: 46
      

      So that is not good for us. We need a javassist version of ClassInfo to use during weaving that does not load the class as part of its instantiation, once woven the class can be put into the WeakClassCache.


        • 1. Re: Using ClassInfo in JBoss AOP
          stalep

          from what i can see in JavassistTypeInfoFactoryImpl and JavassistTypeInfo the usage of java.lang.Class isnt very extensive.
          in JavassistTypeInfoFactoryImpl.instantiate it is used to check classtype, but we can use the CtClass object for that instead.
          JavassistTypeInfo has one public method that uses java.lang.Class (getType), but that is deprecated and the other uses can be replaced by using the CtClass object.

          • 2. Re: Using ClassInfo in JBoss AOP
            kabirkhan

            You should be using the methods from the TypeInfo spi interface.

             TypeInfo getTypeInfo(String name, ClassLoader cl) throws ClassNotFoundException;
            

            could be made to work the way outlined above for JavassistTypeInfoFactoryImpl with some modifications to WeakClassCache. For the other methods I don't think there is much we can do

            • 3. Re: Using ClassInfo in JBoss AOP
              stalep

              creating a simple prototype of JavassistTypeInfoFactoryImpl with the needed changes isnt as straightforward as first thought.
              we need to overwrite the methods WeakClassCache.get(Class clazz) and WeakClassCache.get(String name, ClassLoader cl).

              the problem is that the method WeakClassCache.get(Class clazz) calls JavassistTypeInfoFactoryImpl.instantiate(Class clazz).
              this method needs to be changed too since we dont have any Class object yet. by changing this method we get a lot of errors since it instantiate several JavassistTypeInfo objects, and most of the constructors to the different JavassistTypeInfo/JavassistAnnotationInfo/etc classes are either packaged protected or packaged/override protected.

              JavassistTypeInfo "cannot" be extended since its constructors are package protected and by creating an identical JavassistTypeInfo class inside of the prototype package we need to either extend a lot of *Info classes or create new identical classes.

              anyone got any ideas of how it can be done without creating X number of prototype classes?

              • 4. Re: Using ClassInfo in JBoss AOP
                kabirkhan

                For the prototype, can you not subclass JavassistTypeInfoFactoryImpl to override the getTypeInfo() methods making the one taking ClassLoader+name the "main" one? Similarly, override the get() methods and other functionality from WeakClassCache, so we can sidestep the stuff in there for now. Then subclass JavassistTypeInfoFactory to instantiate your (Prototype)JavassistTypeInfoFactoryImpl.

                I've only had a very quick look, but I think that way everything else should work as before, the only place I can see JTIFI instantiated is in the JavassistTypeInfoFactory

                • 5. Re: Using ClassInfo in JBoss AOP
                  kabirkhan

                  Adrian,

                  Once you are back, FYI this is work we are doing in AOP to test out the ClassInfo stuff with our needs. We'll discuss how to do it properly in jboss-reflect later

                  • 6. Re: Using ClassInfo in JBoss AOP
                    stalep

                    im not sure how since JavassistTypeInfoFactoryImpl.getTypeInfo(String name, ClassLoader cl) calls WeakClassCache.get(Class clazz) that again call JavassistTypeInfoFactoryImpl.instantiate(Class clazz).
                    it is this method that creates the TypeInfo object.

                    even though i override get(Class clazz), i wont be able to call instantiate.

                    • 7. Re: Using ClassInfo in JBoss AOP
                      stalep

                      ok, so we will get around the problem with package restricted constructors by using reflection.
                      what we're doing now is that the Class clazz parameter that is defined in JavassistTypeInfo is never instantiated (eg always null) and we use the CtClass object where clazz earlier where used in JTIFactoryImpl.

                      if we disregard the deprecated public method JavassistTypeInfo.getType(), clazz isnt use at all in JavassistTypeInfo except when throwing an exception in the getSuperclass() method (which could/should be changed to CtClass anyway imo).

                      • 8. Re: Using ClassInfo in JBoss AOP
                        stalep

                        ive created a aopversion of JavassistTypeInfoFactoryImpl that can be found in the aop project as org.jboss.aop.reflectprototype.JavassistAopTypeInfoFactoryImpl.
                        i made a simple test to make sure that the CtClass isnt frozen (eg the class file is loaded) when creating a ClassInfo object from a CtClass.
                        however, when i try to make some changes to that CtClass and try to load a new ClassInfo object that should reflect the changes the changes isnt there. i recon its because the CtClass is cached in WeakClassCache. can this be an issue for aop? it means that after we get a ClassInfo reference of a CtClass, we cant make changes to that CtClass anymore. imo this would be too restrictive for aop.
                        take a look at the test org.jboss.test.aop.reflectprototype.JavassistAopTypeInfoFactoryImplTestCase for an example.

                        • 9. Re: Using ClassInfo in JBoss AOP
                          kabirkhan

                          That is definitely be an issue for aop. From what I can see of the test, when we add a method to the underlying CtClass the ClassInfo wrapper is not updated. I think the ClassInfo's MethodInfos etc. are cached on first load, and not refreshed at all.

                          One solution would be to add a refresh() method to ClassInfo, but there are probably some other and better solutions too. When I mentioned the MutableClassInfo stuff to Ales, he mentioned adding an AOP view to ClassInfo, MethodInfo etc. containing the new stuff added by AOP, but I am not sure how that would be added since instantiation of all those objects are hidden inside jboss-reflect.

                          • 10. Re: Using ClassInfo in JBoss AOP
                            stalep

                            just to clear it up. its weakclasscache that cache the ctclasses, the methodinfos are also cached if they have been fetched earlier from what i can tell.

                            one thing we could do is to add a compare(CtClass,CtClass) method and check if there has been any changes after we've fetched the ClassInfo from cache.
                            since we cant do any changes to JavassistTypeInfo, ill add a simple compare(JavassistTypeInfo,CtClass) in JavassistTypeInfoFactoryImpl when we fetch the object from cache and just compare number of methods/fields. - it should be more thorough, but this would do for now i think.

                            • 11. Re: Using ClassInfo in JBoss AOP
                              kabirkhan

                               

                              "stale.pedersen@jboss.org" wrote:
                              just to clear it up. its weakclasscache that cache the ctclasses, the methodinfos are also cached if they have been fetched earlier from what i can tell.


                              If the same classpool is used, the instance of CtClass will be the same, i.e.:

                              ClassPool pool = ...;
                              CtClass c1 = pool.get("POJO");
                              CtClass c2 = pool.get("POJO")
                              c1 == c2; //Will be true
                              


                              • 12. Re: Using ClassInfo in JBoss AOP
                                kabirkhan

                                Fixed mistake in my previous post