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

Cache result of RepositoryInformationSupport.getQueryMethods() #3066

Closed
dodgex opened this issue Mar 20, 2024 · 5 comments
Closed

Cache result of RepositoryInformationSupport.getQueryMethods() #3066

dodgex opened this issue Mar 20, 2024 · 5 comments
Assignees
Labels
type: enhancement A general enhancement

Comments

@dodgex
Copy link

dodgex commented Mar 20, 2024

Hey there,

I work on a project that has over 250 JPA Repositories (distributed over 3 databases / persistence unites). The startup time in development and also production takes a big hit due to this fact.

While searching for the reason for the long startup times, i came across the getQueryMethods() in RepositoryInformationSupport (in 2.7.x it was in DefaultRepositoryInformation) and that it scans the repository interface for all methods and builds a set of qualified query methods.

This method is called in a lot of places, and multiple times during startup. and some places get all methods, only to search for one single after this...

e.g
RepositoryInformationSupport.java#L126

@Override
public boolean isQueryMethod(Method method) {
	return getQueryMethods().stream().anyMatch(it -> it.equals(method));
}

or RepositoryInformation.java#L85-L91

default boolean hasCustomMethod() {
	return getQueryMethods().stream().anyMatch(this::isCustomMethod);
}

default boolean hasQueryMethods() {
	return getQueryMethods().iterator().hasNext();
}

While in most cases, this should not be a to big problem, for bigger projects this could have huge impact. and even smaller projects could start even faster.

For testing purposes, I created a copy of the org.springframework.data.repository.core.DefaultRepositoryInformation in my development environment and changed the getQueryMethods to keep the result of the first invocation and do not scan the class again.

For 3.2.3 my implementation looks like this:

private Set<Method> resultCache;

@Override
public Streamable<Method> getQueryMethods() {
	if (resultCache == null) {
		Streamable<Method> queryMethods = super.getQueryMethods();
		resultCache = Collections.unmodifiableSet(queryMethods.toSet());
	}
	return Streamable.of(resultCache);
}

With this small change, my project startup time shrinks from ~120 seconds to ~40-45 seconds using Spring Boot 3.2.3. That is an improvement of about 60%.

While I do not have a deeper knowledge of what impact/sideeffect it could have to only scan the repository interface once, I can say that i already had this change for Spring Boot 2 in place for now over a year (actually i stumbled over my custom DefaultRepositoryInformation while upgrading to SB3). For SB3 I havent done too much testing as of now, but at least there where no immediate issues.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Mar 20, 2024
@christophstrobl
Copy link
Member

Thank you @dodgex for raising the issue. Let me take this to the team.

@christophstrobl christophstrobl added the for: team-attention An issue we need to discuss as a team to make progress label Mar 21, 2024
@odrotbohm odrotbohm added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged for: team-attention An issue we need to discuss as a team to make progress labels Mar 25, 2024
odrotbohm added a commit that referenced this issue Mar 25, 2024
Related ticket GH-3066.
@odrotbohm
Copy link
Member

Thanks for bringing this up. I've introduced a cache for the query methods and the information calculated in RepositoryInformation. Would you mind giving the latest snapshots a spin? If those look good, I'd back-port the fix back into the maintenance branches.

@odrotbohm odrotbohm added the status: waiting-for-feedback We need additional information before we can continue label Mar 25, 2024
@dodgex
Copy link
Author

dodgex commented Mar 25, 2024

@odrotbohm I gave your changes a quick try. The application started as "quickly" as with my hacky solution, and as far as I tested everything worked!

And as expected your implementation is a bit cleaner than mine. ;)

I found that your polishing commit got a typo in interfacse.
Also I'm curious if it is intended to have the calculateQueryMethods and calculateHasCustomMethod methods private final instead of just private. And the queryMethods field is Lazy<> while the other two are Supplier<> although all three of them come from Lazy.of().

Thanks for the quick fix.

@odrotbohm
Copy link
Member

Great feedback, thanks! We tend to use private final for methods being invoked from constructors, but it's a cosmetical change. You're right that we can switch the field declaration to a Supplier as we don't need any Lazy-specific methods here.

I'll apply those fixes and backport the changes.

odrotbohm added a commit that referenced this issue Mar 25, 2024
We now calculate information about query methods in RepositoryInformationSupport lazily and keep it around to avoid repeated calculations that involve traversals over declared method and Stream allocations.

Fixes GH-3066.
odrotbohm added a commit that referenced this issue Mar 25, 2024
Related ticket GH-3066.
odrotbohm added a commit that referenced this issue Mar 25, 2024
We now calculate information about query methods in RepositoryInformationSupport lazily and keep it around to avoid repeated calculations that involve traversals over declared method and Stream allocations.

Fixes GH-3066.
odrotbohm added a commit that referenced this issue Mar 25, 2024
Related ticket GH-3066.
odrotbohm added a commit that referenced this issue Mar 25, 2024
We now calculate information about query methods in RepositoryInformationSupport lazily and keep it around to avoid repeated calculations that involve traversals over declared method and Stream allocations.

Fixes GH-3066.
odrotbohm added a commit that referenced this issue Mar 25, 2024
Related ticket GH-3066.
@odrotbohm
Copy link
Member

This should be all in place now.

@odrotbohm odrotbohm added this to the 3.1.11 (2023.0.11) milestone Mar 25, 2024
@odrotbohm odrotbohm removed the status: waiting-for-feedback We need additional information before we can continue label Mar 25, 2024
@odrotbohm odrotbohm changed the title Cache result of RepositoryInformationSupport#getQueryMethods? Cache result of RepositoryInformationSupport.getQueryMethods() Mar 25, 2024
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

4 participants