Avoid list copy on reverse iteration (#12297)

This reverts/modified parts of commit 115ee1c. Co-authored-by: Simone Bordet <[email protected]> --------- Co-authored-by: Simone Bordet <[email protected]>
jetty · Sep 24, 2024 · 18b9782 · 18b9782
1 parent 1d9f108
commit 18b9782
Show file tree

Hide file tree

Showing 6 changed files with 45 additions and 29 deletions.
diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java
@@ -653,11 +653,11 @@ protected void exitScope(Request request, Context lastContext, ClassLoader lastL
      */
     protected void notifyExitScope(Request request)
     {
-        for (ContextScopeListener listener : TypeUtil.reverse(_contextListeners))
+        for (ListIterator<ContextScopeListener> i = TypeUtil.listIteratorAtEnd(_contextListeners); i.hasPrevious();)
         {
             try
             {
-                listener.exitScope(_context, request);
+                i.previous().exitScope(_context, request);
             }
             catch (Throwable e)
             {

diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/TypeUtil.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/TypeUtil.java
@@ -34,6 +34,7 @@
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
+import java.util.ListIterator;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.ServiceConfigurationError;
@@ -174,18 +175,29 @@ public static <T> List<T> asList(T[] a)
     }
 
     /**
-     * <p>Returns a new list with the elements of the specified list in reverse order.</p>
-     * <p>The specified list is not modified, differently from {@link Collections#reverse(List)}.</p>
-     *
-     * @param list the list whose elements are to be reversed
-     * @return a new list with the elements in reverse order
+     * <p>Returns a {@link ListIterator} positioned at the last item in a list.</p>
+     * @param list the list
      * @param <T> the element type
+     * @return A {@link ListIterator} positioned at the last item of the list.
      */
-    public static <T> List<T> reverse(List<T> list)
+    public static <T> ListIterator<T> listIteratorAtEnd(List<T> list)
     {
-        List<T> result = new ArrayList<>(list);
-        Collections.reverse(result);
-        return result;
+        try
+        {
+            int size = list.size();
+            if (size == 0)
+                return Collections.emptyListIterator();
+            return list.listIterator(size);
+        }
+        catch (IndexOutOfBoundsException e)
+        {
+            // list was concurrently modified, so do this the hard way
+            ListIterator<T> i = list.listIterator();
+            while (i.hasNext())
+                i.next();
+
+            return i;
+        }
     }
 
     /**

diff --git a/...etty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextHandler.java b/...etty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextHandler.java
@@ -31,6 +31,7 @@
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
+import java.util.ListIterator;
 import java.util.Locale;
 import java.util.Map;
 import java.util.Objects;
@@ -519,11 +520,11 @@ public void contextDestroyed() throws Exception
                     //Call context listeners
                     Throwable multiException = null;
                     ServletContextEvent event = new ServletContextEvent(getServletContext());
-                    for (ServletContextListener listener : TypeUtil.reverse(_destroyServletContextListeners))
+                    for (ListIterator<ServletContextListener> i = TypeUtil.listIteratorAtEnd(_destroyServletContextListeners); i.hasPrevious();)
                     {
                         try
                         {
-                            callContextDestroyed(listener, event);
+                            callContextDestroyed(i.previous(), event);
                         }
                         catch (Exception x)
                         {
@@ -574,17 +575,17 @@ protected void requestDestroyed(Request baseRequest, HttpServletRequest request)
         if (!_servletRequestListeners.isEmpty())
         {
             final ServletRequestEvent sre = new ServletRequestEvent(getServletContext(), request);
-            for (ServletRequestListener listener : TypeUtil.reverse(_servletRequestListeners))
+            for (ListIterator<ServletRequestListener> i = TypeUtil.listIteratorAtEnd(_servletRequestListeners); i.hasPrevious();)
             {
-                listener.requestDestroyed(sre);
+                i.previous().requestDestroyed(sre);
             }
         }
 
         if (!_servletRequestAttributeListeners.isEmpty())
         {
-            for (ServletRequestAttributeListener listener : TypeUtil.reverse(_servletRequestAttributeListeners))
+            for (ListIterator<ServletRequestAttributeListener> i = TypeUtil.listIteratorAtEnd(_servletRequestAttributeListeners); i.hasPrevious();)
             {
-                scopedRequest.removeEventListener(listener);
+                scopedRequest.removeEventListener(i.previous());
             }
         }
     }
@@ -1223,11 +1224,11 @@ protected void notifyExitScope(Request request)
         ServletContextRequest scopedRequest = Request.as(request, ServletContextRequest.class);
         if (!_contextListeners.isEmpty())
         {
-            for (ServletContextScopeListener listener : TypeUtil.reverse(_contextListeners))
+            for (ListIterator<ServletContextScopeListener> i = TypeUtil.listIteratorAtEnd(_contextListeners); i.hasPrevious(); )
             {
                 try
                 {
-                    listener.exitScope(getContext(), scopedRequest);
+                    i.previous().exitScope(getContext(), scopedRequest);
                 }
                 catch (Throwable e)
                 {

diff --git a/...-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/SessionHandler.java b/...-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/SessionHandler.java
@@ -19,6 +19,7 @@
 import java.util.EventListener;
 import java.util.Iterator;
 import java.util.List;
+import java.util.ListIterator;
 import java.util.Locale;
 import java.util.Map;
 import java.util.Set;
@@ -570,9 +571,9 @@ public void onSessionDestroyed(Session session)
         getSessionContext().run(() ->
         {
             HttpSessionEvent event = new HttpSessionEvent(session.getApi());
-            for (HttpSessionListener  listener : TypeUtil.reverse(_sessionListeners))
+            for (ListIterator<HttpSessionListener> i = TypeUtil.listIteratorAtEnd(_sessionListeners); i.hasPrevious();)
             {
-                listener.sessionDestroyed(event);
+                i.previous().sessionDestroyed(event);
             }
         });
     }

diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ContextHandler.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ContextHandler.java
@@ -29,6 +29,7 @@
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
+import java.util.ListIterator;
 import java.util.Locale;
 import java.util.Map;
 import java.util.Objects;
@@ -1000,17 +1001,17 @@ protected void requestDestroyed(Request baseRequest, HttpServletRequest request)
         if (!_servletRequestListeners.isEmpty())
         {
             final ServletRequestEvent sre = new ServletRequestEvent(_apiContext, request);
-            for (ServletRequestListener listener : TypeUtil.reverse(_servletRequestListeners))
+            for (ListIterator<ServletRequestListener> i = TypeUtil.listIteratorAtEnd(_servletRequestListeners); i.hasPrevious();)
             {
-                listener.requestDestroyed(sre);
+                i.previous().requestDestroyed(sre);
             }
         }
 
         if (!_servletRequestAttributeListeners.isEmpty())
         {
-            for (ServletRequestAttributeListener listener : TypeUtil.reverse(_servletRequestAttributeListeners))
+            for (ListIterator<ServletRequestAttributeListener> i = TypeUtil.listIteratorAtEnd(_servletRequestAttributeListeners); i.hasPrevious();)
             {
-                baseRequest.removeEventListener(listener);
+                baseRequest.removeEventListener(i.previous());
             }
         }
     }
@@ -1070,11 +1071,11 @@ protected void exitScope(Request request)
     {
         if (!_contextListeners.isEmpty())
         {
-            for (ContextScopeListener listener : TypeUtil.reverse(_contextListeners))
+            for (ListIterator<ContextScopeListener> i = TypeUtil.listIteratorAtEnd(_contextListeners); i.hasPrevious();)
             {
                 try
                 {
-                    listener.exitScope(_apiContext, request);
+                    i.previous().exitScope(_apiContext, request);
                 }
                 catch (Throwable e)
                 {

diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/SessionHandler.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/SessionHandler.java
@@ -20,6 +20,7 @@
 import java.util.Enumeration;
 import java.util.EventListener;
 import java.util.List;
+import java.util.ListIterator;
 import java.util.Set;
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.function.Function;
@@ -835,9 +836,9 @@ public void onSessionDestroyed(Session session)
             Runnable r = () ->
             {
                 HttpSessionEvent event = new HttpSessionEvent(session.getApi());
-                for (HttpSessionListener listener : TypeUtil.reverse(_sessionListeners))
+                for (ListIterator<HttpSessionListener> i = TypeUtil.listIteratorAtEnd(_sessionListeners); i.hasPrevious();)
                 {
-                    listener.sessionDestroyed(event);
+                    i.previous().sessionDestroyed(event);
                 }
             };
             _contextHandler.getCoreContextHandler().getContext().run(r);