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