Skip to content

Aligning internal caches with core framework patterns. #3073

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 4 commits into from
Closed

Aligning internal caches with core framework patterns. #3073

wants to merge 4 commits into from

Conversation

christophstrobl
Copy link
Member

Also initialize maps & sets with size where possible.

Closes: #3067

@mp911de mp911de added the type: task A general task label Apr 8, 2024
@mp911de mp911de added this to the 3.3 RC1 (2024.0.0) milestone Apr 8, 2024
mp911de
mp911de reviewed Apr 8, 2024
View reviewed changes
Copy link
Member

@mp911de mp911de left a comment

Choose a reason for hiding this comment

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

I left a few comments on places that would alter reloading behavior due to strong references that cannot be cleared when a GC would clear a ClassLoader.

src/main/java/org/springframework/data/mapping/model/KotlinCopyMethod.java Outdated
@@ -53,7 +53,7 @@
*/
class KotlinCopyMethod {

private static final Map<Class<?>, Optional<KotlinCopyMethod>> COPY_METHOD_CACHE = new ConcurrentReferenceHashMap<>();
private static final Map<Class<?>, Optional<KotlinCopyMethod>> COPY_METHOD_CACHE = new ConcurrentHashMap<>();
Copy link
Member

Choose a reason for hiding this comment

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

This one should remain ConcurrentReferenceHashMap with default soft references to not hinder GC runs.

src/main/java/org/springframework/data/mapping/PropertyPath.java Outdated
@@ -50,7 +50,7 @@ public class PropertyPath implements Streamable<PropertyPath> {
private static final Pattern SPLITTER = Pattern.compile("(?:[%s]?([%s]*?[^%s]+))".replaceAll("%s", DELIMITERS));
private static final Pattern SPLITTER_FOR_QUOTED = Pattern.compile("(?:[%s]?([%s]*?[^%s]+))".replaceAll("%s", "\\."));
private static final Pattern NESTED_PROPERTY_PATTERN = Pattern.compile("\\p{Lu}[\\p{Ll}\\p{Nd}]*$");
private static final Map<Property, PropertyPath> cache = new ConcurrentReferenceHashMap<>();
private static final Map<Property, PropertyPath> cache = new ConcurrentHashMap<>();
Copy link
Member

Choose a reason for hiding this comment

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

This should also stay ConcurrentReferenceHashMap as it is statically held.

...pringframework/data/repository/core/support/EventPublishingRepositoryProxyPostProcessor.java Outdated
@@ -141,7 +141,7 @@ private static boolean isDeleteMethod(String methodName) {
*/
static class EventPublishingMethod {

private static Map<Class<?>, EventPublishingMethod> cache = new ConcurrentReferenceHashMap<>();
Copy link
Member

Choose a reason for hiding this comment

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

static cache should remain ConcurrentReferenceHashMap

src/main/java/org/springframework/data/repository/query/ReturnedType.java Outdated
@@ -46,7 +47,7 @@
*/
public abstract class ReturnedType {

private static final Map<CacheKey, ReturnedType> cache = new ConcurrentReferenceHashMap<>(32);
private static final Map<CacheKey, ReturnedType> cache = new ConcurrentHashMap<>(32);
Copy link
Member

Choose a reason for hiding this comment

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

Should remain ConcurrentReferenceHashMap

src/main/java/org/springframework/data/repository/util/QueryExecutionConverters.java
private static final Set<Function<Object, Object>> UNWRAPPERS = new HashSet<>();
private static final Set<Class<?>> ALLOWED_PAGEABLE_TYPES = new HashSet<>();
private static final Map<Class<?>, ExecutionAdapter> EXECUTION_ADAPTER = new HashMap<>();
private static final Map<Class<?>, Boolean> supportsCache = new ConcurrentReferenceHashMap<>();
private static final Map<Class<?>, ExecutionAdapter> EXECUTION_ADAPTER = new HashMap<>(3, 1f);
Copy link
Member

Choose a reason for hiding this comment

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

Should remain ConcurrentReferenceHashMap

src/main/java/org/springframework/data/util/NullableWrapperConverters.java
@@ -67,7 +67,7 @@ public abstract class NullableWrapperConverters {
private static final Set<WrapperType> WRAPPER_TYPES = new HashSet<WrapperType>();
private static final Set<WrapperType> UNWRAPPER_TYPES = new HashSet<WrapperType>();
private static final Set<Converter<Object, Object>> UNWRAPPERS = new HashSet<Converter<Object, Object>>();
private static final Map<Class<?>, Boolean> supportsCache = new ConcurrentReferenceHashMap<>();
private static final Map<Class<?>, Boolean> supportsCache = new ConcurrentHashMap<>();
Copy link
Member

Choose a reason for hiding this comment

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

Should remain ConcurrentReferenceHashMap

src/main/java/org/springframework/data/util/ParameterTypes.java Outdated
@@ -45,7 +46,7 @@
public class ParameterTypes {

private static final TypeDescriptor OBJECT_DESCRIPTOR = TypeDescriptor.valueOf(Object.class);
private static final ConcurrentMap<List<TypeDescriptor>, ParameterTypes> cache = new ConcurrentReferenceHashMap<>();
private static final ConcurrentMap<List<TypeDescriptor>, ParameterTypes> cache = new ConcurrentHashMap<>();
Copy link
Member

Choose a reason for hiding this comment

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

Should remain ConcurrentReferenceHashMap

src/main/java/org/springframework/data/util/ProxyUtils.java Outdated
@@ -32,7 +32,7 @@
*/
public abstract class ProxyUtils {

private static Map<Class<?>, Class<?>> USER_TYPES = new ConcurrentReferenceHashMap<>();
private static Map<Class<?>, Class<?>> USER_TYPES = new ConcurrentHashMap<>();
Copy link
Member

Choose a reason for hiding this comment

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

Should remain ConcurrentReferenceHashMap

src/main/java/org/springframework/data/util/ReactiveWrappers.java Outdated
@@ -70,7 +70,7 @@ public abstract class ReactiveWrappers {

public static final boolean IS_REACTIVE_AVAILABLE = Arrays.stream(ReactiveLibrary.values())
.anyMatch(ReactiveWrappers::isAvailable);
private static final Map<Class<?>, Boolean> IS_REACTIVE_TYPE = new ConcurrentReferenceHashMap<>();
private static final Map<Class<?>, Boolean> IS_REACTIVE_TYPE = new ConcurrentHashMap<>();
Copy link
Member

Choose a reason for hiding this comment

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

Should remain ConcurrentReferenceHashMap

mp911de pushed a commit that referenced this pull request Jun 13, 2024
@christophstrobl @mp911de
Align internal caches with core framework patterns.
b21b2e8 
Initialize maps and sets with size where possible.

Closes #3067
Original pull request: #3073
mp911de added a commit that referenced this pull request Jun 13, 2024
@mp911de
Polishing.
0e9dc7a 
Provide load-factor to sets with well-known sizing, move annotation auditing metadata cache to DefaultAuditableBeanWrapperFactory to avoid strong static references.

See #3067
Original pull request: #3073
@mp911de
Copy link
Member

mp911de commented Jun 13, 2024

That's merged and polished now.

@mp911de mp911de closed this Jun 13, 2024
@mp911de mp911de deleted the issue/3067 branch June 13, 2024 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mp911de mp911de mp911de left review comments

Assignees

@mp911de mp911de

Labels
type: task A general task
Projects
None yet
Milestone
3.4 M1 (2024.1.0)
Development

Successfully merging this pull request may close these issues.

Aligning internal caches with core framework patterns.
2 participants
@christophstrobl @mp911de