Skip to content

Commit

Permalink
Avoid synthesizing annotations unnecessarily
Browse files Browse the repository at this point in the history
Commit 31fa156 introduced initial support for avoiding unnecessary
annotation synthesis in the MergedAnnotation API; however, it only
avoided synthesis for annotations that do not declare any attributes.

This commit reworks this support to avoid unnecessary annotation
synthesis for annotations that declare attributes.

Specifically, this commit introduces a new `isSynthesizable()` method
in AnnotationTypeMapping that allows the "synthesizable" flag to be
computed once and cached along with the other metadata already cached
in AnnotationTypeMapping instances. TypeMappedAnnotation now delegates
to this new method when determining whether it should synthesize an
annotation.

Closes gh-24861
  • Loading branch information
sbrannen committed Apr 23, 2020
1 parent 0520ee0 commit 8265db0
Show file tree
Hide file tree
Showing 4 changed files with 104 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@ final class AnnotationTypeMapping {

private final Map<Method, List<Method>> aliasedBy;

private final boolean synthesizable;

private final Set<Method> claimedAliases = new HashSet<>();


Expand All @@ -101,6 +103,7 @@ final class AnnotationTypeMapping {
processAliases();
addConventionMappings();
addConventionAnnotationValues();
this.synthesizable = computeSynthesizableFlag();
}


Expand Down Expand Up @@ -307,6 +310,47 @@ private boolean isBetterConventionAnnotationValue(int index, boolean isValueAttr
return !isValueAttribute && existingDistance > mapping.distance;
}

@SuppressWarnings("unchecked")
private boolean computeSynthesizableFlag() {
// Uses @AliasFor for local aliases?
for (int index : this.aliasMappings) {
if (index != -1) {
return true;
}
}

// Uses @AliasFor for attribute overrides in meta-annotations?
if (!this.aliasedBy.isEmpty()) {
return true;
}

// Uses convention-based attribute overrides in meta-annotations?
for (int index : this.conventionMappings) {
if (index != -1) {
return true;
}
}

// Has nested annotations or arrays of annotations that are synthesizable?
if (getAttributes().hasNestedAnnotation()) {
AttributeMethods attributeMethods = getAttributes();
for (int i = 0; i < attributeMethods.size(); i++) {
Method method = attributeMethods.get(i);
Class<?> type = method.getReturnType();
if (type.isAnnotation() || (type.isArray() && type.getComponentType().isAnnotation())) {
Class<? extends Annotation> annotationType =
(Class<? extends Annotation>) (type.isAnnotation() ? type : type.getComponentType());
AnnotationTypeMapping mapping = AnnotationTypeMappings.forAnnotationType(annotationType).get(0);
if (mapping.isSynthesizable()) {
return true;
}
}
}
}

return false;
}

/**
* Method called after all mappings have been set. At this point no further
* lookups from child mappings will occur.
Expand Down Expand Up @@ -478,6 +522,17 @@ MirrorSets getMirrorSets() {
return this.mirrorSets;
}

/**
* Determine if the mapped annotation is <em>synthesizable</em>.
* <p>Consult the documentation for {@link MergedAnnotation#synthesize()}
* for an explanation of what is considered synthesizable.
* @return {@code true} if the mapped annotation is synthesizable
* @since 5.2.6
*/
boolean isSynthesizable() {
return this.synthesizable;
}


private static int[] filledIntArray(int size) {
int[] array = new int[size];
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2019 the original author or authors.
* Copyright 2002-2020 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -479,11 +479,23 @@ <T extends Annotation> MergedAnnotation<T>[] getAnnotationArray(String attribute
<T extends Map<String, Object>> T asMap(Function<MergedAnnotation<?>, T> factory, Adapt... adaptations);

/**
* Create a type-safe synthesized version of this annotation that can be
* used directly in code.
* Create a type-safe synthesized version of this merged annotation that can
* be used directly in code.
* <p>The result is synthesized using a JDK {@link Proxy} and as a result may
* incur a computational cost when first invoked.
* @return a synthesized version of the annotation.
* <p>If this merged annotation was created {@linkplain #from(Annotation) from}
* an annotation instance, that annotation will be returned unmodified if it is
* not <em>synthesizable</em>. An annotation is considered synthesizable if
* one of the following is true.
* <ul>
* <li>The annotation declares attributes annotated with {@link AliasFor @AliasFor}.</li>
* <li>The annotation is a composed annotation that relies on convention-based
* annotation attribute overrides in meta-annotations.</li>
* <li>The annotation declares attributes that are annotations or arrays of
* annotations that are themselves synthesizable.</li>
* </ul>
* @return a synthesized version of the annotation or the original annotation
* unmodified
* @throws NoSuchElementException on a missing annotation
*/
A synthesize() throws NoSuchElementException;
Expand All @@ -493,8 +505,10 @@ <T extends Annotation> MergedAnnotation<T>[] getAnnotationArray(String attribute
* on a condition predicate.
* <p>The result is synthesized using a JDK {@link Proxy} and as a result may
* incur a computational cost when first invoked.
* <p>Consult the documentation for {@link #synthesize()} for an explanation
* of what is considered synthesizable.
* @param condition the test to determine if the annotation can be synthesized
* @return a optional containing the synthesized version of the annotation or
* @return an optional containing the synthesized version of the annotation or
* an empty optional if the condition doesn't match
* @throws NoSuchElementException on a missing annotation
* @see MergedAnnotationPredicates
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -331,19 +331,20 @@ private <T extends Map<String, Object>> Object adaptValueForMapOptions(Method at
@Override
@SuppressWarnings("unchecked")
protected A createSynthesized() {
if (this.rootAttributes instanceof Annotation) {
// Nothing to synthesize?
if (this.resolvedRootMirrors.length == 0 && this.resolvedMirrors.length == 0) {
return (A) this.rootAttributes;
}
// Already synthesized?
if (this.rootAttributes instanceof SynthesizedAnnotation) {
return (A) this.rootAttributes;
}
if (getType().isInstance(this.rootAttributes) && !isSynthesizable()) {
return (A) this.rootAttributes;
}
return SynthesizedMergedAnnotationInvocationHandler.createProxy(this, getType());
}

private boolean isSynthesizable() {
// Already synthesized?
if (this.rootAttributes instanceof SynthesizedAnnotation) {
return false;
}
return this.mapping.isSynthesizable();
}

@Override
public String toString() {
String string = this.string;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2019 the original author or authors.
* Copyright 2002-2020 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -1380,14 +1380,22 @@ void synthesizeAlreadySynthesized() throws Exception {
@Test
void synthesizeShouldNotSynthesizeNonsynthesizableAnnotations() throws Exception {
Method method = getClass().getDeclaredMethod("getId");

Id id = method.getAnnotation(Id.class);
assertThat(id).isNotNull();

Id synthesizedId = MergedAnnotation.from(id).synthesize();
assertThat(id).isEqualTo(synthesizedId);
// It doesn't make sense to synthesize {@code @Id} since it declares zero attributes.
// It doesn't make sense to synthesize @Id since it declares zero attributes.
assertThat(synthesizedId).isNotInstanceOf(SynthesizedAnnotation.class);
assertThat(id).isSameAs(synthesizedId);

GeneratedValue generatedValue = method.getAnnotation(GeneratedValue.class);
assertThat(generatedValue).isNotNull();
GeneratedValue synthesizedGeneratedValue = MergedAnnotation.from(generatedValue).synthesize();
assertThat(generatedValue).isEqualTo(synthesizedGeneratedValue);
// It doesn't make sense to synthesize @GeneratedValue since it declares zero attributes with aliases.
assertThat(synthesizedGeneratedValue).isNotInstanceOf(SynthesizedAnnotation.class);
assertThat(generatedValue).isSameAs(synthesizedGeneratedValue);
}

/**
Expand Down Expand Up @@ -2987,7 +2995,16 @@ public void handleMappedWithDifferentPathAndValueAttributes() {
@interface Id {
}

/**
* Mimics javax.persistence.GeneratedValue
*/
@Retention(RUNTIME)
@interface GeneratedValue {
String strategy();
}

@Id
@GeneratedValue(strategy = "AUTO")
private Long getId() {
return 42L;
}
Expand Down

0 comments on commit 8265db0

Please sign in to comment.