Skip to content

Caching path resolution - #2336

Open
schauder wants to merge 2 commits into
mainfrom
issue/2335
Open

Caching path resolution#2336
schauder wants to merge 2 commits into
mainfrom
issue/2335

Conversation

@schauder

@schauder schauder commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Caches both positive and negative resolution of paths, in order to avoid repeated use of exceptions for program control.
Negative resolution, i.e. no matching path found is cached separately in an LRU cache to avoid memory exhaustion.

Closes #2335

Caches both positive and negative resolution of paths, in order to avoid repeated use of exceptions for program control.
Negative resolution, i.e. no matching path found is cached separately in an LRU cache to avoid memory exhaustion.

Closes #2335

@mp911de mp911de left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The caching isn't effective caused by short-lived QueryMapper objects in JDBC.

R2DBC's binding to DefaultReactiveDataAccessStrategy reduces the query mapper instances to a single instance and for R2DBC, this approach could work.

* Extension of {@link Field} to be backed with mapping metadata.
*/
protected static class MetadataBackedField extends Field {
protected class MetadataBackedField extends Field {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: removing static makes the MappingContext<? extends RelationalPersistentEntity<?>, RelationalPersistentProperty> mappingContext field superfluous


private final JdbcConverter converter;
private final MappingContext<? extends RelationalPersistentEntity<?>, RelationalPersistentProperty> mappingContext;
private final PropertyPathResolver propertyPathResolver;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QueryMapper is created quite frequently (e.g. AggregateReader.createCondition, StatementFactory via PartTreeJdbcQuery.execute or createDeleteQueries), therefore we create many instances of a cache that do not survive long enough to establish meaningful caching.

private static final int UNRESOLVABLE_CACHE_SIZE = 4096;

private final MappingContext<? extends RelationalPersistentEntity<?>, RelationalPersistentProperty> mappingContext;
private final Map<PathResolutionKey, PersistentPropertyPath<RelationalPersistentProperty>> resolved = new ConcurrentHashMap<>();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Map has an unbounded growth imposing out of memory risk.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's why it is used only for the successfully resolved paths, which are limited in number.

The unlimited unresolved paths are only cached in an LRUCache.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PathResolutionKey input is a String. Given the supported variance in expressions, PersonAddressStreetName can be person.address.streetName, person_address_streetName, personAddress_streetName, etc. While the space is finite, we still can have an exponentially growing number of keys.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Derived queries on snake_case mapped columns trigger repeated PropertyReferenceException lookups and create avoidable overhead

3 participants