Skip to content
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

monitor contention at org.springframework.core.MethodParameter.getParameterAnnotations() [SPR-9298] #13936

Closed
spring-projects-issues opened this issue Apr 2, 2012 · 5 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) in: web Issues in web modules (web, webmvc, webflux, websocket) status: duplicate A duplicate of another issue type: enhancement A general enhancement

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Apr 2, 2012

Nikita Tovstoles opened SPR-9298 and commented

On each web request, RequestMappingHandlerAdapter.invokeHandlerMethod() indirectly calls MethodParameter.getParameterAnnotations(), which calls into JRE lib method causing monitor contention. See screen shots & YourKit snapshot. Most likely will need to patch spring (caching results of calls to java.lang.reflect.Method.getParameterAnnotations())

Looking at Spring's MethodParameter:

public Annotation[] getParameterAnnotations() {
     if (this.parameterAnnotations == null) {
          Annotation[][] annotationArray = (this.method != null ?
                        this.method.getParameterAnnotations() : this.constructor.getParameterAnnotations());
          if (this.parameterIndex >= 0 && this.parameterIndex < annotationArray.length) {
               this.parameterAnnotations = annotationArray[this.parameterIndex];
          }
          else {
               this.parameterAnnotations = new Annotation[0];
          }
     }
     return this.parameterAnnotations;
}

results of calls on Method & Constructor:

Annotation[][] annotationArray = (this.method != null ?
              this.method.getParameterAnnotations() : this.constructor.getParameterAnnotations());

should be cached in two static Maps (since MethodParameter object is recreated per request)


Affects: 3.1 GA

Attachments:

Issue Links:

0 votes, 7 watchers

@spring-projects-issues
Copy link
Collaborator Author

Nikita Tovstoles commented

Git pull request containing my suggested fix created

@spring-projects-issues
Copy link
Collaborator Author

Chris Beams commented

Thanks, Nikita. See pull request for comments about the implementation.

commit c10d63dc011c95b3e24d4ed8d7140c73053736c6
Author: Nikita Tovstoles <nikita.tovstoles@gmail.com>
Date:   Tue Apr 3 15:33:41 2012 -0700

    Cache MethodParameter annotation lookup results
    
    Prior to this change, Spring's MethodParameter#getParameterAnnotations
    called java.lang.Method#getParameterAnnotations on every invocation.
    The latter ends up contending for a monitor inside (Sun) JDK code. This
    is problematic when dealing with the high number of @RequestMapping
    invocations that can occur in a Spring MVC @Controller.
    
    This commit eliminates this contention by caching values returned by
    java.lang.Method#getParameterAnnotations in a static ConcurrentMap.
    
    Note that only Method parameter annotations are cached, while
    Constructor parameter annotations are not. This is because the
    issue of primary concern is, as mentioned above, @RequestMapping
    methods. By nature, constructors are called much more infrequently, and
    in most cases in a single-threaded fashion.
    
    Issue: SPR-9298

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

We had to back this change out for 3.2 GA since it can be the source of a ClassLoader leak when the Spring jars live in a common lib directory but operate against classes from a specific web application's lib directory.

There might be other ways of achieving a caching effect for Spring MVC there, trying to cache MethodParameter objects in MVC processing or passing a common cache structure into newly created MethodParameter objects, similar to how we do it with the ParameterNameDiscoverer already.

Rossen, I've assigned this to you for the time being since it primarily affects Spring MVC. Let's have a joint look at this in the 3.2.1 timeframe.

Juergen

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Jan 21, 2013

Rossen Stoyanchev commented

I believe we already have a fix for this with #14382, which is available in 3.1.3. If you could give a try and confirm that would be great!

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Jan 22, 2013

Rossen Stoyanchev commented

Resolving as duplicate of #14382.

@spring-projects-issues spring-projects-issues added type: enhancement A general enhancement in: core Issues in core modules (aop, beans, core, context, expression) status: duplicate A duplicate of another issue in: web Issues in web modules (web, webmvc, webflux, websocket) labels Jan 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) in: web Issues in web modules (web, webmvc, webflux, websocket) status: duplicate A duplicate of another issue type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

2 participants