remove old asm & AnnotationMetadataReader supporting classes; adopt SimpleMetadataReaderFactory

[jdaugherty]

From what I can tell, AnnotationMetadataReader was written for performance reasons. Since Spring 5.2, this was deprecated and finally removed in 6.x. We had copied the file from Spring to get Grails 7 working, but I'm not sure this was the right decision.

Searching the internet + using chatgpt, I noticed this document. Out of curiosity, I removed the AnnotationMetadataReader & replaced it with the upstream Spring reader. All of the tests pass.

I wanted to create this PR as a discussion point since:

1. AnnotationMetadataReader doesn't support @Repeatable
2. Spring acknowledged the performance issues via the rewrite
3. The newer method is both thread safe & readonly - so we could technically cache it now
4. To get spring native working they had to adopt this. I'm not sure we'll ever be able to run with graalvm, but taking steps to support this seems reasonable.
5. This would also have the benefit of simplifying our NOTICE / LICENSE files since both the ASM library & code from Spring can be removed.

I'm going to leave this open for discussion in next week's weekly dev meeting.

[jamesfredley]

This is a good replacement for the changes made in 3.1.x and 7.0.x:

679acbe

a6dd1ec

[matrei]

Won't this put as back to where we were before the changes in 679acbe?
Back to using a MetadataReader, which had performance problems as it reads the entire class file and not just the annotations?

[matrei]

From what I can tell, AnnotationMetadataReader was written for performance reasons. Since Spring 5.2, this was deprecated and finally removed in 6.x.

I don't think the Spring classes were removed in 6.x. But rather replaced by final, package protected classes for internal use only. Hence we had to copy them.

https://docs.spring.io/spring-framework/docs/5.3.9/javadoc-api/org/springframework/core/type/classreading/AnnotationMetadataReadingVisitor.html

As of Spring Framework 5.2, this class has been replaced by SimpleAnnotationMetadataReadingVisitor for internal use within the framework, but there is no public replacement for AnnotationMetadataReadingVisitor.

[jamesfredley]

Is there a reasonable simple way to compare performance and see if the changes made during https://github.com/spring-projects/spring-framework/wiki/MergedAnnotation-API-internals come close?

[jdaugherty]

@matrei We do not need the visitor, only the factory. In the event we need to customize it, we can simply fork it again.

One of the reasons Spring rewrote it was because of performance. The other major concern is the old implementation had several known issues - from bugs to not supporting graalvm.

@jamesfredley the more I think about this, I think its more important to be correct. If it does
introduce a performance regression we can fork the Spring version and apply a similar approach that Graeme did. However, if it was significantly slower we would have seen it between builds. I suspect it will only affect larger apps, like mine. In which case we will just have to do further work at that time.

[matrei]

We do not need the visitor, only the factory.

Yes, if we are not going to use Graeme's AnnotationMetadataReader, this is true.

Is there a reasonable simple way to compare performance and see if the changes made during https://github.com/spring-projects/spring-framework/wiki/MergedAnnotation-API-internals come close?

This would be very interesting to see. I doubt Graeme would have gone through the trouble to implement AnnotationMetadataReader if the performance gain was marginal. But I have no idea what kind of application is needed to see a difference.