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

EventListenerMethodProcessor causes expensive introspection of lazy beans #22293

Closed
garyrussell opened this issue Jan 22, 2019 · 5 comments
Closed
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: invalid An issue that we don't feel is valid type: enhancement A general enhancement

Comments

@garyrussell
Copy link
Contributor

Affects: 5.1.4

I was attempting to fix spring-projects/spring-amqp#885 to avoid eager initialization of a lazy factory bean as reported in the linked Stack Overflow question.

After fixing it, I found the FB is still instantiated, but by the EventListenerMethodProcessor instead...

public void afterSingletonsInstantiated() {
ConfigurableListableBeanFactory beanFactory = this.beanFactory;
Assert.state(this.beanFactory != null, "No ConfigurableListableBeanFactory set");
String[] beanNames = beanFactory.getBeanNamesForType(Object.class);

@snicoll
Copy link
Member

snicoll commented Jan 22, 2019

There are a number of candidates that would do that sort of things. @EventListener and @JmsListener come to mind. If there is a more general approach we could use to do that stuff, It should be applied most probably to other projects as well.

ping @jhoeller

@garyrussell
Copy link
Contributor Author

The simple fix would be beanFactory.getBeanNamesForType(Object.class, false, false); - but that would mean a behavior change because any lazy beans with @EventListener won't get events.

@sdeleuze sdeleuze added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jan 23, 2019
@rstoyanchev rstoyanchev added in: messaging Issues in messaging modules (jms, messaging) in: core Issues in core modules (aop, beans, core, context, expression) and removed in: messaging Issues in messaging modules (jms, messaging) labels Jan 23, 2019
@jhoeller jhoeller self-assigned this Jan 23, 2019
@qsLI
Copy link

qsLI commented Jul 16, 2021

Same here! Our application loaded 20, 000 extra classes because of getDeclaredMethod of lazy beans. We saved about 40 seconds by skipping lazy bean here.

@qsLI
Copy link

qsLI commented Jul 16, 2021

@sbrannen @snicoll Please pay attention to this issue. Thanks in advance.

@jhoeller jhoeller changed the title EventListenerMethodProcessor defeats lazy factory beans EventListenerMethodProcessor causes expensive introspection of lazy beans Dec 29, 2023
@jhoeller jhoeller added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Dec 29, 2023
@jhoeller jhoeller added this to the 6.x Backlog milestone Dec 29, 2023
@jhoeller jhoeller modified the milestones: 6.x Backlog, 6.2.x Feb 28, 2024
@jhoeller
Copy link
Contributor

jhoeller commented Mar 7, 2024

Reviewing this from a 6.2 perspective and experimenting with the effects in a common scenarios, I am not convinced that a change in EventListenerMethodProcessor would make a real difference here. Every getBean(Class) performs the same level of eager initialization for type determination, and there is hardly any application without such a call involved these days.

To reliably avoid eager initialization of a FactoryBean for type determination purposes, I recommend setting FactoryBean.OBJECT_TYPE_ATTRIBUTE via BeanDefinition.setAttribute so that the container can infer the produced type upfront without having to call FactoryBean.getObjectType(). That attribute option is available since 5.2, shortly after the origin of this issue. Aside from that, type determination only creates the FactoryBean instance for a getObjectType() call, not calling getObject() yet; this can be leveraged for certain FactoryBean implementation styles that lazily create the exposed object from an eagerly created FactoryBean instance.

As for expensive reflection steps, we cache declared methods etc, so this should be reasonably efficient for beans that have been introspected before. However, for a lazy-init bean class not seen before at all, we would initiate reflection there indeed. How this could arrive at 20000 extra classes, I have no idea... If there are so many lazy beans with so many different classes involved, I would seriously slice the application's configuration to what is actually needed, instead of apparently defining lots of unnecessary beans which will never get used in the current run. Lazy beans are meant to be used selectively for certain use cases, mostly around rarely used application functionality that is only to be initialized when actually requested. Even there, warming up such lazy classes upfront can be seen as part of the container's startup measures; it's instantiation that is lazy, not introspection of the class.

@jhoeller jhoeller closed this as not planned Won't fix, can't repro, duplicate, stale Mar 7, 2024
@jhoeller jhoeller removed this from the 6.2.x milestone Mar 7, 2024
@jhoeller jhoeller added the status: invalid An issue that we don't feel is valid label Mar 7, 2024
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) status: invalid An issue that we don't feel is valid type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

6 participants