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

Slow SpEL performance due to method sorting in ReflectiveMethodResolver #28377

Closed
aashishin opened this issue Apr 25, 2022 · 2 comments
Closed
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: backported An issue that has been backported to maintenance branches type: bug A general bug
Milestone

Comments

@aashishin
Copy link

aashishin commented Apr 25, 2022

Hi,

I am using Spring expression language (5.3.15).

I found the following method does sorting on a list of methods (ReflectiveMethodResolver).

public MethodExecutor resolve(EvaluationContext context, Object targetObject, String name,
			List<TypeDescriptor> argumentTypes) throws AccessException 

I did some benchmarking and found it cost me at least 100 milliseconds more.

Is it possible to make this sorting optional ?

I am attaching the benchmark logs with and without sorting .

Do we really need the following code ?

			// Sort methods into a sensible order
	/*		if (methods.size() > 1) {
				methods.sort((m1, m2) -> {
					 if(m1==null ||m2==null)
					 {
						 
						 System.out.println("==================Some method came null  ");
					 }
					int m1pl = m1.getParameterCount();
					int m2pl = m2.getParameterCount();
					// vararg methods go last
					if (m1pl == m2pl) {
						if (!m1.isVarArgs() && m2.isVarArgs()) {
							return -1;
						}
						else if (m1.isVarArgs() && !m2.isVarArgs()) {
							return 1;
						}
						else {
							return 0;
						}
					}
					return Integer.compare(m1pl, m2pl);
				});
			}*/

Thanks,

Aashish

translator-without-sorting.log
translatorwith-sorting-.log

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Apr 25, 2022
@jhoeller jhoeller self-assigned this Apr 26, 2022
@jhoeller jhoeller added the in: core Issues in core modules (aop, beans, core, context, expression) label Apr 26, 2022
@jhoeller jhoeller changed the title Slow performance in spring expression language (5.3.15) due to method soring in ReflectiveMethodResolver Slow SpEL performance due to method sorting in ReflectiveMethodResolver Apr 26, 2022
@aashishtyagi
Copy link

Hi,

Are we planning to remove this sorting code in some future release?

Thanks,
Aashish

@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 Nov 24, 2023
@jhoeller
Copy link
Contributor

While the sorting is necessary for properly differentiating between overloaded methods, it turns out that we match the methods by name too late there: We can easily filter for a name match first and only then sort and traverse.

@jhoeller jhoeller added this to the 6.1.2 milestone Nov 24, 2023
@github-actions github-actions bot added status: backported An issue that has been backported to maintenance branches and removed for: backport-to-6.0.x labels Nov 24, 2023
@sbrannen sbrannen changed the title Slow SpEL performance due to method sorting in ReflectiveMethodResolver Slow SpEL performance due to method sorting in ReflectiveMethodResolver Nov 24, 2023
jhoeller added a commit that referenced this issue Nov 24, 2023
jhoeller added a commit that referenced this issue Nov 24, 2023
@jhoeller jhoeller added type: bug A general bug and removed type: enhancement A general enhancement labels Nov 30, 2023
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: backported An issue that has been backported to maintenance branches type: bug A general bug
Projects
None yet
Development

No branches or pull requests

4 participants