-
Notifications
You must be signed in to change notification settings - Fork 38.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Initial set of MergedAnnotations commits #22586
Closed
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
spring-projects-issues
added
the
status: waiting-for-triage
An issue we've not yet triaged or decided on
label
Mar 13, 2019
Add AssertJ as a test scope dependency for all sub-projects. Closes spring-projectsgh-22561
Add a public variant of `getDeclaredMethods` that defensively copies the cached methods array. This is often more faster and more convenient for users than calling `doWithLocalMethods`. We still retain most of the benefits of the cache, namely fewer security manager calls and not as many `Method` instances being created. Closes spring-projectsgh-22580
Add a new test class to help cover annotation introspection failure handling. These tests were previously missing and are important to ensure that annotation util code changes don't introduce regressions. See spring-projectsgh-21697
Add new `MergedAnnotations` and `MergedAnnotation` interfaces that attempt to provide a uniform way for dealing with merged annotations. Specifically, the new API provides alternatives for the static methods in `AnnotationUtils` and `AnnotatedElementUtils` and supports Spring's comprehensive annotation attribute `@AliasFor` features. The interfaces also open the possibility of the same API being exposed from the `AnnotationMetadata` interface. Additional utility classes for collecting, filtering and selecting annotations have also been added. Typical usage for the new API would be something like: MergedAnnotations.from(Example.class) .stream(MyAnnotation.class) .map(a -> a.getString("value")) .forEach(System.out::println); Closes spring-projectsgh-21697
Create internal variants of the existing `AnnotationUtils` and `AnnotatedElementUtils` classes and migrate the existing classes to use them. The internal variants will be used to check that the same results are given as we migrate the utils methods to use the new `MergedAnnotations` API. See spring-projectsgh-22562
Migrate all possible `AnnotationUtils` and `AnnotatedElementUtils` method to the `MergedAnnotation` API, verify results against the old implementations. All migrated methods now call both the new API and the old version and ensure that the same results or exceptions are raised. A full build of both Spring Framework and Spring Boot has been executed to ensure, as much as possible, that the migration does not cause unexpected regressions. See spring-projectsgh-22562
Delete `InternalAnnotationUtils` and `InternalAnnotatedElementUtils` and migrate exclusively to the new `MergedAnnotations` API. Closes spring-projectsgh-22562
Update `ConfigurationClassParser` to skip `java.lang.annotation` types which were often processed but would never provide useful results. Also use a single shared immutable `SourceClass` instance to represent `Object.class`. Closes spring-projectsgh-22563
Update `ConcurrentReferenceHashMap` to make some methods more inline friendly, and to manually inline a few others. These minor optimizations don't make a great deal of difference for most applications, but seem worthwhile since we use `ConcurrentReferenceHashMap` for many internal caches. Closes spring-projectsgh-22566
Add some additional empty array constants to `ReflectionUtils` to save us creating new arrays for zero length results. See spring-projectsgh-22567
Add an early exit to `StringUtils.cleanPath` to save array creating and string concatenation. With a typical Spring application, the `cleanPath` method can be called over 600 times, often with a path constructed by a `ClassPathResource` that is likely to already be clean. Closes spring-projectsgh-22568
Add a cache to `BridgeMethodResolver` to help with repeated calls to resolve the same methods. Since bridge method resolution can be somewhat expensive (especially when resolving generics), and the number of bridge methods is quite small, a cache generally helps. This commit also simplifies the code a little by calling `doWithMethods` directly rather than relying on `ReflectionUtils.getAllDeclaredMethods` to do so. The methods list is now always created, but we save the list creation that `getAllDeclaredMethods` used to do. Closes spring-projectsgh-22579
Refine the element filtering performed by `AnnotationsScanner` to also cover `org.springframework.util` and most `com.sun` classes which turn out to be referenced quite frequently and which we know contain no useful annotations. See spring-projectsgh-21697
jhoeller
added
type: task
A general task
in: core
Issues in core modules (aop, beans, core, context, expression)
and removed
status: waiting-for-triage
An issue we've not yet triaged or decided on
labels
Mar 14, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is the initial set of work in introducing a new
MergedAnnotations
API to Spring Framework. See #22560 for more background.It seemed more sensible to send a single PR covering a number of issues, rather than creating distinct PRs for each subtask.
In order to reduce the likelihood of regressions, I've ran a complete build of both Spring Framework and Spring Boot which calls both the original and new code and compares the results. I've left these commits in this PR as I figure they might be a useful reference.
I've also used https://github.com/wilkinsona/spring-boot-benchmark-harness to check that there are no major performance implications with the new code. The following shows the Spring Boot Tomcat sample application running against a baseline from c0ddaae.
Special thanks should go to @wilkinsona who helped profile and identify several performance improvements.