This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch 10.1.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/10.1.x by this push: new 4201defbea Fix BZ 68089 - performance improvement 4201defbea is described below commit 4201defbeac20ad4147ea73db5905fe072d39433 Author: Mark Thomas <ma...@apache.org> AuthorDate: Tue Nov 7 16:31:19 2023 +0000 Fix BZ 68089 - performance improvement Use an appropriate collection rather than a linear array scan https://bz.apache.org/bugzilla/show_bug.cgi?id=68089 --- .../catalina/core/ApplicationHttpRequest.java | 40 +++++++++--------- .../apache/catalina/core/ApplicationRequest.java | 22 ++++++---- .../TestApplicationHttpRequestPerformance.java | 47 ++++++++++++++++++++++ webapps/docs/changelog.xml | 5 +++ 4 files changed, 88 insertions(+), 26 deletions(-) diff --git a/java/org/apache/catalina/core/ApplicationHttpRequest.java b/java/org/apache/catalina/core/ApplicationHttpRequest.java index db94ccd411..3eba826b3f 100644 --- a/java/org/apache/catalina/core/ApplicationHttpRequest.java +++ b/java/org/apache/catalina/core/ApplicationHttpRequest.java @@ -24,6 +24,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.Enumeration; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.NoSuchElementException; @@ -80,6 +81,17 @@ class ApplicationHttpRequest extends HttpServletRequestWrapper { RequestDispatcher.FORWARD_REQUEST_URI, RequestDispatcher.FORWARD_CONTEXT_PATH, RequestDispatcher.FORWARD_SERVLET_PATH, RequestDispatcher.FORWARD_PATH_INFO, RequestDispatcher.FORWARD_QUERY_STRING, RequestDispatcher.FORWARD_MAPPING }; + /* + * This duplicates specials to some extent but has been added to improve the performance of [get|set|is]Special(). + * It may be possible to remove specials but that will require changes to AttributeNamesEnumerator. + */ + private static final Map<String,Integer> specialsMap = new HashMap<>(); + static { + for (int i = 0; i < specials.length; i++) { + specialsMap.put(specials[i], Integer.valueOf(i)); + } + } + private static final int SPECIALS_FIRST_FORWARD_INDEX = 6; @@ -726,13 +738,7 @@ class ApplicationHttpRequest extends HttpServletRequestWrapper { * @param name Attribute name to be tested */ protected boolean isSpecial(String name) { - - for (String special : specials) { - if (special.equals(name)) { - return true; - } - } - return false; + return specialsMap.containsKey(name); } @@ -742,12 +748,11 @@ class ApplicationHttpRequest extends HttpServletRequestWrapper { * @return the special attribute pos, or -1 if it is not a special attribute */ protected int getSpecial(String name) { - for (int i = 0; i < specials.length; i++) { - if (specials[i].equals(name)) { - return i; - } + Integer index = specialsMap.get(name); + if (index == null) { + return -1; } - return -1; + return index.intValue(); } @@ -757,13 +762,12 @@ class ApplicationHttpRequest extends HttpServletRequestWrapper { * @return true if the attribute was a special attribute, false otherwise */ protected boolean setSpecial(String name, Object value) { - for (int i = 0; i < specials.length; i++) { - if (specials[i].equals(name)) { - specialAttributes[i] = value; - return true; - } + Integer index = specialsMap.get(name); + if (index == null) { + return false; } - return false; + specialAttributes[index.intValue()] = value; + return true; } diff --git a/java/org/apache/catalina/core/ApplicationRequest.java b/java/org/apache/catalina/core/ApplicationRequest.java index e0347353ee..8e0ec4576d 100644 --- a/java/org/apache/catalina/core/ApplicationRequest.java +++ b/java/org/apache/catalina/core/ApplicationRequest.java @@ -16,9 +16,12 @@ */ package org.apache.catalina.core; +import java.util.Arrays; import java.util.Collections; import java.util.Enumeration; import java.util.HashMap; +import java.util.HashSet; +import java.util.Set; import jakarta.servlet.RequestDispatcher; import jakarta.servlet.ServletRequest; @@ -38,10 +41,12 @@ import jakarta.servlet.ServletRequestWrapper; */ class ApplicationRequest extends ServletRequestWrapper { - /** * The set of attribute names that are special for request dispatchers. + * + * @deprecated Will be removed without replacement in Tomcat 11 onwards. */ + @Deprecated protected static final String specials[] = { RequestDispatcher.INCLUDE_REQUEST_URI, RequestDispatcher.INCLUDE_CONTEXT_PATH, RequestDispatcher.INCLUDE_SERVLET_PATH, RequestDispatcher.INCLUDE_PATH_INFO, @@ -49,6 +54,10 @@ class ApplicationRequest extends ServletRequestWrapper { RequestDispatcher.FORWARD_REQUEST_URI, RequestDispatcher.FORWARD_CONTEXT_PATH, RequestDispatcher.FORWARD_SERVLET_PATH, RequestDispatcher.FORWARD_PATH_INFO, RequestDispatcher.FORWARD_QUERY_STRING, RequestDispatcher.FORWARD_MAPPING }; + /* + * This duplicates specials but has been added to improve the performance of isSpecial(). + */ + private static final Set<String> specialsSet = new HashSet<>(Arrays.asList(specials)); /** @@ -56,7 +65,6 @@ class ApplicationRequest extends ServletRequestWrapper { */ protected final HashMap<String,Object> attributes = new HashMap<>(); - /** * Construct a new wrapped request around the specified servlet request. * @@ -157,13 +165,11 @@ class ApplicationRequest extends ServletRequestWrapper { * Is this attribute name one of the special ones that is added only for included servlets? * * @param name Attribute name to be tested + * + * @deprecated Will be removed without replacement in Tomcat 11 onwards. */ + @Deprecated protected boolean isSpecial(String name) { - for (String special : specials) { - if (special.equals(name)) { - return true; - } - } - return false; + return specialsSet.contains(name); } } diff --git a/test/org/apache/catalina/core/TestApplicationHttpRequestPerformance.java b/test/org/apache/catalina/core/TestApplicationHttpRequestPerformance.java new file mode 100644 index 0000000000..a0a1556a8e --- /dev/null +++ b/test/org/apache/catalina/core/TestApplicationHttpRequestPerformance.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 org.apache.catalina.core; + +import org.junit.Test; + +import org.apache.catalina.connector.Request; + +public class TestApplicationHttpRequestPerformance { + + @Test + public void testGetAttribute() { + org.apache.coyote.Request coyoteRequest = new org.apache.coyote.Request(); + Request request = new Request(null, coyoteRequest); + ApplicationHttpRequest applicationHttpRequest = new ApplicationHttpRequest(request, null ,false); + + // Warm-up + doTestGetAttribute(applicationHttpRequest); + + long start = System.nanoTime(); + doTestGetAttribute(applicationHttpRequest); + long duration = System.nanoTime() - start; + + System.out.println(duration + "ns"); + } + + + private void doTestGetAttribute(ApplicationHttpRequest request) { + for (int i = 0; i < 100000000; i++) { + request.getAttribute("Unknown"); + } + } +} diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index b3a972a0ea..6131f6bf49 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -140,6 +140,11 @@ <bug>68054</bug>: Avoid some file canonicalization calls introduced by the fix for <bug>65433</bug>. (remm) </fix> + <fix> + <bug>68089</bug>: Improve performance of request attribute access for + <code>ApplicationHttpRequest</code> and <code>ApplicationRequest</code>. + (markt) + </fix> </changelog> </subsection> <subsection name="Coyote"> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org