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

Optimize ordered event listener performance [SPR-14642] #19208

Closed
spring-projects-issues opened this issue Aug 30, 2016 · 5 comments
Closed

Optimize ordered event listener performance [SPR-14642] #19208

spring-projects-issues opened this issue Aug 30, 2016 · 5 comments
Assignees
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Aug 30, 2016

Christoph Dreis opened SPR-14642 and commented

Hey,

while doing some loadtests in our project, I noticed that AnnotatedElementUtils.searchWithFindSemantics() or rather AnnotatedElementUtils.findMergedAnnotation() are consuming quite some CPU. It's even above HashMap.getNode(int, Object) in some tests, which I find quite precarious for such a utility class. It looks like this is especially true for using @Order somewhere in the application in combination with @EventListener functionality.

I was wondering if this could be improved in any way? Maybe by caching merged annotation lookups much like in AnnotationUtils. Or maybe by just caching the order in the ApplicationListener context.

!annotated_element_utils_jmc.jpg|thumbnail!

Cheers,
Christoph


Affects: 4.3.2

Attachments:

Issue Links:

  • #16245 ApplicationListener-like annotation for consuming application events
  • #18020 Use AnnotatedElementUtils instead of AnnotationUtils wherever feasible

Referenced from: commits 58fa63f, 3b91dec

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Aug 30, 2016

Juergen Hoeller commented

Since findMergedAnnotation is indeed an expensive lookup call, we aim for caching the outcome in appropriate places. This is fortunately straightforward in the present case here: ApplicationListenerMethodAdapter initializes an order field on construction now, simply returning the value from getOrder().

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Aug 30, 2016

Christoph Dreis commented

Great. Saw your commit already. Just out of curiosity - are the ApplicationListenerMethodAdapter instances created only once at startup? Or are they created over and over again? The latter would be still quite intense for the new constructor behaviour, don't you think?

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Aug 30, 2016

Juergen Hoeller commented

EventListenerMethodProcessor creates one ApplicationListenerMethodAdapter per annotated method and registers the instance via ConfigurableApplicationContext.addApplicationListener directly, so we should be good here.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Aug 30, 2016

Christoph Dreis commented

Perfect. Thanks again :-)

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Aug 31, 2016

Stéphane Nicoll commented

I second that!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants