Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
[pde-ui-dev] PDE API tools and ASM bytecode manipulation framework

Hi everyone,

Chris Aniszczyk mentioned another day that PDE API tools are using ASM framework. Since I am one of the developers of ASM, I thought it would be useful to share my thoughts and comments about PDE usage of ASM as well as few questions that could help me to get better understanding of your implementation and use cases. My observations are more or less random and mostly based on a quick scanning trough ASM API calls and don't assume deep analysis of the PDE implementation. Please don't take those notes as criticism, those simply suggestions for better usage of the ASM API that could help to improve performance and memory usage for your application. I think it is really great application for ASM library and I am very excited about it.

First of all I see that PDE API tools is using "asm" and "asm-tree" jars. However "asm-tree" jar is usually used for complex in-memory class transformations and method analysis, which doesn't seem the case for PDE API tools. I found two uses of the "tree" classes and they both can be removed: org.eclipse.pde.api.tools.internal.comparator.TypeDescriptor and org.eclipse.pde.api.tools.internal.search.ClassFileVisitor Basically those classes are making inefficient use of ClassNode, where it can be replaced either with org.objectweb.asm.commons.EmptyVisitor from "asm-commons" jar or your own dummy implementation of all ASM's visitor interfaces, like the org.eclipse.pde.api.tools.internal.util.ClassVisitorAdapter you have. That would improve processing performance and eliminate unnecessary memory allocation.

If I understood TypeDescriptor.initialize() method correctly, it is not interested in the method code, so you could use classReader.accept(visitor, ClassReader.SKIP_CODE); to completely skip all methods code from visiting. Same applies to implementation of SearchEngine.getExtraction(..) and TagScanner.Visitor.getMethods(..) methods, where you also can add ClassReader.SKIP_CODE to avoid visiting method code.

There is a feature in ASM that allows to skip unneeded methods and other class artifacts. So you can return a null from visitMethod() call, then method code will be also skipped (that also happens when you visit an abstract or native method). For example, in Converter.MyClassFileAdapter.visitMethod() you could rewrite the following code:

MethodVisitor visitor = super.visitMethod(accessFlags, methodName, desc, signature, exceptions);
           if (visitor != null) {
               if (reportRefs) {
                   visitor = new MyMethodAdapter(visitor);
               } else {
                   visitor = new ClearCodeAttributeMethodAdapter(visitor);
               }
           }
           return visitor;

 like this :

MethodVisitor visitor = super.visitMethod(accessFlags, methodName, desc, signature, exceptions);
           if (visitor != null) {
               if (reportRefs) {
                   visitor = new MyMethodAdapter(visitor);
               } else {
                   visitor.visitEnd();  // for safety
                   visitor = null;
               }
           }
           return visitor;

The above essentially eliminating ClearCodeAttributeMethodAdapter, which should probably have been implementing MethodVisitor directly instead of extending MethodAdapter, or org.objectweb.asm.commons.EmptyVisitor should have been used instead of it.

If none of your visitors actually cares about StackMap information, you may as well add ClassReader.SKIP_FRAMES flag to ClassReader.accept(..) call when reading or transforming classes.

It is better to not extend ClassAdapter when visitor doesn't do any transformations and it is used only to collect data, like ClassFileDescriptorBuilder does. I would suggest to use something like EmptyVisitor for that. Though it is worth to mention that org.objectweb.asm.commons.EmptyVisitor is flattening the visiting events, i.e. it don't make difference between method and fields annotations, but you can return new instance of the EmptyVisitor subclass if you need to keep hierarchy. Also, code in ClassFileDescriptorBuilder.visitMethod() that extracts value for default annotation look fairly similar to the code in org.objectweb.asm.util.TraceAnnotationVisitor class from ASM's "asm-util" jar, but your version doesn't seem add separator between array entries.

 In ClassFileVisitor.visitMethod(..) method. The following code:
   switch(type.getSort()) {
     case Type.LONG :
     case Type.DOUBLE :
       argumentcount += 2;
    default:
       argumentcount++;
   }

 can be replaced with: argumentcount += type.getSize();

The state machine used in Converter.MyMethodAdapter (passing stringLiteral between visitLdcInsn() and visitMethodInsn() methods) is probably going to work for classes compiled by javac or JDT, but there could be legal bytecode that won't fit into that pattern, but I am not sure what is the implication of that for PDE API tools.

I think I understand what MyMethodAdapter does about reference types (which seem very neat idea), but I'd be interested to learn why you have special processing for ASTORE, IRETURN, RETURN, ATHROW and POP opcodes and not all INVOKE* opcodes?

In Converter.MyClassFileAdapter.visit(..) it is better to use Opcodes.V1_5 instead of magic number "49". Though I wonder why you need to use that version instead of version from the original class? In the same method, it might be better to use annotation instead of custom attribute. Annotations are usually easier to access (in the bytecode as well as trough reflectio API) and you can still store them into classes with version <49. Though that is obviously matter of personal preferences. I also curious what Converter.MyMethodWriter is used for? I don't see any use of collected lastOffset value.

Hope you would find these notes somehow useful. Please let me know if I could be at any help in regards to ASM framework and thanks again for using it.

 Eugene




Back to the top