You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@myfaces.apache.org by ta...@apache.org on 2026/06/04 22:33:48 UTC
(myfaces) branch 4.1.x updated: [perf] backport improvements
This is an automated email from the ASF dual-hosted git repository.
tandraschko pushed a commit to branch 4.1.x
in repository https://gitbox.apache.org/repos/asf/myfaces.git
The following commit(s) were added to refs/heads/4.1.x by this push:
new 7f601c426 [perf] backport improvements
7f601c426 is described below
commit 7f601c426068f4dcc99329708c6151e0a2cf72fa
Author: tandraschko <ta...@apache.org>
AuthorDate: Fri Jun 5 00:33:37 2026 +0200
[perf] backport improvements
---
.../myfaces/application/ResourceHandlerImpl.java | 4 +-
.../el/resolver/CompositeComponentELResolver.java | 41 ++++-------
.../apache/myfaces/lifecycle/LifecycleImpl.java | 4 +-
.../myfaces/lifecycle/PhaseListenerManager.java | 69 +++++++++++-------
.../facelets/PartialStateManagementStrategy.java | 52 +++++++-------
.../myfaces/view/facelets/impl/DefaultFacelet.java | 6 +-
.../view/facelets/impl/DefaultFaceletFactory.java | 19 +++--
.../myfaces/view/facelets/tag/MetaRulesetImpl.java | 82 +++++++---------------
.../tag/composite/CompositeMetaRulesetImpl.java | 38 +++++-----
.../myfaces/webapp/FacesInitializerImpl.java | 5 --
10 files changed, 151 insertions(+), 169 deletions(-)
diff --git a/impl/src/main/java/org/apache/myfaces/application/ResourceHandlerImpl.java b/impl/src/main/java/org/apache/myfaces/application/ResourceHandlerImpl.java
index 35f142df7..cde2ee918 100644
--- a/impl/src/main/java/org/apache/myfaces/application/ResourceHandlerImpl.java
+++ b/impl/src/main/java/org/apache/myfaces/application/ResourceHandlerImpl.java
@@ -1766,9 +1766,9 @@ private List<String> resolveProgrammaticViewIds(FacesContext facesContext)
{
for (Annotation anno : bean.getBeanClass().getAnnotations())
{
- if (anno instanceof View view)
+ if (anno instanceof View)
{
- views.add(view.value());
+ views.add(((View) anno).value());
}
}
}
diff --git a/impl/src/main/java/org/apache/myfaces/el/resolver/CompositeComponentELResolver.java b/impl/src/main/java/org/apache/myfaces/el/resolver/CompositeComponentELResolver.java
index 0277c7b69..349489249 100644
--- a/impl/src/main/java/org/apache/myfaces/el/resolver/CompositeComponentELResolver.java
+++ b/impl/src/main/java/org/apache/myfaces/el/resolver/CompositeComponentELResolver.java
@@ -21,12 +21,11 @@
import java.beans.BeanInfo;
import java.beans.FeatureDescriptor;
import java.beans.PropertyDescriptor;
-import java.lang.ref.WeakReference;
import java.util.Collection;
+import java.util.IdentityHashMap;
import java.util.Iterator;
import java.util.Map;
import java.util.Set;
-import java.util.WeakHashMap;
import jakarta.el.ELContext;
import jakarta.el.ELResolver;
@@ -210,39 +209,25 @@ private Map<String, Object> _getCompositeComponentAttributesMapWrapper(
{
Map<Object, Object> contextMap = facesContext(elContext).getAttributes();
- // We use a WeakHashMap<UIComponent, WeakReference<Map<String, Object>>> to
- // hold attribute map wrappers by two reasons:
- //
- // 1. The wrapper is used multiple times for a very short amount of time (in fact on current request).
- // 2. The original attribute map has an inner reference to UIComponent, so we need to wrap it
- // with WeakReference.
- //
- Map<UIComponent, WeakReference<Map<String, Object>>> compositeComponentAttributesMaps =
- (Map<UIComponent, WeakReference<Map<String, Object>>>) contextMap.get(COMPOSITE_COMPONENT_ATTRIBUTES_MAPS);
+ // IdentityHashMap: FacesContext is not thread-safe; keys are UIComponent instances by identity.
+ // The map lives only for the request (FacesContext attributes), avoiding WeakHashMap sync cost.
+ IdentityHashMap<UIComponent, Map<String, Object>> compositeComponentAttributesMaps =
+ (IdentityHashMap<UIComponent, Map<String, Object>>) contextMap.get(COMPOSITE_COMPONENT_ATTRIBUTES_MAPS);
Map<String, Object> attributesMap = null;
- WeakReference<Map<String, Object>> weakReference;
if (compositeComponentAttributesMaps != null)
{
- weakReference = compositeComponentAttributesMaps.get(baseComponent);
- if (weakReference != null)
- {
- attributesMap = weakReference.get();
- }
- if (attributesMap == null)
- {
- //create a wrapper map
- attributesMap = new CompositeComponentAttributesMapWrapper(baseComponent);
- compositeComponentAttributesMaps.put(baseComponent, new WeakReference<>(attributesMap));
- }
+ attributesMap = compositeComponentAttributesMaps.get(baseComponent);
}
- else
+ if (attributesMap == null)
{
- //Create both required maps
attributesMap = new CompositeComponentAttributesMapWrapper(baseComponent);
- compositeComponentAttributesMaps = new WeakHashMap<>();
- compositeComponentAttributesMaps.put(baseComponent, new WeakReference<>(attributesMap));
- contextMap.put(COMPOSITE_COMPONENT_ATTRIBUTES_MAPS, compositeComponentAttributesMaps);
+ if (compositeComponentAttributesMaps == null)
+ {
+ compositeComponentAttributesMaps = new IdentityHashMap<>(8);
+ contextMap.put(COMPOSITE_COMPONENT_ATTRIBUTES_MAPS, compositeComponentAttributesMaps);
+ }
+ compositeComponentAttributesMaps.put(baseComponent, attributesMap);
}
return attributesMap;
}
diff --git a/impl/src/main/java/org/apache/myfaces/lifecycle/LifecycleImpl.java b/impl/src/main/java/org/apache/myfaces/lifecycle/LifecycleImpl.java
index 7a2562919..a6b37835a 100755
--- a/impl/src/main/java/org/apache/myfaces/lifecycle/LifecycleImpl.java
+++ b/impl/src/main/java/org/apache/myfaces/lifecycle/LifecycleImpl.java
@@ -119,7 +119,7 @@ public void execute(FacesContext facesContext) throws FacesException
new FacesConfigurator(facesContext.getExternalContext()).update();
}
- PhaseListenerManager phaseListenerMgr = new PhaseListenerManager(this, facesContext, getPhaseListeners());
+ PhaseListenerManager phaseListenerMgr = new PhaseListenerManager(this, facesContext, phaseListeners);
for (PhaseExecutor executor : lifecycleExecutors)
{
if (executePhase(facesContext, executor, phaseListenerMgr))
@@ -219,7 +219,7 @@ public void render(FacesContext facesContext) throws FacesException
log.finest("Entering " + renderExecutor.getPhase() + " in " + LifecycleImpl.class.getName());
}
- PhaseListenerManager phaseListenerMgr = new PhaseListenerManager(this, facesContext, getPhaseListeners());
+ PhaseListenerManager phaseListenerMgr = new PhaseListenerManager(this, facesContext, phaseListeners);
Flash flash = facesContext.getExternalContext().getFlash();
try
diff --git a/impl/src/main/java/org/apache/myfaces/lifecycle/PhaseListenerManager.java b/impl/src/main/java/org/apache/myfaces/lifecycle/PhaseListenerManager.java
index 15ae0190e..a51944b10 100644
--- a/impl/src/main/java/org/apache/myfaces/lifecycle/PhaseListenerManager.java
+++ b/impl/src/main/java/org/apache/myfaces/lifecycle/PhaseListenerManager.java
@@ -19,8 +19,8 @@
package org.apache.myfaces.lifecycle;
-import java.util.HashMap;
-import java.util.Map;
+import java.util.Arrays;
+import java.util.List;
import jakarta.faces.context.FacesContext;
import jakarta.faces.event.ExceptionQueuedEvent;
import jakarta.faces.event.ExceptionQueuedEventContext;
@@ -39,20 +39,38 @@ class PhaseListenerManager
{
private Lifecycle lifecycle;
private FacesContext facesContext;
- private PhaseListener[] phaseListeners;
+ private List<PhaseListener> phaseListeners;
// Tracks success in the beforePhase. Listeners that throw an exception
// in beforePhase or were never called because a previous listener threw
// an exception should not have its afterPhase called
- private Map<PhaseId, boolean[]> listenerSuccessMap;
+ private boolean[] beforePhaseSuccessScratch;
- PhaseListenerManager(Lifecycle lifecycle, FacesContext facesContext, PhaseListener[] phaseListeners)
+ /** Listener count at start of informPhaseListenersBefore; used for matching afterPhase. */
+ private int listenerCountForActivePhase;
+
+ private PhaseId activePhaseIdForBeforeSuccess;
+ private PhaseEvent phaseEventForActivePhase;
+
+ PhaseListenerManager(Lifecycle lifecycle, FacesContext facesContext, List<PhaseListener> phaseListeners)
{
this.lifecycle = lifecycle;
this.facesContext = facesContext;
this.phaseListeners = phaseListeners;
- this.listenerSuccessMap = new HashMap<>(PhaseId.VALUES.size(), 1f);
-
+ }
+
+ /** Bridge for test infrastructure that instantiates via reflection with PhaseListener[]. */
+ PhaseListenerManager(Lifecycle lifecycle, FacesContext facesContext, PhaseListener[] phaseListeners)
+ {
+ this(lifecycle, facesContext, Arrays.asList(phaseListeners));
+ }
+
+ private void ensureScratch(int minSize)
+ {
+ if (beforePhaseSuccessScratch == null || beforePhaseSuccessScratch.length < minSize)
+ {
+ beforePhaseSuccessScratch = new boolean[minSize];
+ }
}
private boolean isListenerForThisPhase(PhaseListener phaseListener, PhaseId phaseId)
@@ -63,24 +81,27 @@ private boolean isListenerForThisPhase(PhaseListener phaseListener, PhaseId phas
void informPhaseListenersBefore(PhaseId phaseId)
{
- boolean[] beforePhaseSuccess = new boolean[phaseListeners.length];
- listenerSuccessMap.put(phaseId, beforePhaseSuccess);
+ listenerCountForActivePhase = phaseListeners.size();
+ ensureScratch(listenerCountForActivePhase);
+ Arrays.fill(beforePhaseSuccessScratch, 0, listenerCountForActivePhase, false);
+ activePhaseIdForBeforeSuccess = phaseId;
PhaseEvent event = new PhaseEvent(facesContext, phaseId, lifecycle);
+ phaseEventForActivePhase = event;
- for (int i = 0; i < phaseListeners.length; i++)
+ for (int i = 0; i < listenerCountForActivePhase; i++)
{
- PhaseListener phaseListener = phaseListeners[i];
+ PhaseListener phaseListener = phaseListeners.get(i);
if (isListenerForThisPhase(phaseListener, phaseId))
{
try
{
phaseListener.beforePhase(event);
- beforePhaseSuccess[i] = true;
+ beforePhaseSuccessScratch[i] = true;
}
catch (Throwable e)
{
- beforePhaseSuccess[i] = false; // redundant - for clarity
+ beforePhaseSuccessScratch[i] = false; // redundant - for clarity
// Faces 2.0: publish exceptions instead of logging them.
@@ -94,25 +115,23 @@ void informPhaseListenersBefore(PhaseId phaseId)
void informPhaseListenersAfter(PhaseId phaseId)
{
- boolean[] beforePhaseSuccess = listenerSuccessMap.get(phaseId);
-
- if (beforePhaseSuccess == null)
+ if (activePhaseIdForBeforeSuccess != phaseId)
{
// informPhaseListenersBefore method was not called : maybe an exception in LifecycleImpl.executePhase
return;
}
- PhaseEvent event = null;
+ PhaseEvent event = phaseEventForActivePhase;
- for (int i = phaseListeners.length - 1; i >= 0; i--)
+ for (int i = listenerCountForActivePhase - 1; i >= 0; i--)
{
- PhaseListener phaseListener = phaseListeners[i];
- if (isListenerForThisPhase(phaseListener, phaseId) && beforePhaseSuccess[i])
+ if (i >= phaseListeners.size())
+ {
+ continue;
+ }
+ PhaseListener phaseListener = phaseListeners.get(i);
+ if (isListenerForThisPhase(phaseListener, phaseId) && beforePhaseSuccessScratch[i])
{
- if (event == null)
- {
- event = new PhaseEvent(facesContext, phaseId, lifecycle);
- }
try
{
phaseListener.afterPhase(event);
@@ -126,6 +145,8 @@ void informPhaseListenersAfter(PhaseId phaseId)
}
}
+ activePhaseIdForBeforeSuccess = null;
+ phaseEventForActivePhase = null;
}
private void publishException(Throwable e, PhaseId phaseId, String key)
diff --git a/impl/src/main/java/org/apache/myfaces/view/facelets/PartialStateManagementStrategy.java b/impl/src/main/java/org/apache/myfaces/view/facelets/PartialStateManagementStrategy.java
index 5e0dd158e..205802530 100644
--- a/impl/src/main/java/org/apache/myfaces/view/facelets/PartialStateManagementStrategy.java
+++ b/impl/src/main/java/org/apache/myfaces/view/facelets/PartialStateManagementStrategy.java
@@ -608,30 +608,31 @@ public void invokeContextCallback(FacesContext context, UIComponent target)
UIComponent parent = target;
if (parent.getChildCount() > 0)
{
+ List<UIComponent> children = parent.getChildren();
String tagId = (String) child.getAttributes().get(ComponentSupport.MARK_CREATED);
- if (childIndex < parent.getChildCount())
+ if (childIndex < children.size())
{
// Try to find the component quickly
- UIComponent dup = parent.getChildren().get(childIndex);
+ UIComponent dup = children.get(childIndex);
if (tagId.equals(dup.getAttributes().get(ComponentSupport.MARK_CREATED)))
{
// Replace
- parent.getChildren().remove(childIndex.intValue());
- parent.getChildren().add(childIndex, child);
+ children.remove(childIndex.intValue());
+ children.add(childIndex, child);
done = true;
}
}
if (!done)
{
// Fallback to iteration
- for (int i = 0, childCount = parent.getChildCount(); i < childCount; i ++)
+ for (int i = 0, childCount = children.size(); i < childCount; i ++)
{
- UIComponent dup = parent.getChildren().get(i);
+ UIComponent dup = children.get(i);
if (tagId.equals(dup.getAttributes().get(ComponentSupport.MARK_CREATED)))
{
// Replace
- parent.getChildren().remove(i);
- parent.getChildren().add(i, child);
+ children.remove(i);
+ children.add(i, child);
done = true;
break;
}
@@ -644,16 +645,17 @@ public void invokeContextCallback(FacesContext context, UIComponent target)
try
{
UIComponent parent = target;
- for (int i = 0, childCount = parent.getChildCount(); i < childCount; i ++)
+ List<UIComponent> children = parent.getChildren();
+ for (int i = 0, childCount = children.size(); i < childCount; i ++)
{
- UIComponent dup = parent.getChildren().get(i);
+ UIComponent dup = children.get(i);
// myfaces-4623: avoid duplicate id exceptions w/ c:forEach for Resource Dependencies
if (child.getId().startsWith(UIViewRoot.UNIQUE_ID_PREFIX + ComponentUtils.RD_ID_PREFIX)
&& child.getId().equals(dup.getId()))
{
// Replace
- parent.getChildren().remove(i);
- parent.getChildren().add(i, child);
+ children.remove(i);
+ children.add(i, child);
done = true;
break;
}
@@ -1003,23 +1005,24 @@ public VisitResult visit(VisitContext context, UIComponent target)
ComponentState componentAddedAfterBuildView
= (ComponentState) target.getAttributes().get(COMPONENT_ADDED_AFTER_BUILD_VIEW);
UIComponent parent = target.getParent();
+ String targetClientId = target.getClientId(facesContext);
//Note if UIViewRoot has this marker, Faces 1.2 like state saving is used.
if (componentAddedAfterBuildView != null && (parent != null))
{
if (ComponentState.REMOVE_ADD.equals(componentAddedAfterBuildView))
{
- registerOnAddRemoveList(facesContext, target.getClientId(facesContext));
+ registerOnAddRemoveList(facesContext, targetClientId);
target.getAttributes().put(COMPONENT_ADDED_AFTER_BUILD_VIEW, ComponentState.ADDED);
}
else if (ComponentState.ADD.equals(componentAddedAfterBuildView))
{
- registerOnAddList(facesContext, target.getClientId(facesContext));
+ registerOnAddList(facesContext, targetClientId);
target.getAttributes().put(COMPONENT_ADDED_AFTER_BUILD_VIEW, ComponentState.ADDED);
}
else if (ComponentState.ADDED.equals(componentAddedAfterBuildView))
{
- registerOnAddList(facesContext, target.getClientId(facesContext));
+ registerOnAddList(facesContext, targetClientId);
}
ensureClearInitialState(target);
//Save all required info to restore the subtree.
@@ -1028,7 +1031,7 @@ else if (ComponentState.ADDED.equals(componentAddedAfterBuildView))
int childIndex = parent.getChildren().indexOf(target);
if (childIndex >= 0)
{
- states.put(target.getClientId(facesContext), new AttachedFullStateWrapper(
+ states.put(targetClientId, new AttachedFullStateWrapper(
new Object[]{
parent.getClientId(facesContext),
null,
@@ -1050,7 +1053,7 @@ else if (ComponentState.ADDED.equals(componentAddedAfterBuildView))
}
}
}
- states.put(target.getClientId(facesContext),new AttachedFullStateWrapper(new Object[]{
+ states.put(targetClientId, new AttachedFullStateWrapper(new Object[]{
parent.getClientId(facesContext),
facetName,
null,
@@ -1065,7 +1068,7 @@ else if (parent != null)
if (state != null)
{
// Save by client ID into our map.
- states.put(target.getClientId(facesContext), state);
+ states.put(targetClientId, state);
}
return VisitResult.ACCEPT;
@@ -1232,6 +1235,7 @@ public VisitResult visit(VisitContext context, UIComponent target)
ComponentState componentAddedAfterBuildView
= (ComponentState) target.getAttributes().get(COMPONENT_ADDED_AFTER_BUILD_VIEW);
UIComponent parent = target.getParent();
+ String targetClientId = target.getClientId(facesContext);
//Note if UIViewRoot has this marker, Faces 1.2 like state saving is used.
if (componentAddedAfterBuildView != null && (parent != null))
@@ -1256,19 +1260,19 @@ public VisitResult visit(VisitContext context, UIComponent target)
{
//If the view has removed components, set the view as non resetable
setViewResetable(false);
- registerOnAddRemoveList(facesContext, target.getClientId(facesContext));
+ registerOnAddRemoveList(facesContext, targetClientId);
target.getAttributes().put(COMPONENT_ADDED_AFTER_BUILD_VIEW, ComponentState.ADDED);
}
else if (ComponentState.ADD.equals(componentAddedAfterBuildView))
{
- registerOnAddList(facesContext, target.getClientId(facesContext));
+ registerOnAddList(facesContext, targetClientId);
target.getAttributes().put(COMPONENT_ADDED_AFTER_BUILD_VIEW, ComponentState.ADDED);
}
else if (ComponentState.ADDED.equals(componentAddedAfterBuildView))
{
// Later on the check of removed components we'll see if the view
// is resetable or not.
- registerOnAddList(facesContext, target.getClientId(facesContext));
+ registerOnAddList(facesContext, targetClientId);
}
ensureClearInitialState(target);
//Save all required info to restore the subtree.
@@ -1277,7 +1281,7 @@ else if (ComponentState.ADDED.equals(componentAddedAfterBuildView))
int childIndex = parent.getChildren().indexOf(target);
if (childIndex >= 0)
{
- states.put(target.getClientId(facesContext), new AttachedFullStateWrapper(
+ states.put(targetClientId, new AttachedFullStateWrapper(
new Object[]{
parent.getClientId(facesContext),
null,
@@ -1299,7 +1303,7 @@ else if (ComponentState.ADDED.equals(componentAddedAfterBuildView))
}
}
}
- states.put(target.getClientId(facesContext), new AttachedFullStateWrapper(new Object[]{
+ states.put(targetClientId, new AttachedFullStateWrapper(new Object[]{
parent.getClientId(facesContext),
facetName,
null,
@@ -1327,7 +1331,7 @@ else if (parent != null)
if (state != null)
{
// Save by client ID into our map.
- states.put(target.getClientId (facesContext), state);
+ states.put(targetClientId, state);
if (isViewResetable())
{
diff --git a/impl/src/main/java/org/apache/myfaces/view/facelets/impl/DefaultFacelet.java b/impl/src/main/java/org/apache/myfaces/view/facelets/impl/DefaultFacelet.java
index fadc5a211..8b3bb1497 100644
--- a/impl/src/main/java/org/apache/myfaces/view/facelets/impl/DefaultFacelet.java
+++ b/impl/src/main/java/org/apache/myfaces/view/facelets/impl/DefaultFacelet.java
@@ -31,7 +31,7 @@
import java.util.Iterator;
import java.util.List;
import java.util.Map;
-import java.util.WeakHashMap;
+import java.util.concurrent.ConcurrentHashMap;
import java.util.logging.Level;
import java.util.logging.Logger;
@@ -95,7 +95,7 @@ public DefaultFacelet(DefaultFaceletFactory factory, ExpressionFactory el, URL s
_createTime = System.currentTimeMillis();
_refreshPeriod = _factory.getRefreshPeriod();
- _relativePaths = Collections.synchronizedMap(new WeakHashMap());
+ _relativePaths = new ConcurrentHashMap<>();
_isBuildingCompositeComponentMetadata = false;
_encodingHandler = (root instanceof EncodingHandler);
this.viewUniqueIdsCacheEnabled = viewUniqueIdsCacheEnabled;
@@ -115,7 +115,7 @@ public DefaultFacelet(DefaultFaceletFactory factory, ExpressionFactory el, URL s
_createTime = System.currentTimeMillis();
_refreshPeriod = _factory.getRefreshPeriod();
- _relativePaths = Collections.synchronizedMap(new WeakHashMap());
+ _relativePaths = new ConcurrentHashMap<>();
_isBuildingCompositeComponentMetadata = isBuildingCompositeComponentMetadata;
_encodingHandler = (root instanceof EncodingHandler);
this.viewUniqueIdsCacheEnabled = viewUniqueIdsCacheEnabled;
diff --git a/impl/src/main/java/org/apache/myfaces/view/facelets/impl/DefaultFaceletFactory.java b/impl/src/main/java/org/apache/myfaces/view/facelets/impl/DefaultFaceletFactory.java
index 705ffa2a3..6e573763b 100644
--- a/impl/src/main/java/org/apache/myfaces/view/facelets/impl/DefaultFaceletFactory.java
+++ b/impl/src/main/java/org/apache/myfaces/view/facelets/impl/DefaultFaceletFactory.java
@@ -26,8 +26,6 @@
import java.util.Optional;
import java.util.logging.Level;
import java.util.logging.Logger;
-import java.util.regex.Pattern;
-
import jakarta.el.ELException;
import jakarta.enterprise.inject.spi.BeanManager;
import jakarta.faces.FacesException;
@@ -594,12 +592,19 @@ public Facelet compileComponentFacelet(String taglibURI, String tagName, Map<Str
* @param toRemove
* @return
*/
- private String _removeFirst(String string, String toRemove)
+ protected String _removeFirst(String string, String toRemove)
{
- // do exactly what String.replaceFirst(toRemove, "") internally does,
- // except treating the search as literal text and not as regex
-
- return Pattern.compile(toRemove, Pattern.LITERAL).matcher(string).replaceFirst("");
+ // Literal first occurrence removal (same idea as Pattern.LITERAL + replaceFirst, without regex setup).
+ if (toRemove == null || toRemove.isEmpty())
+ {
+ return string;
+ }
+ int idx = string.indexOf(toRemove);
+ if (idx < 0)
+ {
+ return string;
+ }
+ return string.substring(0, idx) + string.substring(idx + toRemove.length());
}
}
diff --git a/impl/src/main/java/org/apache/myfaces/view/facelets/tag/MetaRulesetImpl.java b/impl/src/main/java/org/apache/myfaces/view/facelets/tag/MetaRulesetImpl.java
index d34e383cb..b2a4f7b81 100644
--- a/impl/src/main/java/org/apache/myfaces/view/facelets/tag/MetaRulesetImpl.java
+++ b/impl/src/main/java/org/apache/myfaces/view/facelets/tag/MetaRulesetImpl.java
@@ -18,7 +18,6 @@
*/
package org.apache.myfaces.view.facelets.tag;
-import org.apache.myfaces.util.lang.ClassUtils;
import jakarta.faces.view.facelets.MetaRule;
import jakarta.faces.view.facelets.MetaRuleset;
import jakarta.faces.view.facelets.Metadata;
@@ -31,7 +30,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
-import java.util.WeakHashMap;
+import java.util.concurrent.ConcurrentHashMap;
import java.util.logging.Level;
import java.util.logging.Logger;
import jakarta.faces.context.FacesContext;
@@ -49,44 +48,29 @@ public final class MetaRulesetImpl extends MetaRuleset
{
private final static Logger log = Logger.getLogger(MetaRulesetImpl.class.getName());
- /**
- * Cache the MetadataTarget instances per ClassLoader using the Class-Name of the type variable
- * of MetadataTarget.
- * NOTE that we do it this way, because the only other valid way in order to support a shared
- * classloader scenario would be to use a WeakHashMap<Class<?>, MetadataTarget>, but this
- * creates a cyclic reference between the key and the value of the WeakHashMap which will
- * most certainly cause a memory leak! Furthermore we can manually cleanup the Map when
- * the webapp is undeployed just by removing the Map for the current ClassLoader.
- */
- private volatile static WeakHashMap<ClassLoader, Map<String, MetadataTarget>> metadata
- = new WeakHashMap<ClassLoader, Map<String, MetadataTarget>>();
+ private static final String METADATA_KEY = MetaRulesetImpl.class.getName() + ".METADATA";
/**
- * Removes the cached MetadataTarget instances in order to prevent a memory leak.
+ * MetadataTarget cache keyed by component class name, stored in the JSF application map so it
+ * is released with the web application (no separate undeploy hook).
*/
- public static void clearMetadataTargetCache()
- {
- metadata.remove(ClassUtils.getContextClassLoader());
- }
-
- private static Map<String, MetadataTarget> getMetaData()
+ @SuppressWarnings("unchecked")
+ private static ConcurrentHashMap<String, MetadataTarget> getMetaData()
{
- ClassLoader cl = ClassUtils.getContextClassLoader();
-
- Map<String, MetadataTarget> metadata = MetaRulesetImpl.metadata.get(cl);
-
- if (metadata == null)
+ FacesContext facesContext = FacesContext.getCurrentInstance();
+ if (facesContext == null)
{
- // Ensure thread-safe put over _metadata, and only create one map
- // per classloader to hold metadata.
- synchronized (MetaRulesetImpl.metadata)
- {
- metadata = MetaRulesetImpl.metadata.computeIfAbsent(cl,
- k -> new HashMap<>());
- }
+ throw new IllegalStateException("No FacesContext is available");
}
-
- return metadata;
+ Map<String, Object> applicationMap = facesContext.getExternalContext().getApplicationMap();
+ ConcurrentHashMap<String, MetadataTarget> map =
+ (ConcurrentHashMap<String, MetadataTarget>) applicationMap.get(METADATA_KEY);
+ if (map != null)
+ {
+ return map;
+ }
+ return (ConcurrentHashMap<String, MetadataTarget>) applicationMap.computeIfAbsent(METADATA_KEY,
+ k -> new ConcurrentHashMap<>(64));
}
private final static TagAttribute[] EMPTY = new TagAttribute[0];
@@ -320,40 +304,28 @@ public MetaRuleset ignoreAll()
private MetadataTarget _getMetadataTarget()
{
- Map<String, MetadataTarget> metadata = getMetaData();
+ ConcurrentHashMap<String, MetadataTarget> metadata = getMetaData();
String metaKey = _type.getName();
-
MetadataTarget meta = metadata.get(metaKey);
- if (meta == null)
+ if (meta != null)
+ {
+ return meta;
+ }
+ return metadata.computeIfAbsent(metaKey, key ->
{
try
{
if (PropertyDescriptorUtils.isUseLambdaMetafactory(
FacesContext.getCurrentInstance().getExternalContext()))
{
- meta = new LambdaMetadataTargetImpl(_type);
- }
- else
- {
- meta = new MetadataTargetImpl(_type);
+ return new LambdaMetadataTargetImpl(_type);
}
+ return new MetadataTargetImpl(_type);
}
catch (IntrospectionException e)
{
throw new TagException(_tag, "Error Creating TargetMetadata", e);
}
-
- synchronized(metadata)
- {
- // Use a synchronized block to ensure proper operation on concurrent use cases.
- // This is a racy single check, because initialization over the same class could happen
- // multiple times, but the same result is always calculated. The synchronized block
- // just ensure thread-safety, because only one thread will modify the cache map
- // at the same time.
- metadata.put(metaKey, meta);
- }
- }
-
- return meta;
+ });
}
}
diff --git a/impl/src/main/java/org/apache/myfaces/view/facelets/tag/composite/CompositeMetaRulesetImpl.java b/impl/src/main/java/org/apache/myfaces/view/facelets/tag/composite/CompositeMetaRulesetImpl.java
index 1d5b58a56..0a0ec55cd 100644
--- a/impl/src/main/java/org/apache/myfaces/view/facelets/tag/composite/CompositeMetaRulesetImpl.java
+++ b/impl/src/main/java/org/apache/myfaces/view/facelets/tag/composite/CompositeMetaRulesetImpl.java
@@ -34,6 +34,7 @@
import java.beans.IntrospectionException;
import java.util.ArrayList;
import java.util.HashMap;
+import java.util.concurrent.ConcurrentHashMap;
import java.util.List;
import java.util.Map;
import java.util.logging.Level;
@@ -49,15 +50,18 @@ public class CompositeMetaRulesetImpl extends MetaRuleset
private static final String METADATA_KEY = CompositeMetaRulesetImpl.class.getName() + ".METADATA";
- private static Map<String, MetadataTarget> getMetaData()
+ private static ConcurrentHashMap<String, MetadataTarget> getMetaData()
{
FacesContext facesContext = FacesContext.getCurrentInstance();
Map<String, Object> applicationMap = facesContext.getExternalContext().getApplicationMap();
-
- Map<String, MetadataTarget> metadata = (Map<String, MetadataTarget>) applicationMap.computeIfAbsent(
- METADATA_KEY, k -> new HashMap<>());
-
- return metadata;
+ ConcurrentHashMap<String, MetadataTarget> map =
+ (ConcurrentHashMap<String, MetadataTarget>) applicationMap.get(METADATA_KEY);
+ if (map != null)
+ {
+ return map;
+ }
+ return (ConcurrentHashMap<String, MetadataTarget>) applicationMap.computeIfAbsent(METADATA_KEY,
+ k -> new ConcurrentHashMap<>(64));
}
private final Map<String, TagAttribute> _attributes;
@@ -206,32 +210,28 @@ private MetadataTarget _getMetadataTarget()
private MetadataTarget _getBaseMetadataTarget()
{
- Map<String, MetadataTarget> metadata = getMetaData();
+ ConcurrentHashMap<String, MetadataTarget> metadata = getMetaData();
String key = _type.getName();
-
MetadataTarget meta = metadata.get(key);
- if (meta == null)
+ if (meta != null)
+ {
+ return meta;
+ }
+ return metadata.computeIfAbsent(key, k ->
{
try
{
if (PropertyDescriptorUtils.isUseLambdaMetafactory(
FacesContext.getCurrentInstance().getExternalContext()))
{
- meta = new LambdaMetadataTargetImpl(_type);
- }
- else
- {
- meta = new MetadataTargetImpl(_type);
+ return new LambdaMetadataTargetImpl(_type);
}
+ return new MetadataTargetImpl(_type);
}
catch (IntrospectionException e)
{
throw new TagException(_tag, "Error Creating TargetMetadata", e);
}
-
- metadata.put(key, meta);
- }
-
- return meta;
+ });
}
}
diff --git a/impl/src/main/java/org/apache/myfaces/webapp/FacesInitializerImpl.java b/impl/src/main/java/org/apache/myfaces/webapp/FacesInitializerImpl.java
index 6e77859b9..67d5e284b 100644
--- a/impl/src/main/java/org/apache/myfaces/webapp/FacesInitializerImpl.java
+++ b/impl/src/main/java/org/apache/myfaces/webapp/FacesInitializerImpl.java
@@ -31,8 +31,6 @@
import org.apache.myfaces.spi.InjectionProviderException;
import org.apache.myfaces.spi.InjectionProviderFactory;
import org.apache.myfaces.util.ExternalSpecifications;
-import org.apache.myfaces.view.facelets.tag.MetaRulesetImpl;
-
import jakarta.el.ELManager;
import jakarta.el.ExpressionFactory;
import jakarta.faces.application.Application;
@@ -328,9 +326,6 @@ public void destroyFaces(ServletContext servletContext)
_dispatchApplicationEvent(servletContext, PreDestroyApplicationEvent.class);
_callPreDestroyOnInjectedJSFArtifacts(facesContext);
-
- // clear the cache of MetaRulesetImpl in order to prevent a memory leak
- MetaRulesetImpl.clearMetadataTargetCache();
if (facesContext.getExternalContext().getApplicationMap().containsKey(PUSH_INITIALIZED))
{