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

Cache NotFound results in ImportHandler to save repeated attempts to load classes that do not exist.

git-svn-id: https://svn.apache.org/repos/asf/tomcat/tc8.0.x/trunk@1660134 13f79535-47bb-0310-9956-ffa450edef68

java/javax/el/ImportHandler.java

@@ -140,7 +140,11 @@ public java.lang.Class<?> resolveClass(String name) {
         Class<?> result = clazzes.get(name);
 
         if (result != null) {
-            return result;
+            if (NotFound.class.equals(result)) {
+                return null;
+            } else {
+                return result;
+            }
         }
 
         // Search the class imports
@@ -167,7 +171,11 @@ public java.lang.Class<?> resolveClass(String name) {
                 result = clazz;
             }
         }
-        if (result != null) {
+        if (result == null) {
+            // Cache NotFound results to save repeated calls to findClass()
+            // which is relatively slow
+            clazzes.put(name, NotFound.class);
+        } else {
             clazzes.put(name, result);
         }
 
@@ -199,4 +207,12 @@ private Class<?> findClass(String name) {
 
         return clazz;
     }
+
+
+    /*
+     * Marker class used because null values are not permitted in a
+     * ConcurrentHashMap.
+     */
+    private static class NotFound {
+    }
 }

test/javax/servlet/jsp/TesterPageContext.java

@@ -0,0 +1,195 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package javax.servlet.jsp;
+
+import java.io.IOException;
+import java.util.Enumeration;
+
+import javax.el.ELContext;
+import javax.servlet.Servlet;
+import javax.servlet.ServletConfig;
+import javax.servlet.ServletContext;
+import javax.servlet.ServletException;
+import javax.servlet.ServletRequest;
+import javax.servlet.ServletResponse;
+import javax.servlet.http.HttpSession;
+
+public class TesterPageContext extends PageContext {
+
+    @Override
+    public void initialize(Servlet servlet, ServletRequest request,
+            ServletResponse response, String errorPageURL,
+            boolean needsSession, int bufferSize, boolean autoFlush)
+            throws IOException, IllegalStateException, IllegalArgumentException {
+        // NO-OP
+    }
+
+    @Override
+    public void release() {
+        // NO-OP
+    }
+
+    @Override
+    public HttpSession getSession() {
+        // NO-OP
+        return null;
+    }
+
+    @Override
+    public Object getPage() {
+        // NO-OP
+        return null;
+    }
+
+    @Override
+    public ServletRequest getRequest() {
+        // NO-OP
+        return null;
+    }
+
+    @Override
+    public ServletResponse getResponse() {
+        // NO-OP
+        return null;
+    }
+
+    @Override
+    public Exception getException() {
+        // NO-OP
+        return null;
+    }
+
+    @Override
+    public ServletConfig getServletConfig() {
+        // NO-OP
+        return null;
+    }
+
+    @Override
+    public ServletContext getServletContext() {
+        // NO-OP
+        return null;
+    }
+
+    @Override
+    public void forward(String relativeUrlPath) throws ServletException,
+            IOException {
+        // NO-OP
+
+    }
+
+    @Override
+    public void include(String relativeUrlPath) throws ServletException,
+            IOException {
+        // NO-OP
+    }
+
+    @Override
+    public void include(String relativeUrlPath, boolean flush)
+            throws ServletException, IOException {
+        // NO-OP
+    }
+
+    @Override
+    public void handlePageException(Exception e) throws ServletException,
+            IOException {
+        // NO-OP
+    }
+
+    @Override
+    public void handlePageException(Throwable t) throws ServletException,
+            IOException {
+        // NO-OP
+    }
+
+    @Override
+    public void setAttribute(String name, Object value) {
+        // NO-OP
+    }
+
+    @Override
+    public void setAttribute(String name, Object value, int scope) {
+        // NO-OP
+    }
+
+    @Override
+    public Object getAttribute(String name) {
+        // NO-OP
+        return null;
+    }
+
+    @Override
+    public Object getAttribute(String name, int scope) {
+        // NO-OP
+        return null;
+    }
+
+    @Override
+    public Object findAttribute(String name) {
+        // NO-OP
+        return null;
+    }
+
+    @Override
+    public void removeAttribute(String name) {
+        // NO-OP
+    }
+
+    @Override
+    public void removeAttribute(String name, int scope) {
+        // NO-OP
+    }
+
+    @Override
+    public int getAttributesScope(String name) {
+        // NO-OP
+        return 0;
+    }
+
+    @Override
+    public Enumeration<String> getAttributeNamesInScope(int scope) {
+        // NO-OP
+        return null;
+    }
+
+    @Override
+    public JspWriter getOut() {
+        // NO-OP
+        return null;
+    }
+
+    @Override
+    @Deprecated
+    public javax.servlet.jsp.el.ExpressionEvaluator getExpressionEvaluator() {
+        // NO-OP
+        return null;
+    }
+
+    @Override
+    public ELContext getELContext() {
+        // NO-OP
+        return null;
+    }
+
+    @Override
+    @Deprecated
+    public javax.servlet.jsp.el.VariableResolver getVariableResolver() {
+        // NO-OP
+        return null;
+    }
+
+}

test/javax/servlet/jsp/el/TestScopedAttributeELResolverPerformance.java

@@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package javax.servlet.jsp.el;
+
+import javax.el.ELContext;
+import javax.el.ELManager;
+import javax.el.ELResolver;
+import javax.el.StandardELContext;
+import javax.servlet.jsp.JspContext;
+import javax.servlet.jsp.TesterPageContext;
+
+import org.junit.Test;
+
+public class TestScopedAttributeELResolverPerformance {
+
+    /*
+     * With the caching of NotFound responses this test takes ~20ms. Without the
+     * caching it takes ~6s.
+     */
+    @Test
+    public void testGetValuePerformance() throws Exception {
+
+        ELContext context = new StandardELContext(ELManager.getExpressionFactory());
+
+        context.putContext(JspContext.class, new TesterPageContext());
+
+        ELResolver resolver = new ScopedAttributeELResolver();
+
+        for (int i = 0; i < 100000; i++) {
+            resolver.getValue(context, null, "unknown");
+        }
+    }
+}

webapps/docs/changelog.xml

@@ -45,6 +45,16 @@
   issues to not "pop up" wrt. others).
 -->
 <section name="Tomcat 8.0.21 (markt)" rtext="in development">
+  <subsection name="Jasper">
+    <changelog>
+      <fix>
+        <bug>57583</bug>: Cache &apos;Not Found&apos; results in
+        <code>javax.el.ImportHandler.resolveClass()</code> to save repeated
+        attempts to load classes that are known not to exists to improve
+        performance. (markt)
+      </fix>
+    </changelog>
+  </subsection>
 </section>
 <section name="Tomcat 8.0.20 (markt)" rtext="voting in progress">
   <subsection name="Coyote">