Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [aspectj-users] Can I force around advice (especially for constructors) to be inlined?

This was a deliberate decision back in the day, not to inline.  By keeping them separate you could iterate on the aspect (change the advice) without needing to rebuild the whole system. It also, initially, was necessary to enable debugging to work properly because the mapping attributes weren't possible in the old days (where you indicate the content of one class file is the result of different source files).

The existence of the org.aspectj.matcher artifact as a separate entry to compiler/weaver was for cases where the matching infrastructure wanted to be used standalone. But I've never tried integrating it with another backend. Spring AOP does point cut matching but only against a subset of the possible joinpoints.

I think forcing inlining (or at least attempting to, there are cases where it is hard - I think around advice proceed cases) would be a good option to have these days.

cheers,
Andy

On Wed, 27 May 2020 at 20:58, Alexander Kriegisch <alexander@xxxxxxxxxxxxxx> wrote:

Actually it would be really good if inlining could be forced, e.g. globally with a compiler switch or per aspect/advice with an annotation, e.g. @Inline, if necessary with additional annotation parameters instructing the compiler to only inline into constructors or whatever. The annotation would work as a compiler directive in both native syntax and @AspectJ style.

As for this concrete case, of course it should be fixed in the compiler. It fails at least since Java 11 (I assume since Java 9 but I have no JDK 9 installed in order to test). The problem does not occur in Java 8.

The idea to (optionally) do more or even heavy inlining would be really helpful for people who want to retransform loaded classes and not just weave them during class loading. That would open up whole new ways of AspectJ application incl. JDK weaving during runtime. I have played with byte code engineering frameworks a bit lately, e.g. advising bootstrap classes via ByteBuddy advice. I have also implemented what I showed you here in Javassist, and it also works with bootstrap classes, even already loaded ones, as long as all transformations are inlined. The downside is the boilerplate I need in order to identify "joinpoints" for my transformations because I cannot use AspectJ's matcher. Well, in theory maybe I could (like Spring AOP does), but not the rest of AspectJ. There are so many interesting options with inlining, until three weeks ago I had no idea how much is possible.


--
Alexander Kriegisch
https://scrum-master.de

 

Andrew Clement schrieb am 28.05.2020 00:03 (GMT +07:00):

Of course, this is hard to detect by a compiler such as Ajc, but if there was an option to force inlining the around advice for the constructor, this scheme would work. So my questions are:

  1. Is there such an option? I did not find any obvious compiler switch.
  2. If not, do you see any chance to implement it, given technical feasibility?
The only force option you have is -XnoInline which forces the opposite of what you want ;)
 
Not 100% sure but the check about who can initially set some of those fields seems more policed lately than in older versions of Java. (There have been a few bugs reported about the ajc specific static initializer block setting final static fields that never triggered an access error before. I hadn’t seen it with instance fields though until you showed this).  What version of Java are you trying it on? Did you try it on Java 8 to compare? (Just out of interest, I’m not saying that is the fix).
 
I suspect we do need to do more inlining to fix this across the board.
 
 

On May 21, 2020, at 2:52 AM, Alexander Kriegisch <alexander@xxxxxxxxxxxxxx> wrote:

Today I got a tricky one. I thought about opening a Bugzilla ticket, but this is actually more of a question, maybe a future feature request if what I want is technically possible at all. So it depends on your answer if I shall open a ticket or not.

The requirement is simple (to explain, not to implement): I want to around-advise constructors in order suppress any side effects from happening there. The purpose would be to (ab)use AspectJ in order to create some kind of mock object which does not do anything expensive while being constructed. BTW, actually I am thinking about doing this with a library like ASM, but first I wanted to play with AspectJ in order to see what is possible there and create a proof of concept. But let me not get ahead of myself and set the stage first:

 

package de.scrum_master.app;

public class Base {
protected final int id;

public Base(int id) {
System.out.println("Constructing Base -> " + this);
this.id = id;
}
}

package de.scrum_master.app;

public class Sub extends Base{
private final String name;

public Sub(int id, String name) {
super(id);
System.out.println("Constructing Sub -> " + this);
this.name = name;
}

@Override
public String toString() {
return "Sub@" + this.hashCode() + " [name=" + name + ", id=" + id + "]";
}
}

package de.scrum_master.app;

public class AnotherSub extends Base{
private final String name;

public AnotherSub(int id, String name) {
super(id);
System.out.println("Constructing AnotherSub -> " + this);
this.name = name;
}

@Override
public String toString() {
return "AnotherSub@" + this.hashCode() + " [name=" + name + ", id=" + id + "]";
}
}

package de.scrum_master.aspect;

import de.scrum_master.app.Base;
import de.scrum_master.app.Sub;

public aspect ConstructorAspect {
void around() : execution(Base+.new(..)) && target(Sub) {
return;
}
}

package de.scrum_master.app;

public class Application {
public static void main(String[] args) {
System.out.println(new Sub(11, "Xander"));
System.out.println("----------");
System.out.println(new AnotherSub(22, "Someone"));
}
}

 

As you can see, I did the following:

  • Create a base class Base with two subclasses Sub and AnotherSub.
  • The aspect tries to
    • suppress constructor execution for one Sub (with the intent to "mock" it, just imagine its methods get advised by additional behaviour),
    • but not for AnotherSub (not a mock target) of the subclasses.
  • The aspect does this by
    • making sure that super class constructors are also suppressed via execution(Base+.new(..)),
    • excluding anything that is not the target type via target(Sub) and finally
    • simply not proceeding to the constructor by just returning from the around advice.

Now this works beautifully whenever creating a Sub instance, but fails whenever creating an AnotherSub instance:

 

Sub@1211076369 [name=null, id=0]
----------
Constructing Base -> AnotherSub@1551870003 [name=null, id=0]
Exception in thread "main" java.lang.IllegalAccessError: Update to non-static final field de.scrum_master.app.Base.id attempted from a different method (init$_aroundBody0) than the initializer method <init>
at de.scrum_master.app.Base.init$_aroundBody0(Base.java:8)
at de.scrum_master.app.Base.<init>(Base.java:6)
at de.scrum_master.app.AnotherSub.<init>(AnotherSub.java:7)
at de.scrum_master.app.Application.main(Application.java:7)

 

The explanation is pretty straightforward if we look at the console log and the class definitions:

  • The Base class has a final instance field.
  • AspectJ factors the Base constructor code out into a private helper method Base.init$_aroundBody0 and dispatches to it from the instrumented constructor Base.<init> instead of initialising the field directly from there.
  • While this does no harm for the "mock target" class Sub because there the helper method is never called (the no-op around advice kicks),
  • it fails miserably when creating an AnotherSub instance because then the helper method does get called but a helper method must not violate the JVM rule that final fields can only be initialised directly from a constructor (or during declaration).

Of course, this is hard to detect by a compiler such as Ajc, but if there was an option to force inlining the around advice for the constructor, this scheme would work. So my questions are:

  1. Is there such an option? I did not find any obvious compiler switch.
  2. If not, do you see any chance to implement it, given technical feasibility?
_______________________________________________
aspectj-users mailing list
aspectj-users@xxxxxxxxxxx
To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/aspectj-users

Back to the top