From: Pete Muir <pmuir(a)redhat.com&gt; To: cdi-dev <cdi-dev(a)lists.jboss.org&gt; Cc: Sent: Thursday, August 16, 2012 1:43 PM Subject: [cdi-dev] Mutable annotation literals All Marko has added caching for the hashcode value on AnnotationLiteral, which is based on the premise that an AnnotationLiteral is not mutable. https://github.com/luksa/cdi/commit/f3c200fcdf8ca8681c194c921567aedcce375102 This is a reasonable thing to require (as real annotations are normally immutable!) and a very good optimisation, however it is not backwards compatible, as we have not previously required AnnotationLiteral to be immutable. An alternative option is to introduce the ImmutableAnnotationLiteral subclass, which would require any concrete instances to be immutable and could use this optimisation. We could "deprecate" AnnotationLiteral (as described in java.net/projects/javaee-spec/pages/CompatibilityRequirements). Note that regardless, we can apply the minor optimisation suggested by Mark, that we return 0 if the annotation has no members. Pete _______________________________________________ cdi-dev mailing list cdi-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/cdi-dev

From: Pete Muir <pmuir(a)redhat.com&gt; To: Mark Struberg <struberg(a)yahoo.de&gt; Cc: cdi-dev <cdi-dev(a)lists.jboss.org&gt; Sent: Thursday, August 16, 2012 2:20 PM Subject: Re: [cdi-dev] Mutable annotation literals I guess what Marko/I am proposing is that we provide a utility for the common case of an immutable annotation literal with member values... On 16 Aug 2012, at 13:18, Mark Struberg wrote: > Imo we don't need to cache the hashCode in the AnnotationLiteral itself. We just need to cache if there are value methods or not. If not, then we can immediately return 0. If there are value methods present, then you need to extend the AnnotationLiteral class anyway and can add an own hashCode which delegates to super.hashCode() and caches the result if the values will not be changed. > > LieGrue, > strub > > > > > ----- Original Message ----- >> From: Pete Muir <pmuir(a)redhat.com&gt; >> To: cdi-dev <cdi-dev(a)lists.jboss.org&gt; >> Cc: >> Sent: Thursday, August 16, 2012 1:43 PM >> Subject: [cdi-dev] Mutable annotation literals >> >> All >> >> Marko has added caching for the hashcode value on AnnotationLiteral, which is >> based on the premise that an AnnotationLiteral is not mutable. >> >> https://github.com/luksa/cdi/commit/f3c200fcdf8ca8681c194c921567aedcce375102 >> >> This is a reasonable thing to require (as real annotations are normally >> immutable!) and a very good optimisation, however it is not backwards >> compatible, as we have not previously required AnnotationLiteral to be >> immutable. >> >> An alternative option is to introduce the ImmutableAnnotationLiteral subclass, >> which would require any concrete instances to be immutable and could use this >> optimisation. We could "deprecate" AnnotationLiteral (as described in >> java.net/projects/javaee-spec/pages/CompatibilityRequirements). >> >> Note that regardless, we can apply the minor optimisation suggested by Mark, >> that we return 0 if the annotation has no members. >> >> Pete >> _______________________________________________ >> cdi-dev mailing list >> cdi-dev(a)lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/cdi-dev >>

On 16 Aug 2012, at 13:58, Mark Struberg wrote: > The problem is an AnnotationLiteral could theoretically be mutable as well. Thus the AnnotationLiteral class must not make such assumptions imo. Agreed. > > > I the case of an empty value-methods list we could btw also improve the equals method. Yes. > > In the case where you have value-methods, you already need to implement the interface manually. It would be no problem to just add the 2 lines for caching the hashCode in case the values do not change. Except that the hash code rules for annotations aren't that easy to follow.

> > > LieGrue, > strub > > > > ----- Original Message ----- >> From: Pete Muir <pmuir(a)redhat.com&gt; >> To: Mark Struberg <struberg(a)yahoo.de&gt; >> Cc: cdi-dev <cdi-dev(a)lists.jboss.org&gt; >> Sent: Thursday, August 16, 2012 2:20 PM >> Subject: Re: [cdi-dev] Mutable annotation literals >> >> I guess what Marko/I am proposing is that we provide a utility for the common >> case of an immutable annotation literal with member values... >> >> On 16 Aug 2012, at 13:18, Mark Struberg wrote: >> >>> Imo we don't need to cache the hashCode in the AnnotationLiteral >> itself. We just need to cache if there are value methods or not. If not, then we >> can immediately return 0. If there are value methods present, then you need to >> extend the AnnotationLiteral class anyway and can add an own hashCode which >> delegates to super.hashCode() and caches the result if the values will not be >> changed. >>> >>> LieGrue, >>> strub >>> >>> >>> >>> >>> ----- Original Message ----- >>>> From: Pete Muir <pmuir(a)redhat.com&gt; >>>> To: cdi-dev <cdi-dev(a)lists.jboss.org&gt; >>>> Cc: >>>> Sent: Thursday, August 16, 2012 1:43 PM >>>> Subject: [cdi-dev] Mutable annotation literals >>>> >>>> All >>>> >>>> Marko has added caching for the hashcode value on AnnotationLiteral, >> which is >>>> based on the premise that an AnnotationLiteral is not mutable. >>>> >>>> >> https://github.com/luksa/cdi/commit/f3c200fcdf8ca8681c194c921567aedcce375102 >>>> >>>> This is a reasonable thing to require (as real annotations are normally >> >>>> immutable!) and a very good optimisation, however it is not backwards >>>> compatible, as we have not previously required AnnotationLiteral to be >>>> immutable. >>>> >>>> An alternative option is to introduce the ImmutableAnnotationLiteral >> subclass, >>>> which would require any concrete instances to be immutable and could >> use this >>>> optimisation. We could "deprecate" AnnotationLiteral (as >> described in >>>> java.net/projects/javaee-spec/pages/CompatibilityRequirements). >>>> >>>> Note that regardless, we can apply the minor optimisation suggested by >> Mark, >>>> that we return 0 if the annotation has no members. >>>> >>>> Pete >>>> _______________________________________________ >>>> cdi-dev mailing list >>>> cdi-dev(a)lists.jboss.org >>>> https://lists.jboss.org/mailman/listinfo/cdi-dev >>>> >>

> Except that the hash code rules for annotations aren't that easy to follow. Exactly! That was the reason why I made the sample with the super.hashCode() code. For any 'constant' AnnotationLiteral you just additionally add a few lines which 'caches' the hashCode. public class TransactionScopedLiteral extends AnnotationLiteral<TransactionScoped> implements TransactionScoped { private Integer hashCode = null; public int hashCode() { if (hashCode == null) { hashCode = super.hashCode(); } return hashCode; } ... } we could of course also add a new additional constructor with a 'boolean constant' parameter LieGrue, strub ----- Original Message ----- > From: Pete Muir <pmuir(a)redhat.com&gt; > To: Mark Struberg <struberg(a)yahoo.de&gt; > Cc: cdi-dev <cdi-dev(a)lists.jboss.org&gt; > Sent: Thursday, August 16, 2012 3:01 PM > Subject: Re: [cdi-dev] Mutable annotation literals > > > On 16 Aug 2012, at 13:58, Mark Struberg wrote: > >> The problem is an AnnotationLiteral could theoretically be mutable as well. > Thus the AnnotationLiteral class must not make such assumptions imo. > > Agreed. > >> >> >> I the case of an empty value-methods list we could btw also improve the > equals method. > > Yes. > >> >> In the case where you have value-methods, you already need to implement the > interface manually. It would be no problem to just add the 2 lines for caching > the hashCode in case the values do not change. > > Except that the hash code rules for annotations aren't that easy to follow. > >> >> >> LieGrue, >> strub >> >> >> >> ----- Original Message ----- >>> From: Pete Muir <pmuir(a)redhat.com&gt; >>> To: Mark Struberg <struberg(a)yahoo.de&gt; >>> Cc: cdi-dev <cdi-dev(a)lists.jboss.org&gt; >>> Sent: Thursday, August 16, 2012 2:20 PM >>> Subject: Re: [cdi-dev] Mutable annotation literals >>> >>> I guess what Marko/I am proposing is that we provide a utility for the > common >>> case of an immutable annotation literal with member values... >>> >>> On 16 Aug 2012, at 13:18, Mark Struberg wrote: >>> >>>> Imo we don't need to cache the hashCode in the > AnnotationLiteral >>> itself. We just need to cache if there are value methods or not. If > not, then we >>> can immediately return 0. If there are value methods present, then you > need to >>> extend the AnnotationLiteral class anyway and can add an own hashCode > which >>> delegates to super.hashCode() and caches the result if the values will > not be >>> changed. >>>> >>>> LieGrue, >>>> strub >>>> >>>> >>>> >>>> >>>> ----- Original Message ----- >>>>> From: Pete Muir <pmuir(a)redhat.com&gt; >>>>> To: cdi-dev <cdi-dev(a)lists.jboss.org&gt; >>>>> Cc: >>>>> Sent: Thursday, August 16, 2012 1:43 PM >>>>> Subject: [cdi-dev] Mutable annotation literals >>>>> >>>>> All >>>>> >>>>> Marko has added caching for the hashcode value on > AnnotationLiteral, >>> which is >>>>> based on the premise that an AnnotationLiteral is not mutable. >>>>> >>>>> >>> > https://github.com/luksa/cdi/commit/f3c200fcdf8ca8681c194c921567aedcce375102 >>>>> >>>>> This is a reasonable thing to require (as real annotations are > normally >>> >>>>> immutable!) and a very good optimisation, however it is not > backwards >>>>> compatible, as we have not previously required > AnnotationLiteral to be >>>>> immutable. >>>>> >>>>> An alternative option is to introduce the > ImmutableAnnotationLiteral >>> subclass, >>>>> which would require any concrete instances to be immutable and > could >>> use this >>>>> optimisation. We could "deprecate" AnnotationLiteral > (as >>> described in >>>>> java.net/projects/javaee-spec/pages/CompatibilityRequirements). >>>>> >>>>> Note that regardless, we can apply the minor optimisation > suggested by >>> Mark, >>>>> that we return 0 if the annotation has no members. >>>>> >>>>> Pete >>>>> _______________________________________________ >>>>> cdi-dev mailing list >>>>> cdi-dev(a)lists.jboss.org >>>>> https://lists.jboss.org/mailman/listinfo/cdi-dev >>>>> >>> > _______________________________________________ cdi-dev mailing list cdi-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/cdi-dev