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

Scheduled/JmsListenerAnnotationBeanPostProcessor needlessly scans every scoped instance [SPR-12189] #16803

Closed
spring-projects-issues opened this issue Sep 13, 2014 · 3 comments
Assignees
Labels
in: core status: backported type: enhancement
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Sep 13, 2014

Jean-Baptiste Nizet opened SPR-12189 and commented

ScheduledAnnotationBeanPostProcessor scans the methods of all the beans created by Spring for @Scheduled annotations. But this also includes, in a web application, all the request- and session-scoped beans, created long after the context has been started and the tasks have been scheduled.
It thus has two implications:

  • if a Scheduled annotation is placed on a request-scoped bean (stupid, I know), a new Runnable is created at each request, referencing the request-scoped bean, and placed in a list of the registrar, which causes a big memory leak
  • several classes are needlessly scanned at each request for nothing if request-scoped beans are used in the application, consituting a loss of performance, that grows with the number of request-scoped beans used.

A flag could be set to true as soon as afterSingletonsInstantiated() has been called, and the postProcessAfterInitialization() method would do nothing if this flag is set.


Affects: 3.2.11, 4.0.7, 4.1 GA

Issue Links:

  • #17306 @Scheduled no longer works in case of multiple proxied target classes implementing the same interface
  • #16830 ScheduledAnnotationBeanPostProcessor should unregister tasks on destruction of individual beans
  • #19741 Scheduled/JmsListenerAnnotationBeanPostProcessor free heap space

Referenced from: commits 58b22ce, 37da706, d2e8b7e

Backported to: 4.0.8, 3.2.12

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Sep 17, 2014

Juergen Hoeller commented

The needless scanning of every scoped bean instance also applies to the newly introduced JmsListenerAnnotationBeanPostProcessor. I'll use this JIRA issue to track that performance impact of both, also for backporting.

As for the general concern of tracking non-singleton instances with @Scheduled annotations on them, causing a memory leak, let's create a separate JIRA issue for it. We should arguably either disallow it (hard at this point) or make sure we unregister such callbacks on individual destruction of each instance.

Juergen

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Sep 17, 2014

Juergen Hoeller commented

We're avoiding needless re-scans through caching non-annotated classes in both ScheduledListenerAnnotationBeanPostProcessor and JmsListenerAnnotationBeanPostProcessor now. We're not preventing scheduling on non-singleton beans since this may be desired in some scenarios and can easily be avoided if not desired.

Automatic cleanup on destruction of non-singleton beans is a follow-up improvement which I've put into the 4.x Backlog: #16830.

Juergen

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Sep 17, 2014

Jean-Baptiste Nizet commented

At least in the case of ScheduledAnnotationBeanPostProcessor, I fail to see the point in continuing to process beans instantiated after the singletons are initialized.
Indeed, once the singletons are instantiated, the method afterSingletonsInstantiated() is called, which in turn calls the method afterPropertiesSet()of the ScheduledTaskRegistrar, which submits the tasks to the task scheduler. Unless I miss something, post-processing beans instantiated after that will create tasks and add them to the registrar, but these tasks will never be submitted to the scheduler anyway. So it's a waste of time and memory.

It seems that JmsListenerAnnotationBeanPostProcessor is designed the same way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core status: backported type: enhancement
Projects
None yet
Development

No branches or pull requests

2 participants