Creation of method tables in ClassAdvisor

[jaikiran]

Need some input on a couple of things in the org.jboss.aop.ClassAdvisor http://anonsvn.jboss.org/repos/jbossas/projects/aop/branches/Branch_2_1/aop/src/main/java/org/jboss/aop/ClassAdvisor.java. During initialization and creation of method tables, the ClassAdvisor in it's addDeclaredMethod does this:

protected void addDeclaredMethods(Class<?> superclass) throws Exception
 {
 Method[] declaredMethods = superclass.getDeclaredMethods();
 for (int i = 0; i < declaredMethods.length; i++)
 {
 if (ClassAdvisor.isAdvisable(declaredMethods[ i]))
 {
 long hash = MethodHashing.methodHash(declaredMethods[ i]);
 advisedMethods.put(hash, declaredMethods[ i]);
 try
 {
 Method m = declaredMethods[ i];
 Method un = superclass.getDeclaredMethod(ClassAdvisor.notAdvisedMethodName(superclass.getName(),
 m.getName()),
 m.getParameterTypes());
 un.setAccessible(true);
 unadvisedMethods.put(hash, un);
 }
 catch (NoSuchMethodException ignored)
 {
 }
 }
 }
 }

 public static String notAdvisedMethodName(String className,
 String methodName)
 {
 return className.replace('.', '$') + "$" + methodName +
 NOT_TRANSFORMABLE_SUFFIX;
 }

 /**
 * Suffix added to unadvised methods.
 */
 public static final String NOT_TRANSFORMABLE_SUFFIX = "$aop";

Assuming i have this expression:

<domain name="XXX" extends="YYY" inheritBindings="true">
 <bind pointcut="execution(public * *->*(..))">
 ...
 </bind>

package org.myapp;


public class SomeClass
{
 public void method1()
 {
 ..
 }

 public void method2()
 {
 ..
 }
}

1) So the addDeclaredMethod piece of code, above, is going to generate for each method an "unadvised method name" as:

org$myapp$SomeClass$method1$aop
org$myapp$SomeClass$method2$aop

and later on when the

superclass.getDeclaredMethod(ClassAdvisor.notAdvisedMethodName(superclass.getName(), m.getName()),m.getParameterTypes());

is called it's always going to throw a NoSuchMethodException, isn't it?

What is the purpose of this code? I guess, there is some specific scenario where this comes into picture

2) The MethodHashing.methodHash() does some very specific/involved logic. How is it different from a normal method.hashCode()?

[flavia.rainone]

Jaikiran wrote:
What is the purpose of this code? I guess, there is some specific scenario where this comes into picture

Yes, there is a scenario where this comes into picture. If you navigate through the call hierarchy, you will see that this method is being invoked by both createMethodTables and populateMethodTables, but you will see that createMethodTables invokes populateMethodTables for the superclass of the advised classe:

protected void createMethodTables()
 throws Exception
 {
 initAdvisedMethodsMap();
 populateMethodTables(clazz.getSuperclass());
 addDeclaredMethods(clazz);
 }

Plus, populateMethodTables is recursive on the superclass:

private void populateMethodTables(Class<?> superclass)
 throws Exception
 {
 if (superclass == null) return;
 if (superclass.equals(Object.class)) return;

 populateMethodTables(superclass.getSuperclass());
 ...

So, we will first verify if the superclass has the unadvised method, if it has, we will use it. There are specific scenarios where the superclass may have the unadvised method, such as when there is a method in the superclass that matches a pointcut. In this scenario, the invocation to getDeclaredMethod that you mentioned is not going to fail.

[flavia.rainone]

Jaikiran wrote:
2) The MethodHashing.methodHash() does some very specific/involved logic. How is it different from a normal method.hashCode()?

Taking a look at the J2SE API (http://java.sun.com/javase/6/docs/api/):

public int hashCode()

Returns a hashcode for this Method. The hashcode is computed as the exclusive-or of the hashcodes for the underlying method's declaring class name and the method's name.

And at the MethodHashing.methodHash(Method) implementation:

public static long methodHash(Method method)
 throws Exception
 {
 Class<?>[] parameterTypes = method.getParameterTypes();
 StringBuffer methodDesc = new StringBuffer(method.getName()+"(");
 for(int j = 0; j < parameterTypes.length; j++)
 {
 methodDesc.append(getTypeString(parameterTypes[j]));
 }
 methodDesc.append(")"+getTypeString(method.getReturnType()));
 return createHash(methodDesc.toString());
 }

We can see that the Sun's version takes into account the name of the class and the name of the method, whilest the JBoss AOP implementation takes into account the name of the method, the types of the parameters and the reutrn type of the method. This is because we need an unique hash code per method:

long hash = MethodHashing.methodHash(declaredMethods);
 advisedMethods.put(hash, declaredMethods);

The complicated bits are in the createHash method:

public static long createHash(String methodDesc)
 throws Exception
 {
 long hash = 0;
 ByteArrayOutputStream bytearrayoutputstream = new ByteArrayOutputStream(512);
 MessageDigest messagedigest = MessageDigest.getInstance("SHA");
 DataOutputStream dataoutputstream = new DataOutputStream(new DigestOutputStream(bytearrayoutputstream, messagedigest));
 dataoutputstream.writeUTF(methodDesc);
 dataoutputstream.flush();
 byte abyte0[] = messagedigest.digest();
 for(int j = 0; j < Math.min(8, abyte0.length); j++)
 hash += (long)(abyte0[j] & 0xff) << j * 8;
 return hash;

 }

This method processes the string with a Message Digester, using the SHA algorithm, generating an unique byte array for each string. Then, the resulting array is transformed in a single byte by concatenating each first 8 bits of the first 8 bytes in the array. The result should be an unique hash code for each method. I'm not sure about this, since we are disposing of part of the resulting byte array, and using only the first 8 bits of the first 8 bytes (I guess I would have to ask to Mark Fleury, the author of this class). But I think that, if the hash code weren't unique, we would see failures at the JBoss AOP testsuite.

So, the short answer is that: Method.hasCode() is going to generate the same code for overloaded methods, but we need a hash code that is unique per method.

[jaikiran]

Thanks for the detailed explanation, Flavia.