Skip to content

Commit 5a37097

Browse files
tandraschkotandraschko
committed
[perf] some feedback from claude
1 parent 1dd0a4a commit 5a37097

9 files changed

Lines changed: 86 additions & 102 deletions

File tree

‎api/src/main/java/org/apache/myfaces/core/api/shared/lang/PropertyDescriptorUtils.java‎

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -61,29 +61,28 @@ public class PropertyDescriptorUtils
6161

6262
private static Map<String, Map<String, ? extends PropertyDescriptorWrapper>> getCache(ExternalContext ec)
6363
{
64+
Map<String, Object> appMap = ec.getApplicationMap();
6465
Map<String, Map<String, ? extends PropertyDescriptorWrapper>> cache =
65-
(Map<String, Map<String, ? extends PropertyDescriptorWrapper>>) ec.getApplicationMap().get(CACHE_KEY);
66-
if (cache == null)
66+
(Map<String, Map<String, ? extends PropertyDescriptorWrapper>>) appMap.get(CACHE_KEY);
67+
if (cache != null)
6768
{
68-
cache = new ConcurrentHashMap<>(1000);
69-
ec.getApplicationMap().put(CACHE_KEY, cache);
69+
return cache;
7070
}
71-
72-
return cache;
71+
return (Map<String, Map<String, ? extends PropertyDescriptorWrapper>>) appMap.computeIfAbsent(CACHE_KEY,
72+
k -> new ConcurrentHashMap<>(1000));
7373
}
7474

7575
public static Map<String, ? extends PropertyDescriptorWrapper> getCachedPropertyDescriptors(ExternalContext ec,
7676
Class<?> caller,
7777
Class<?> target)
7878
{
79-
Map<String, ? extends PropertyDescriptorWrapper> cache = getCache(ec).get(target.getName());
80-
if (cache == null)
79+
Map<String, Map<String, ? extends PropertyDescriptorWrapper>> outer = getCache(ec);
80+
Map<String, ? extends PropertyDescriptorWrapper> cache = outer.get(target.getName());
81+
if (cache != null)
8182
{
82-
final Class<?> realTarget = target;
83-
cache = getCache(ec).computeIfAbsent(target.getName(), k -> getPropertyDescriptors(ec, caller, target));
83+
return cache;
8484
}
85-
86-
return cache;
85+
return outer.computeIfAbsent(target.getName(), k -> getPropertyDescriptors(ec, caller, target));
8786
}
8887

8988
public static boolean isUseLambdas(ExternalContext ec)
@@ -112,8 +111,13 @@ public static boolean isUseLambdas(ExternalContext ec)
112111

113112
if (isUseLambdas(ec))
114113
{
115-
List<Module> cache = moduleAccessCache.computeIfAbsent(caller.getModule(),
116-
(k) -> new CopyOnWriteArrayList<>());
114+
Module callerModule = caller.getModule();
115+
List<Module> cache = moduleAccessCache.get(callerModule);
116+
if (cache == null)
117+
{
118+
cache = moduleAccessCache.computeIfAbsent(callerModule,
119+
k -> new CopyOnWriteArrayList<>());
120+
}
117121
if (!cache.contains(target.getModule()))
118122
{
119123
if (!caller.getModule().canRead(target.getModule()))

‎impl/src/main/java/org/apache/myfaces/config/RuntimeConfig.java‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,8 +124,13 @@ public static RuntimeConfig getCurrentInstance(FacesContext facesContext)
124124

125125
public static RuntimeConfig getCurrentInstance(ExternalContext externalContext)
126126
{
127-
return (RuntimeConfig) externalContext.getApplicationMap().computeIfAbsent(
128-
APPLICATION_MAP_PARAM_NAME, k -> new RuntimeConfig());
127+
Map<String, Object> appMap = externalContext.getApplicationMap();
128+
RuntimeConfig config = (RuntimeConfig) appMap.get(APPLICATION_MAP_PARAM_NAME);
129+
if (config != null)
130+
{
131+
return config;
132+
}
133+
return (RuntimeConfig) appMap.computeIfAbsent(APPLICATION_MAP_PARAM_NAME, k -> new RuntimeConfig());
129134
}
130135

131136
@Override

‎impl/src/main/java/org/apache/myfaces/context/RequestViewContext.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ public static RequestViewContext newInstance(RequestViewMetadata rvm)
111111
public static void setCurrentInstance(FacesContext ctx, UIViewRoot root, RequestViewContext rvc)
112112
{
113113
Map<UIViewRoot, RequestViewContext> map = (Map<UIViewRoot, RequestViewContext>) ctx.getAttributes()
114-
.computeIfAbsent(VIEW_CONTEXT_KEY, k -> new HashMap<>());
114+
.computeIfAbsent(VIEW_CONTEXT_KEY, k -> new HashMap<>(5));
115115
map.put(root, rvc);
116116
}
117117

‎impl/src/main/java/org/apache/myfaces/context/servlet/ServletExternalContextImpl.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -407,7 +407,7 @@ protected void pushResource(String resourceUrl)
407407
}
408408

409409
Set<String> pushedResourceUrls = (Set<String>) facesContext.getAttributes()
410-
.computeIfAbsent(PUSHED_RESOURCE_URLS, a -> new HashSet<>());
410+
.computeIfAbsent(PUSHED_RESOURCE_URLS, a -> new HashSet<>());
411411
if (pushedResourceUrls.contains(resourceUrl))
412412
{
413413
return;

‎impl/src/main/java/org/apache/myfaces/el/TracingELResolverPhaseListener.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,13 +61,13 @@ public static void getTypeHit(Class<? extends ELResolver> resolver)
6161

6262
public static TracingInfo getTracingInfo(Class<? extends ELResolver> resolver)
6363
{
64-
return getTracingInfos().computeIfAbsent(resolver, (k) -> new TracingInfo());
64+
return getTracingInfos().computeIfAbsent(resolver, k -> new TracingInfo());
6565
}
6666

6767
public static Map<Class<? extends ELResolver>, TracingInfo> getTracingInfos()
6868
{
6969
return (Map<Class<? extends ELResolver>, TracingInfo>) FacesContext.getCurrentInstance().getAttributes()
70-
.computeIfAbsent(TRACING_INFOS, (k) -> new HashMap<>());
70+
.computeIfAbsent(TRACING_INFOS, k -> new HashMap<>());
7171
}
7272

7373
@Override

‎impl/src/main/java/org/apache/myfaces/spi/impl/DefaultStateCacheProviderFactory.java‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020

2121
import java.lang.reflect.InvocationTargetException;
2222
import java.util.List;
23+
import java.util.Map;
2324
import java.util.logging.Level;
2425
import java.util.logging.Logger;
2526
import jakarta.faces.context.ExternalContext;
@@ -49,7 +50,13 @@ private Logger getLogger()
4950
@Override
5051
public StateCacheProvider getStateCacheProvider(ExternalContext externalContext)
5152
{
52-
return (StateCacheProvider) externalContext.getApplicationMap().computeIfAbsent(STATE_CACHE_PROVIDER_INSTANCE,
53+
Map<String, Object> appMap = externalContext.getApplicationMap();
54+
StateCacheProvider provider = (StateCacheProvider) appMap.get(STATE_CACHE_PROVIDER_INSTANCE);
55+
if (provider != null)
56+
{
57+
return provider;
58+
}
59+
return (StateCacheProvider) appMap.computeIfAbsent(STATE_CACHE_PROVIDER_INSTANCE,
5360
k -> createStateCacheProvider(externalContext));
5461
}
5562

@@ -85,9 +92,9 @@ private StateCacheProvider resolveStateCacheProviderFromService(
8592
List<String> classList = (List<String>) externalContext.getApplicationMap().get(STATE_CACHE_PROVIDER_LIST);
8693
if (classList == null)
8794
{
88-
classList = ServiceProviderFinderFactory.getServiceProviderFinder(externalContext).
89-
getServiceProviderList(STATE_CACHE_PROVIDER);
90-
externalContext.getApplicationMap().put(STATE_CACHE_PROVIDER_LIST, classList);
95+
classList = (List<String>) externalContext.getApplicationMap().computeIfAbsent(STATE_CACHE_PROVIDER_LIST,
96+
k -> ServiceProviderFinderFactory.getServiceProviderFinder(externalContext).
97+
getServiceProviderList(STATE_CACHE_PROVIDER));
9198
}
9299
return ClassUtils.buildApplicationObject(StateCacheProvider.class, classList, new StateCacheProviderImpl());
93100
}

‎impl/src/main/java/org/apache/myfaces/view/facelets/tag/MetaRulesetImpl.java‎

Lines changed: 27 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
*/
1919
package org.apache.myfaces.view.facelets.tag;
2020

21-
import org.apache.myfaces.util.lang.ClassUtils;
2221
import jakarta.faces.view.facelets.MetaRule;
2322
import jakarta.faces.view.facelets.MetaRuleset;
2423
import jakarta.faces.view.facelets.Metadata;
@@ -31,7 +30,7 @@
3130
import java.util.HashMap;
3231
import java.util.List;
3332
import java.util.Map;
34-
import java.util.WeakHashMap;
33+
import java.util.concurrent.ConcurrentHashMap;
3534
import java.util.logging.Level;
3635
import java.util.logging.Logger;
3736
import jakarta.faces.context.FacesContext;
@@ -49,44 +48,29 @@ public final class MetaRulesetImpl extends MetaRuleset
4948
{
5049
private final static Logger log = Logger.getLogger(MetaRulesetImpl.class.getName());
5150

52-
/**
53-
* Cache the MetadataTarget instances per ClassLoader using the Class-Name of the type variable
54-
* of MetadataTarget.
55-
* NOTE that we do it this way, because the only other valid way in order to support a shared
56-
* classloader scenario would be to use a WeakHashMap<Class<?>, MetadataTarget>, but this
57-
* creates a cyclic reference between the key and the value of the WeakHashMap which will
58-
* most certainly cause a memory leak! Furthermore we can manually cleanup the Map when
59-
* the webapp is undeployed just by removing the Map for the current ClassLoader.
60-
*/
61-
private volatile static WeakHashMap<ClassLoader, Map<String, MetadataTarget>> metadata
62-
= new WeakHashMap<ClassLoader, Map<String, MetadataTarget>>();
51+
private static final String METADATA_KEY = MetaRulesetImpl.class.getName() + ".METADATA";
6352

6453
/**
65-
* Removes the cached MetadataTarget instances in order to prevent a memory leak.
54+
* MetadataTarget cache keyed by component class name, stored in the JSF application map so it
55+
* is released with the web application (no separate undeploy hook).
6656
*/
67-
public static void clearMetadataTargetCache()
68-
{
69-
metadata.remove(ClassUtils.getContextClassLoader());
70-
}
71-
72-
private static Map<String, MetadataTarget> getMetaData()
57+
@SuppressWarnings("unchecked")
58+
private static ConcurrentHashMap<String, MetadataTarget> getMetaData()
7359
{
74-
ClassLoader cl = ClassUtils.getContextClassLoader();
75-
76-
Map<String, MetadataTarget> metadata = MetaRulesetImpl.metadata.get(cl);
77-
78-
if (metadata == null)
60+
FacesContext facesContext = FacesContext.getCurrentInstance();
61+
if (facesContext == null)
7962
{
80-
// Ensure thread-safe put over _metadata, and only create one map
81-
// per classloader to hold metadata.
82-
synchronized (MetaRulesetImpl.metadata)
83-
{
84-
metadata = MetaRulesetImpl.metadata.computeIfAbsent(cl,
85-
k -> new HashMap<>());
86-
}
63+
throw new IllegalStateException("No FacesContext is available");
8764
}
88-
89-
return metadata;
65+
Map<String, Object> applicationMap = facesContext.getExternalContext().getApplicationMap();
66+
ConcurrentHashMap<String, MetadataTarget> map =
67+
(ConcurrentHashMap<String, MetadataTarget>) applicationMap.get(METADATA_KEY);
68+
if (map != null)
69+
{
70+
return map;
71+
}
72+
return (ConcurrentHashMap<String, MetadataTarget>) applicationMap.computeIfAbsent(METADATA_KEY,
73+
k -> new ConcurrentHashMap<>(64));
9074
}
9175

9276
private final static TagAttribute[] EMPTY = new TagAttribute[0];
@@ -320,40 +304,28 @@ public MetaRuleset ignoreAll()
320304

321305
private MetadataTarget _getMetadataTarget()
322306
{
323-
Map<String, MetadataTarget> metadata = getMetaData();
307+
ConcurrentHashMap<String, MetadataTarget> metadata = getMetaData();
324308
String metaKey = _type.getName();
325-
326309
MetadataTarget meta = metadata.get(metaKey);
327-
if (meta == null)
310+
if (meta != null)
311+
{
312+
return meta;
313+
}
314+
return metadata.computeIfAbsent(metaKey, key ->
328315
{
329316
try
330317
{
331318
if (PropertyDescriptorUtils.isUseLambdas(
332319
FacesContext.getCurrentInstance().getExternalContext()))
333320
{
334-
meta = new LambdaMetadataTargetImpl(_type);
335-
}
336-
else
337-
{
338-
meta = new MetadataTargetImpl(_type);
321+
return new LambdaMetadataTargetImpl(_type);
339322
}
323+
return new MetadataTargetImpl(_type);
340324
}
341325
catch (IntrospectionException e)
342326
{
343327
throw new TagException(_tag, "Error Creating TargetMetadata", e);
344328
}
345-
346-
synchronized(metadata)
347-
{
348-
// Use a synchronized block to ensure proper operation on concurrent use cases.
349-
// This is a racy single check, because initialization over the same class could happen
350-
// multiple times, but the same result is always calculated. The synchronized block
351-
// just ensure thread-safety, because only one thread will modify the cache map
352-
// at the same time.
353-
metadata.put(metaKey, meta);
354-
}
355-
}
356-
357-
return meta;
329+
});
358330
}
359331
}

‎impl/src/main/java/org/apache/myfaces/view/facelets/tag/composite/CompositeMetaRulesetImpl.java‎

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import java.util.HashMap;
3737
import java.util.List;
3838
import java.util.Map;
39+
import java.util.concurrent.ConcurrentHashMap;
3940
import java.util.logging.Level;
4041
import java.util.logging.Logger;
4142
import org.apache.myfaces.core.api.shared.lang.PropertyDescriptorUtils;
@@ -49,15 +50,18 @@ public class CompositeMetaRulesetImpl extends MetaRuleset
4950

5051
private static final String METADATA_KEY = CompositeMetaRulesetImpl.class.getName() + ".METADATA";
5152

52-
private static Map<String, MetadataTarget> getMetaData()
53+
private static ConcurrentHashMap<String, MetadataTarget> getMetaData()
5354
{
5455
FacesContext facesContext = FacesContext.getCurrentInstance();
5556
Map<String, Object> applicationMap = facesContext.getExternalContext().getApplicationMap();
56-
57-
Map<String, MetadataTarget> metadata = (Map<String, MetadataTarget>) applicationMap.computeIfAbsent(
58-
METADATA_KEY, k -> new HashMap<>());
59-
60-
return metadata;
57+
ConcurrentHashMap<String, MetadataTarget> map =
58+
(ConcurrentHashMap<String, MetadataTarget>) applicationMap.get(METADATA_KEY);
59+
if (map != null)
60+
{
61+
return map;
62+
}
63+
return (ConcurrentHashMap<String, MetadataTarget>) applicationMap.computeIfAbsent(METADATA_KEY,
64+
k -> new ConcurrentHashMap<>(64));
6165
}
6266

6367
private final Map<String, TagAttribute> _attributes;
@@ -206,32 +210,28 @@ private MetadataTarget _getMetadataTarget()
206210

207211
private MetadataTarget _getBaseMetadataTarget()
208212
{
209-
Map<String, MetadataTarget> metadata = getMetaData();
213+
ConcurrentHashMap<String, MetadataTarget> metadata = getMetaData();
210214
String key = _type.getName();
211-
212215
MetadataTarget meta = metadata.get(key);
213-
if (meta == null)
216+
if (meta != null)
217+
{
218+
return meta;
219+
}
220+
return metadata.computeIfAbsent(key, k ->
214221
{
215222
try
216223
{
217224
if (PropertyDescriptorUtils.isUseLambdas(
218225
FacesContext.getCurrentInstance().getExternalContext()))
219226
{
220-
meta = new LambdaMetadataTargetImpl(_type);
221-
}
222-
else
223-
{
224-
meta = new MetadataTargetImpl(_type);
227+
return new LambdaMetadataTargetImpl(_type);
225228
}
229+
return new MetadataTargetImpl(_type);
226230
}
227231
catch (IntrospectionException e)
228232
{
229233
throw new TagException(_tag, "Error Creating TargetMetadata", e);
230234
}
231-
232-
metadata.put(key, meta);
233-
}
234-
235-
return meta;
235+
});
236236
}
237237
}

‎impl/src/main/java/org/apache/myfaces/webapp/FacesInitializerImpl.java‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,6 @@
6464
import org.apache.myfaces.util.lang.ClassUtils;
6565
import org.apache.myfaces.util.lang.StringUtils;
6666
import org.apache.myfaces.view.facelets.ViewPoolProcessor;
67-
import org.apache.myfaces.view.facelets.tag.MetaRulesetImpl;
6867

6968
import javax.naming.InitialContext;
7069
import javax.naming.NamingException;
@@ -330,9 +329,6 @@ public void destroyFaces(ServletContext servletContext)
330329
_dispatchApplicationEvent(servletContext, PreDestroyApplicationEvent.class);
331330

332331
_callPreDestroyOnInjectedJSFArtifacts(facesContext);
333-
334-
// clear the cache of MetaRulesetImpl in order to prevent a memory leak
335-
MetaRulesetImpl.clearMetadataTargetCache();
336332

337333
if (facesContext.getExternalContext().getApplicationMap().containsKey(PUSH_INITIALIZED))
338334
{

0 commit comments

Comments
 (0)