1) CreateDispatchCodeInCallAllBindingsAdapter::getBoxedArguments() -- line 74

instructions.add(new IntInsnNode(Opcodes.ALOAD, 2));

The ASM documentation says, that for IntInsnNode the Opcode must be either BIPUSH, SIPUSH or NEWARRAY, but ALOAD is used. Here ALOAD is used which loads a local variable reference. Is this a trick to shorten the byte code as I understand that the local variable will be directly casted into an int? This has been done a lot in the OT/J codebase. Even to load "this". And when I search for how people use this I always see they use VarInsnNode(Opcode.ALOAD, 0) (etc).

2) ObjectTeamsTransformer::transform() -- line 78 - 88

if (!weavingContext.isWeavable(sourceClassName) || loader == null) {
if (clazz != null) {
if (isWeavable(className) && clazz.needsWeaving()) {
// only print out for now, exceptions thrown by us are silently caught by TransformerManager.
new LinkageError("Classs "+className+" requires weaving, but is not weavable!").printStackTrace();
} else {
clazz.markAsUnweavable(); // mark in case we'll try to weave later
}
}
return null;
}

The error message says that the class needs weaving, but is not weave-able. Is this due to loader is null? Or, due isWeavable(className) returns false? In this case, a negation is missing (as not weaveable + needsWeaving contradicts).

1. weavingContext.isWeavable() excludes some primordial classes from packages like java.lang, org.objectteams, org.objectweb.asm, which technically fall outside the scope of weavable classes
   
2. weavingContext.isWeavable() may also use a file specified via -Dot.weavable which defines "weavable regions" (useful for admitting runtime weaving for certain parts of an application only)
   
3. ObjectTeamsTransformer.isWeavable() is identical to the first item, by rechecking we restrict any subsequent error reporting to classes that could in principal be woven.
   
4. clazz.needsWeaving() expresses requests from the class, due to actual bindings (callin or callout with decapsulation) that require weaving of this class. Basic weaving is performed even if needsWeaving() answers false, because "baseness" of a class may be detected much later of it is loaded.
   

• Either the loader is null, or the class falls outside the weavable regions (ot.weavable),
  
• And, the class is affected by (callin/callout) bindings
  

3) Overall understanding.

The joinpointId is a global unique Id which identifies every possible join point for the advices.
The callinId is a per binding identifier unique inside a team. It identifies a specific binding to a methodId.

4) Maven

The current provided Maven version does not work. I think there is a bug report about that because the compiler does not compile OTJ specific stuff.
How about to mavenize the whole build process, so one could actually work on using JMH to identify more problems? And even automate much more of the jar generation part. So for local testing a simple mvn install would suffice to use a new build local version in depending projects.

5) Some comments

The source level could be taken to Java 8 as Eclipse and JDT already require Java 8. The ASM library could be upgraded to ASM 5.2 (released 23rd Dec 16).

6) Compiler

Is there anything like compiler phases where one can hook in in order to add optimisations (e.g., on the AST)? I think it will currently just try to make the best Java code out of the OTJ code (which is already a good junk of work to do, no doubt).