6323374: (coll) Optimize Collections.unmodifiable* and synchronized*

Reviewed-by: redestad, smarks, darcy

Author: Ian Graves

src/java.base/share/classes/java/util/Collections.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2021, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -1008,12 +1008,17 @@ public static int lastIndexOfSubList(List<?> source, List<?> target) {
      * The returned collection will be serializable if the specified collection
      * is serializable.
      *
+     * @implNote This method may return its argument if the argument is already unmodifiable.
      * @param  <T> the class of the objects in the collection
      * @param  c the collection for which an unmodifiable view is to be
      *         returned.
      * @return an unmodifiable view of the specified collection.
      */
+    @SuppressWarnings("unchecked")
     public static <T> Collection<T> unmodifiableCollection(Collection<? extends T> c) {
+        if (c.getClass() == UnmodifiableCollection.class) {
+            return (Collection<T>) c;
+        }
         return new UnmodifiableCollection<>(c);
     }
 
@@ -1116,11 +1121,17 @@ public Stream<E> parallelStream() {
      * The returned set will be serializable if the specified set
      * is serializable.
      *
+     * @implNote This method may return its argument if the argument is already unmodifiable.
      * @param  <T> the class of the objects in the set
      * @param  s the set for which an unmodifiable view is to be returned.
      * @return an unmodifiable view of the specified set.
      */
+    @SuppressWarnings("unchecked")
     public static <T> Set<T> unmodifiableSet(Set<? extends T> s) {
+        // Not checking for subclasses because of heap pollution and information leakage.
+        if (s.getClass() == UnmodifiableSet.class) {
+            return (Set<T>) s;
+        }
         return new UnmodifiableSet<>(s);
     }
 
@@ -1148,12 +1159,17 @@ static class UnmodifiableSet<E> extends UnmodifiableCollection<E>
      * The returned sorted set will be serializable if the specified sorted set
      * is serializable.
      *
+     * @implNote This method may return its argument if the argument is already unmodifiable.
      * @param  <T> the class of the objects in the set
      * @param s the sorted set for which an unmodifiable view is to be
      *        returned.
      * @return an unmodifiable view of the specified sorted set.
      */
     public static <T> SortedSet<T> unmodifiableSortedSet(SortedSet<T> s) {
+        // Not checking for subclasses because of heap pollution and information leakage.
+        if (s.getClass() == UnmodifiableSortedSet.class) {
+            return s;
+        }
         return new UnmodifiableSortedSet<>(s);
     }
 
@@ -1197,13 +1213,17 @@ public SortedSet<E> tailSet(E fromElement) {
      * The returned navigable set will be serializable if the specified
      * navigable set is serializable.
      *
+     * @implNote This method may return its argument if the argument is already unmodifiable.
      * @param  <T> the class of the objects in the set
      * @param s the navigable set for which an unmodifiable view is to be
      *        returned
      * @return an unmodifiable view of the specified navigable set
      * @since 1.8
      */
     public static <T> NavigableSet<T> unmodifiableNavigableSet(NavigableSet<T> s) {
+        if (s.getClass() == UnmodifiableNavigableSet.class) {
+            return s;
+        }
         return new UnmodifiableNavigableSet<>(s);
     }
 
@@ -1289,11 +1309,17 @@ public NavigableSet<E> tailSet(E fromElement, boolean inclusive) {
      * is serializable. Similarly, the returned list will implement
      * {@link RandomAccess} if the specified list does.
      *
+     * @implNote This method may return its argument if the argument is already unmodifiable.
      * @param  <T> the class of the objects in the list
      * @param  list the list for which an unmodifiable view is to be returned.
      * @return an unmodifiable view of the specified list.
      */
+    @SuppressWarnings("unchecked")
     public static <T> List<T> unmodifiableList(List<? extends T> list) {
+        if (list.getClass() == UnmodifiableList.class || list.getClass() == UnmodifiableRandomAccessList.class) {
+           return (List<T>) list;
+        }
+
         return (list instanceof RandomAccess ?
                 new UnmodifiableRandomAccessList<>(list) :
                 new UnmodifiableList<>(list));
@@ -1438,12 +1464,18 @@ private Object writeReplace() {
      * The returned map will be serializable if the specified map
      * is serializable.
      *
+     * @implNote This method may return its argument if the argument is already unmodifiable.
      * @param <K> the class of the map keys
      * @param <V> the class of the map values
      * @param  m the map for which an unmodifiable view is to be returned.
      * @return an unmodifiable view of the specified map.
      */
+    @SuppressWarnings("unchecked")
     public static <K,V> Map<K,V> unmodifiableMap(Map<? extends K, ? extends V> m) {
+        // Not checking for subclasses because of heap pollution and information leakage.
+        if (m.getClass() == UnmodifiableMap.class) {
+            return (Map<K,V>) m;
+        }
         return new UnmodifiableMap<>(m);
     }
 
@@ -1795,13 +1827,19 @@ public boolean equals(Object o) {
      * The returned sorted map will be serializable if the specified sorted map
      * is serializable.
      *
+     * @implNote This method may return its argument if the argument is already unmodifiable.
      * @param <K> the class of the map keys
      * @param <V> the class of the map values
      * @param m the sorted map for which an unmodifiable view is to be
      *        returned.
      * @return an unmodifiable view of the specified sorted map.
      */
+    @SuppressWarnings("unchecked")
     public static <K,V> SortedMap<K,V> unmodifiableSortedMap(SortedMap<K, ? extends V> m) {
+        // Not checking for subclasses because of heap pollution and information leakage.
+        if (m.getClass() == UnmodifiableSortedMap.class) {
+            return (SortedMap<K,V>) m;
+        }
         return new UnmodifiableSortedMap<>(m);
     }
 
@@ -1840,14 +1878,19 @@ public SortedMap<K,V> tailMap(K fromKey)
      * The returned navigable map will be serializable if the specified
      * navigable map is serializable.
      *
+     * @implNote This method may return its argument if the argument is already unmodifiable.
      * @param <K> the class of the map keys
      * @param <V> the class of the map values
      * @param m the navigable map for which an unmodifiable view is to be
      *        returned
      * @return an unmodifiable view of the specified navigable map
      * @since 1.8
      */
+    @SuppressWarnings("unchecked")
     public static <K,V> NavigableMap<K,V> unmodifiableNavigableMap(NavigableMap<K, ? extends V> m) {
+        if (m.getClass() == UnmodifiableNavigableMap.class) {
+            return (NavigableMap<K,V>) m;
+        }
         return new UnmodifiableNavigableMap<>(m);
     }
 

test/jdk/java/util/Collections/WrappedUnmodifiableCollections.java

@@ -0,0 +1,102 @@
+/*
+ * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 6323374
+ * @run testng WrappedUnmodifiableCollections
+ */
+
+import java.util.*;
+import java.util.function.Function;
+import org.testng.annotations.Test;
+import static org.testng.Assert.*;
+
+
+@Test
+public class WrappedUnmodifiableCollections {
+
+    private static <T,E extends T> void testWrapping(T collection, Function<T,E> wrapper) {
+        var collection1 = wrapper.apply(collection);
+        var collection2 = wrapper.apply(collection1);
+        assertNotSame(collection, collection2);
+        assertSame(collection1, collection2);
+    }
+
+    public void testUnmodifiableListsDontWrap() {
+        List<List<?>> lists = List.of(List.of(), List.of(1,2,3), List.of(1),
+                List.of(1,2,3,4,5,6),
+                List.of(1,2,3).subList(0,1),
+                new LinkedList<>(List.of(1,2,3)),
+                new ArrayList<>(List.of(1,2,3)));
+
+        for(List<?> list : lists) {
+            testWrapping(list, Collections::unmodifiableList);
+        }
+    }
+
+    public void testUnmodifiableCollectionsDontWrap() {
+        Collection<?> list = List.of();
+        testWrapping(list, Collections::unmodifiableCollection);
+    }
+
+    public void testUnmodifiableSetsDontWrap() {
+
+        List<Set<?>> sets = List.of(new TreeSet<>(),
+                                    Set.of(1, 2),
+                                    Set.of(1,2,3,4,5,6));
+
+        for (Set<?> set : sets) {
+            testWrapping(set, Collections::unmodifiableSet);
+        }
+
+        TreeSet<?> treeSet = new TreeSet<>();
+
+        //Collections.UnmodifiableSortedSet
+        testWrapping((SortedSet<?>) treeSet, Collections::unmodifiableSortedSet);
+
+        //Collections.UnmodifiableNavigableSet
+        testWrapping((NavigableSet<?>) treeSet, Collections::unmodifiableNavigableSet);
+
+    }
+
+    public void testUnmodifiableMapsDontWrap() {
+        TreeMap<?,?> treeMap = new TreeMap<>();
+
+        List<Map<?,?>> maps = List.of(treeMap,
+                Map.of(1,1),
+                Map.of(1, 1, 2, 2, 3, 3, 4, 4));
+
+        for (Map<?,?> map : maps) {
+            testWrapping(map, Collections::unmodifiableMap);
+        }
+
+        //Collections.UnModifiableSortedMap
+        testWrapping((SortedMap<?,?>) treeMap, Collections::unmodifiableSortedMap);
+
+        //Collections.UnModifiableNavigableMap
+        testWrapping((NavigableMap<?,?>) treeMap, Collections::unmodifiableNavigableMap);
+
+    }
+
+}