This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/9.0.x by this push:
     new 59503e798c BZ 69419: getAttribute() performance for nested 
ApplicationHttpRequest
59503e798c is described below

commit 59503e798cfe0a1e131ee68a2661e3034c777387
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Mon Nov 4 11:53:18 2024 +0000

    BZ 69419: getAttribute() performance for nested ApplicationHttpRequest
    
    Avoid repeated getSpecial() calls with nested ApplicationHttpRequest
    https://bz.apache.org/bugzilla/show_bug.cgi?id=69419
    Based on a patch by John Engebretson
---
 .../catalina/core/ApplicationHttpRequest.java      | 12 +++++-
 .../catalina/core/TestApplicationHttpRequest.java  | 44 ++++++++++++++++------
 .../TesterApplicationHttpRequestPerformance.java   | 34 ++++++++++++++++-
 3 files changed, 75 insertions(+), 15 deletions(-)

diff --git a/java/org/apache/catalina/core/ApplicationHttpRequest.java 
b/java/org/apache/catalina/core/ApplicationHttpRequest.java
index df29788973..ef9f89761f 100644
--- a/java/org/apache/catalina/core/ApplicationHttpRequest.java
+++ b/java/org/apache/catalina/core/ApplicationHttpRequest.java
@@ -235,7 +235,17 @@ class ApplicationHttpRequest extends 
HttpServletRequestWrapper {
 
         int pos = getSpecial(name);
         if (pos == -1) {
-            return getRequest().getAttribute(name);
+            /*
+             * With nested includes there will be nested 
ApplicationHttpRequests. The calls to getSpecial() are
+             * relatively expensive and it is known at this point that the 
attribute is not special. Therefore, jump to
+             * the first wrapped request that isn't an instance of 
ApplicationHttpRequest before calling getAttribute()
+             * to avoid a call to getSpecial() for each nested 
ApplicationHttpRequest.
+             */
+            ServletRequest wrappedRequest = getRequest();
+            while (wrappedRequest instanceof ApplicationHttpRequest) {
+                wrappedRequest = ((ApplicationHttpRequest) 
wrappedRequest).getRequest();
+            }
+            return wrappedRequest.getAttribute(name);
         } else {
             if ((specialAttributes[pos] == null) && 
(specialAttributes[SPECIALS_FIRST_FORWARD_INDEX] == null) &&
                     (pos >= SPECIALS_FIRST_FORWARD_INDEX)) {
diff --git a/test/org/apache/catalina/core/TestApplicationHttpRequest.java 
b/test/org/apache/catalina/core/TestApplicationHttpRequest.java
index 45a7945529..6cac1091f1 100644
--- a/test/org/apache/catalina/core/TestApplicationHttpRequest.java
+++ b/test/org/apache/catalina/core/TestApplicationHttpRequest.java
@@ -31,6 +31,7 @@ import org.junit.Assert;
 import org.junit.Test;
 
 import org.apache.catalina.Context;
+import org.apache.catalina.connector.Request;
 import org.apache.catalina.startup.Tomcat;
 import org.apache.catalina.startup.TomcatBaseTest;
 import org.apache.tomcat.util.ExceptionUtils;
@@ -170,15 +171,15 @@ public class TestApplicationHttpRequest extends 
TomcatBaseTest {
         String test = "\u0422\u0435\u0441\u0442";
         String query = test + "=%D0%A2%D0%B5%D1%81%D1%82";
 
-        Map<String, String[]> expected = new HashMap<>();
+        Map<String,String[]> expected = new HashMap<>();
         expected.put("a", new String[] { "b" });
         expected.put(test, new String[] { test });
         doQueryStringTest("a=b", query, expected);
     }
 
 
-    private void doQueryStringTest(String originalQueryString, String 
forwardQueryString,
-            Map<String,String[]> expected) throws Exception {
+    private void doQueryStringTest(String originalQueryString, String 
forwardQueryString, Map<String,String[]> expected)
+            throws Exception {
         Tomcat tomcat = getTomcatInstance();
 
         // No file system docBase required
@@ -211,6 +212,29 @@ public class TestApplicationHttpRequest extends 
TomcatBaseTest {
     }
 
 
+    private static HttpServletRequest getNestedRequest(int depth) {
+        if (depth <= 0) {
+            org.apache.coyote.Request coyoteRequest = new 
org.apache.coyote.Request();
+            Request request = new Request(null);
+            request.setCoyoteRequest(coyoteRequest);
+            request.setAttribute("key", "value");
+            return request;
+        }
+        return new ApplicationHttpRequest(getNestedRequest(depth - 1), null, 
false);
+    }
+
+
+    @Test
+    public void testAttributeRetrieval() {
+        int[] depths = { 0, 1, 4, 7 };
+        for (int depth : depths) {
+            HttpServletRequest httpRequest = getNestedRequest(depth);
+            Assert.assertEquals("value", httpRequest.getAttribute("key"));
+            Assert.assertEquals(null, httpRequest.getAttribute("nonexistent 
entry"));
+        }
+    }
+
+
     @Test
     public void testParameterImmutability() throws Exception {
         Tomcat tomcat = getTomcatInstance();
@@ -248,8 +272,7 @@ public class TestApplicationHttpRequest extends 
TomcatBaseTest {
         }
 
         @Override
-        protected void doGet(HttpServletRequest req, HttpServletResponse resp)
-                throws ServletException, IOException {
+        protected void doGet(HttpServletRequest req, HttpServletResponse resp) 
throws ServletException, IOException {
             req.setCharacterEncoding("UTF-8");
             req.getRequestDispatcher(target).forward(req, resp);
         }
@@ -267,8 +290,7 @@ public class TestApplicationHttpRequest extends 
TomcatBaseTest {
         }
 
         @Override
-        protected void doGet(HttpServletRequest req, HttpServletResponse resp)
-                throws ServletException, IOException {
+        protected void doGet(HttpServletRequest req, HttpServletResponse resp) 
throws ServletException, IOException {
             req.setCharacterEncoding("UTF-8");
             resp.setContentType("text/plain");
             resp.setCharacterEncoding("UTF-8");
@@ -278,8 +300,7 @@ public class TestApplicationHttpRequest extends 
TomcatBaseTest {
             boolean ok = true;
             for (Entry<String,String[]> entry : actual.entrySet()) {
                 String[] expectedValue = expected.get(entry.getKey());
-                if (expectedValue == null ||
-                        expectedValue.length != entry.getValue().length) {
+                if (expectedValue == null || expectedValue.length != 
entry.getValue().length) {
                     ok = false;
                     break;
                 }
@@ -329,10 +350,9 @@ public class TestApplicationHttpRequest extends 
TomcatBaseTest {
 
         // Suppress warnings generated because the code is trying to put the
         // wrong type of values into the Map
-        @SuppressWarnings({"rawtypes", "unchecked"})
+        @SuppressWarnings({ "rawtypes", "unchecked" })
         @Override
-        protected void doGet(HttpServletRequest req, HttpServletResponse resp)
-                throws ServletException, IOException {
+        protected void doGet(HttpServletRequest req, HttpServletResponse resp) 
throws ServletException, IOException {
             Map map = req.getParameterMap();
 
             boolean insertWorks;
diff --git 
a/test/org/apache/catalina/core/TesterApplicationHttpRequestPerformance.java 
b/test/org/apache/catalina/core/TesterApplicationHttpRequestPerformance.java
index 03d5932eb3..9412686e55 100644
--- a/test/org/apache/catalina/core/TesterApplicationHttpRequestPerformance.java
+++ b/test/org/apache/catalina/core/TesterApplicationHttpRequestPerformance.java
@@ -16,6 +16,8 @@
  */
 package org.apache.catalina.core;
 
+import javax.servlet.http.HttpServletRequest;
+
 import org.junit.Test;
 
 import org.apache.catalina.connector.Request;
@@ -31,7 +33,7 @@ public class TesterApplicationHttpRequestPerformance {
         org.apache.coyote.Request coyoteRequest = new 
org.apache.coyote.Request();
         Request request = new Request(null);
         request.setCoyoteRequest(coyoteRequest);
-        ApplicationHttpRequest applicationHttpRequest = new 
ApplicationHttpRequest(request, null ,false);
+        ApplicationHttpRequest applicationHttpRequest = new 
ApplicationHttpRequest(request, null, false);
 
         // Warm-up
         doTestGetAttribute(applicationHttpRequest);
@@ -44,9 +46,37 @@ public class TesterApplicationHttpRequestPerformance {
     }
 
 
-    private void doTestGetAttribute(ApplicationHttpRequest request) {
+    private void doTestGetAttribute(HttpServletRequest request) {
         for (int i = 0; i < 100000000; i++) {
             request.getAttribute("Unknown");
         }
     }
+
+    private static HttpServletRequest getRequest(int depth) {
+        if (depth <= 0) {
+            org.apache.coyote.Request coyoteRequest = new 
org.apache.coyote.Request();
+            Request request = new Request(null);
+            request.setCoyoteRequest(coyoteRequest);
+            return request;
+        }
+        return new ApplicationHttpRequest(getRequest(depth - 1), null, false);
+    }
+
+
+    @Test
+    public void testGetAttributeNested() {
+        int[] depths = { 0, 1, 4, 7 };
+        for (int depth : depths) {
+            HttpServletRequest httpRequest = getRequest(depth);
+
+            // Warm-up
+            doTestGetAttribute(httpRequest);
+
+            long start = System.nanoTime();
+            doTestGetAttribute(httpRequest);
+            long duration = System.nanoTime() - start;
+
+            System.out.println("Depth " + depth + ": " + duration + "ns");
+        }
+    }
 }


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to