Re: [pde-ui-dev] PDE API tools and ASM bytecode manipulation framework

[Darin Wright <Darin_Wright@xxxxxxxxxx>]

Thanks Eugene,

Your insight and suggestions are very
useful. We will reivew your comments in more detail and clean things up
as suggested. We may have a few questions... :-)

Darin Wright

Eugene Kuleshov <eu@xxxxxxxx>
Sent by: pde-ui-dev-bounces@xxxxxxxxxxx

01/07/2008 11:59 PM

Please respond to
"Eclipse PDE UI developers list." <pde-ui-dev@xxxxxxxxxxx>

To	pde-ui-dev@xxxxxxxxxxx
cc
Subject	[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


_______________________________________________
pde-ui-dev mailing list
pde-ui-dev@xxxxxxxxxxx
https://dev.eclipse.org/mailman/listinfo/pde-ui-dev