Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=57583

Improve long standing performance issue with EL and undefined attributes. git-svn-id: https://svn.apache.org/repos/asf/tomcat/tc8.0.x/trunk@1734419 13f79535-47bb-0310-9956-ffa450edef68
apache · Mar 10, 2016 · b69ed7e · b69ed7e
1 parent ad33dbd
commit b69ed7e
Show file tree

Hide file tree

Showing 3 changed files with 56 additions and 4 deletions.
diff --git a/java/javax/servlet/jsp/el/ScopedAttributeELResolver.java b/java/javax/servlet/jsp/el/ScopedAttributeELResolver.java
@@ -35,8 +35,18 @@
 */
 public class ScopedAttributeELResolver extends ELResolver {
 
-    public ScopedAttributeELResolver() {
-        super();
+    // Indicates if a performance short-cut is available
+    private static final Class<?> AST_IDENTIFIER_KEY;
+
+    static {
+        Class<?> key = null;
+        try {
+            key = Class.forName("org.apache.el.parser.AstIdentifier");
+        } catch (Exception e) {
+            // Ignore: Expected if not running on Tomcat. Not a problem since
+            //         this just allows a short-cut.
+        }
+        AST_IDENTIFIER_KEY = key;
     }
 
     @Override
@@ -56,10 +66,26 @@ public Object getValue(ELContext context, Object base, Object property) {
                 result = page.findAttribute(key);
 
                 if (result == null) {
+                    boolean resolveClass = true;
+                    // Performance short-cut available when running on Tomcat
+                    if (AST_IDENTIFIER_KEY != null) {
+                        // Tomcat will set this key to Boolean.TRUE if the
+                        // identifier is a stand-alone identifier (i.e.
+                        // identifier) rather than part of an AstValue (i.e.
+                        // identifier.something). Imports do not need to be
+                        // checked if this is a stand-alone identifier
+                        Boolean value = (Boolean) context.getContext(AST_IDENTIFIER_KEY);
+                        if (value != null && value.booleanValue()) {
+                            resolveClass = false;
+                        }
+                    }
                     // This might be the name of an imported class
                     ImportHandler importHandler = context.getImportHandler();
                     if (importHandler != null) {
-                        Class<?> clazz = importHandler.resolveClass(key);
+                        Class<?> clazz = null;
+                        if (resolveClass) {
+                            clazz = importHandler.resolveClass(key);
+                        }
                         if (clazz != null) {
                             result = new ELClass(clazz);
                         }

diff --git a/java/org/apache/el/parser/AstIdentifier.java b/java/org/apache/el/parser/AstIdentifier.java
@@ -77,7 +77,27 @@ public Object getValue(EvaluationContext ctx) throws ELException {
 
         // EL Resolvers
         ctx.setPropertyResolved(false);
-        Object result = ctx.getELResolver().getValue(ctx, null, this.image);
+        Object result;
+        /* Putting the Boolean into the ELContext is part of a performance
+         * optimisation for ScopedAttributeELResolver. When looking up "foo",
+         * the resolver can't differentiate between ${ foo } and ${ foo.bar }.
+         * This is important because the expensive class lookup only needs to
+         * be performed in the later case. This flag tells the resolver if the
+         * lookup can be skipped.
+         */
+        if (parent instanceof AstValue) {
+            ctx.putContext(this.getClass(), Boolean.FALSE);
+        } else {
+            ctx.putContext(this.getClass(), Boolean.TRUE);
+        }
+        try {
+            result = ctx.getELResolver().getValue(ctx, null, this.image);
+        } finally {
+            // Always reset the flag to false so the optimisation is not applied
+            // inappropriately
+            ctx.putContext(this.getClass(), Boolean.FALSE);
+        }
+
         if (ctx.isPropertyResolved()) {
             return result;
         }

diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
@@ -195,6 +195,12 @@
   </subsection>
   <subsection name="Jasper">
     <changelog>
+      <fix>
+        <bug>57583</bug>: Improve the performance of
+        <code>javax.servlet.jsp.el.ScopedAttributeELResolver</code> when
+        resolving attributes that do not exist. This improvement only works when
+        Jasper is used with with Tomcat's EL implementation. (markt)
+      </fix>
       <update>
         <bug>58111</bug>: Update to the Eclipse JDT Compiler 4.5. (markt)
       </update>