Skip to content

Avoid unnecessary reflection in TaskExecutorBuilder #23107

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

Closed
dsyer opened this issue Aug 27, 2020 · 3 comments
Closed

Avoid unnecessary reflection in TaskExecutorBuilder #23107

dsyer opened this issue Aug 27, 2020 · 3 comments
Labels
type: enhancement A general enhancement
Milestone

Comments

@dsyer
Copy link
Member

dsyer commented Aug 27, 2020

It could easily keep the existing methods but switch to using a Supplier and method references.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Aug 27, 2020
@wilkinsona
Copy link
Member

Given the existing configure(T) method, I can't see much benefit to a Supplier-based approach. To avoid using reflection by default, we could change build() to the following:

public ThreadPoolTaskExecutor build() {
	return configure(new ThreadPoolTaskExecutor());
}

This would remove the use of reflection in the code path driven by TaskExecutionAutoConfiguration. Anyone using TaskExecutorBuilder themselves can make a similar change and call configure(new T()) rather than build(Class<T>).

@dsyer
Copy link
Member Author

dsyer commented Aug 27, 2020

Makes sense I think. Just one possible snag: TheadPoolTaskExecutor is BeanNameAware and InitializingBean and DisposableBean. We might need to take care of those callbacks as well (although I can't see that they are being called right now)?

@wilkinsona
Copy link
Member

If you want those callbacks to be invoked, the expectation is that the instance that's built or configured by TaskExecutorBuilder is defined as a @Bean as we do in TaskExecutionAutoConfiguration.

@wilkinsona wilkinsona changed the title TaskExecutorBuilder uses reflection to create a TaskExecutor by default Avoid unnecessary reflection in TaskExecutorBuilder Sep 1, 2020
@wilkinsona wilkinsona added type: task A general task and removed status: waiting-for-triage An issue we've not yet triaged labels Sep 1, 2020
@wilkinsona wilkinsona added this to the 2.4.x milestone Sep 1, 2020
@wilkinsona wilkinsona added type: enhancement A general enhancement and removed type: task A general task labels Sep 1, 2020
@wilkinsona wilkinsona modified the milestones: 2.4.x, 2.4.0-M3 Sep 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

3 participants