Skip to content

Gradle incremental annotation rocessing#373

Open
bric3 wants to merge 2 commits intodoanduyhai:masterfrom
bric3:gradle-incremental-annotation-rocessing
Open

Gradle incremental annotation rocessing#373
bric3 wants to merge 2 commits intodoanduyhai:masterfrom
bric3:gradle-incremental-annotation-rocessing

Conversation

@bric3
Copy link
Contributor

@bric3 bric3 commented Feb 7, 2021

Hi @doanduyhai

I'd liek to propose the following enhancement: Gradle supports incremental annotation processing, meaning the annotation processor won't have to run on every new compilation.

For that there is a simple guide to follow there to make sure the annotation processor is eligible to incremental annotation processing: https://docs.gradle.org/current/userguide/java_plugin.html#sec:incremental_annotation_processing

If I'm not mistaken achilles core is ok with these points :

  • They must generate their files using the Filer API. Writing files any other way will result in silent failures later on, as these files won’t be cleaned up correctly. If your processor does this, it cannot be incremental.

  • They must not depend on compiler-specific APIs like com.sun.source.util.Trees. Gradle wraps the processing APIs, so attempts to cast to compiler-specific types will fail. If your processor does this, it cannot be incremental, unless you have some fallback mechanism.

  • If they use Filer#createResource, the location argument must be one of these values from StandardLocation: CLASS_OUTPUT, SOURCE_OUTPUT, or NATIVE_HEADER_OUTPUT. Any other argument will disable incremental processing.

I believe this annotation processor falls aggregatig category as it reaches to other elements like codecs or compiler registry. I followed the specific constraints :

"Aggregating" processors have the following limitations:

  • They can only read CLASS or RUNTIME retention annotations
  • They can only read parameter names if the user passes the -parameters compiler argument.

The changes looks to be minimal.

If possible it would be great to backport this change to the 5.3.x line.

Thank you in advance for the consideration.

bric3 added 2 commits February 7, 2021 14:13
There's a few constraints that are mentioned on the gradle doc
where annotation processors must abide in order to profit from
incremental annotation processing, thanks to the current code
and best practice the current processor seems to be within the
supported scope.

https://docs.gradle.org/current/userguide/java_plugin.html#sec:incremental_annotation_processing
@doanduyhai
Copy link
Owner

Thanks for the PR @bric3

Unfortunately, Achilles processor cannot be incremental for several reasons:

  1. It does not use the Filer API. Instead it is using com.squareup.javapoet.JavaFile to generate the source classes: https://github.com/doanduyhai/Achilles/blob/master/achilles-core/src/main/java/info/archinnov/achilles/internals/apt/processors/meta/AchillesProcessor.java#L118-L162

  2. It does relies on internal Sun compiler classes to parse nested structures like Map<String, @Frozen MyUDT>:

    } else if (isJavaCompiler(varElm)) {
    final Map<Class<? extends Annotation>, TypedMap> annotationInfo = varElm.getAnnotationMirrors()
    .stream()
    .filter(x ->
    areSameByClass(x, JSON.class) ||
    areSameByClass(x, EmptyCollectionIfNull.class) ||
    areSameByClass(x, Enumerated.class) ||
    areSameByClass(x, Frozen.class) ||
    areSameByClass(x, Computed.class) ||
    areSameByClass(x, Counter.class) ||
    areSameByClass(x, TimeUUID.class) ||
    areSameByClass(x, ASCII.class) ||
    areSameByClass(x, Codec.class) ||
    areSameByClass(x, RuntimeCodec.class) ||
    areSameByClass(x, Index.class) ||
    areSameByClass(x, SASI.class) ||
    areSameByClass(x, DSE_Search.class) ||
    areSameByClass(x, PartitionKey.class) ||
    areSameByClass(x, ClusteringColumn.class)
    )
    .map(x -> (AnnotationMirror) x)
    .collect(Collectors.toMap(x -> toAnnotation_Javac(aptUtils, x),
    x -> inspectSupportedAnnotation_Javac(aptUtils, currentType, x)));
    final AnnotationTree annotationTree = new AnnotationTree(currentType, annotationInfo, 1);
    final SymbolMetadata metadata = ((Symbol.VarSymbol) varElm).getMetadata();

At that time, this was the only solution to parse nested annotations inside generic types. I don't know if since then things have improved on the front of Java annotation processor to avoid using internal classes. If so I'll be happy to refactor this part of the code to get rid of the Sun internal classes

@bric3
Copy link
Contributor Author

bric3 commented Feb 7, 2021

Oh I missed those 😅.

I have seen other annotation processors using JavaPoet and activating Gradle's incremental processing. In preliminary tests this worked either in isolating and aggregating modes, but the model don't rely on UDT. Maybe I was mislead.

Thanks for the quick feedback and the consideration of this feature!

@doanduyhai
Copy link
Owner

My bad. The code https://github.com/doanduyhai/Achilles/blob/master/achilles-core/src/main/java/info/archinnov/achilles/internals/apt/processors/meta/AchillesProcessor.java#L118-L162 still uses the Filer API in the end. It is just handled by com.squareup.javapoet.JavaFile class itself: https://github.com/square/javapoet/blob/8ebce31cd655cf181ebdde59dcf04c50cbda7264/src/main/java/com/squareup/javapoet/JavaFile.java#L164

However the 2nd issue still remains, if I stop using Sun javac internal class, it will break all support of nested annotations

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants