mojarra icon indicating copy to clipboard operation
mojarra copied to clipboard

Performance issue with ELUtils#isCompositeComponentMethodExprLookup

Open javaserverfaces opened this issue 10 years ago • 6 comments

On a complex page I hit a performance issue apparently caused by the regular expression-based methods in ELUtils that are used for classifying tag attributes:

isCompositeComponentLookupWithArgs isCompositeComponentMethodExprLookup isCompositeComponentExpr

These methods get called many hundreds of times on my page, and I suspect that the performance drop can be attributed mostly to a few way too long EL expressions.

With the attached naive caching of expressions I can cut the server-side rendering time of this page in half.

Since these methods are only called from TagAttributeImpl, I think that a cleaner solution would be to cache the results of these methods in three boolean fields in the TagAttributeImpl class itself - as is already being done for the result of ELUtils.isLiteral().

However this would require a small refactoring of the private (public?) method getValueExpression(FaceletContext ctx, String expr, Class type), since it can be called with any expression (not just the expression of the TagAttributeImpl). Thoughts?

Environment

Glassfish 3.1.2

Affected Versions

[2.1.6]

javaserverfaces avatar Nov 12 '15 16:11 javaserverfaces

Reported by dlichtenberger

javaserverfaces avatar Nov 12 '15 16:11 javaserverfaces

dlichtenberger said: Apparently I cannot add attachments, so here's the patch (against 2.1.6, but the source in the current Git master seems to be unchanged):

--- jsf-ri/src/main/java/com/sun/faces/el/ELUtils.java	(revision 13974)
+++ jsf-ri/src/main/java/com/sun/faces/el/ELUtils.java	(revision )
@@ -66,6 +66,8 @@
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;

@@ -106,6 +108,10 @@
     private static final Pattern METHOD_EXPRESSION_LOOKUP =
           Pattern.compile(".[{]cc[.]attrs[.]\\w+[}]");

+    private static final ConcurrentMap<String, Boolean> CACHE_COMPOSITE_WITH_ARGS = new ConcurrentHashMap<String, Boolean>();
+    private static final ConcurrentMap<String, Boolean> CACHE_COMPOSITE_EXPR = new ConcurrentHashMap<String, Boolean>();
+    private static final ConcurrentMap<String, Boolean> CACHE_METHOD_EXPR = new ConcurrentHashMap<String, Boolean>();
+
     private static final String APPLICATION_SCOPE = "applicationScope";
     private static final String SESSION_SCOPE = "sessionScope";
     private static final String REQUEST_SCOPE = "requestScope";
@@ -188,30 +194,31 @@

     public static boolean isCompositeComponentExpr(String expression) {
+        return cachedMatcher(CACHE_COMPOSITE_EXPR, COMPOSITE_COMPONENT_EXPRESSION, expression);

-        // TODO we should be trying to re-use the Matcher by calling -        // m.reset(expression); -        Matcher m = COMPOSITE_COMPONENT_EXPRESSION.matcher(expression);
-        return m.find();
-
     }

     public static boolean isCompositeComponentMethodExprLookup(String expression) {
+        return cachedMatcher(CACHE_METHOD_EXPR, METHOD_EXPRESSION_LOOKUP, expression);

-        Matcher m = METHOD_EXPRESSION_LOOKUP.matcher(expression);
-        return m.matches();
-
     }

     public static boolean isCompositeComponentLookupWithArgs(String expression) {
+        return cachedMatcher(CACHE_COMPOSITE_WITH_ARGS, COMPOSITE_COMPONENT_LOOKUP_WITH_ARGS, expression);
-
+        
-        // TODO we should be trying to re-use the Matcher by calling -        // m.reset(expression); -        Matcher m = COMPOSITE_COMPONENT_LOOKUP_WITH_ARGS.matcher(expression);
-        return m.find();
+    }
-        
+
+    private static boolean cachedMatcher(ConcurrentMap<String, Boolean> cache, Pattern regexp, String expression) {
+        Boolean result = cache.get(expression);
+        if (result != null) {
+            return result;
+        }
+        Matcher m = regexp.matcher(expression);
+        result = m.find();
+        cache.put(expression, result);
+        return result;
     }

javaserverfaces avatar Nov 12 '15 16:11 javaserverfaces

Was assigned to ren.zhijun.oracle

javaserverfaces avatar Nov 12 '15 16:11 javaserverfaces

This issue was imported from java.net JIRA JAVASERVERFACES-4078

javaserverfaces avatar May 02 '17 10:05 javaserverfaces

Please see this important message regarding community contributions to Mojarra.

https://javaee.groups.io/g/jsf-spec/message/30

Also, please consider joining that group, as that group has taken the place of the old [email protected] mailing list.

Thanks,

Ed Burns

edburns avatar Oct 29 '17 03:10 edburns

This will not be fixed until somebody who signed the OCA contributes a PR, correct? Or will @xinyuan-zhang apply the patch by dlichtenberger?

Roughly 80% of CPU Time of our application is spend in these methods :-\

kguelzau avatar May 23 '18 15:05 kguelzau