RFR: 8368178: Add specialization of SequencedCollection methods to emptyList, singletonList and nCopies
Pavel Rappo
prappo at openjdk.org
Sun Sep 21 14:51:12 UTC 2025
On Sat, 20 Sep 2025 20:36:43 GMT, Pavel Rappo <prappo at openjdk.org> wrote:
>> Please review this small change. If you have more ideas which classes may miss specializations of SequencedCollection methods, I can add them to this PR as well.
>
>> If you have more ideas which classes may miss specializations of SequencedCollection methods, I can add them to this PR as well.
>
> Have you considered something like this?
>
> diff --git a/src/java.base/share/classes/java/util/ImmutableCollections.java b/src/java.base/share/classes/java/util/ImmutableCollections.java
> index 38cc45122a2..d8c3499a958 100644
> --- a/src/java.base/share/classes/java/util/ImmutableCollections.java
> +++ b/src/java.base/share/classes/java/util/ImmutableCollections.java
> @@ -352,7 +352,7 @@ public boolean contains(Object o) {
>
> @Override
> public List<E> reversed() {
> - return ReverseOrderListView.of(this, false);
> + return size() < 2 ? this : ReverseOrderListView.of(this, false);
> }
>
> IndexOutOfBoundsException outOfBounds(int index) {
> @@ -636,6 +636,11 @@ public int lastIndexOf(Object o) {
> }
> }
>
> + @Override
> + public List<E> reversed() {
> + return size() == 1 ? this : super.reversed();
> + }
> +
> @java.io.Serial
> private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundException {
> throw new InvalidObjectException("not serial proxy");
> @pavelrappo re `ImmutableCollections`, I was thinking about them. One thing that bugs me is that `List.copyOf(immutableList)` doesn't copy, but `List.copyOf(immutableList.reversed())` produces a fresh copy. Probably this is not a very common scenario to optimize, though.
>
> Re `List12.reversed()` with size == 2: probably it's better to create a fresh list like `new List12<>(e1, e0)`, like
>
> ```java
> @SuppressWarnings("unchecked")
> @Override
> public List<E> reversed() {
> return e1 == EMPTY ? this : new List12<>((E) e1, e0);
> }
> ```
>
> This way, we have fewer indirections and always return the instance of `List12`, which may probably help with devirtualization sometimes. The `List12` instance size is small. With compressed OOPs it's likely the same size as `ReverseOrderListView` (two object fields, compared to object+boolean fields in `ReverseOrderListView`), and the original list could become eligible for GC. One downside of this approach is that `listOf2.reversed().reversed()` will return a fresh instance, rather than the original one. But I guess it's a rare scenario and the cost is still small. What do you think?
> One more concern is that `ReverseOrderListView` is not serializable, but `List12` and `ListN` are serializable. With this change, some reversed lists will be serializable while others are not. I don't see anything in the specification regarding the serializability of the reversed lists, but such a discrepancy might be confusing for some users.
---
Your comments are good and thought-through. When dealing with classes like that, it's time to bring a microscope and be concerned with as many aspects as possible.
As for the first comment, List.of and List.copyOf do not specify the class whose instance they return, or even if and how that class overrides `reversed()`. As such, I think this `@implSpec` leaves us with no choice but to abide by the roundtrip property (i.e. `list.reversed().reversed() == list`):
> **Implementation Requirements:**
>
> The implementation in this interface returns a reverse-ordered List view. The `reversed()` method of the view returns a reference to this List.
As for serializability, it also bugs be. Unmodifiable lists are designed to be consistent and strict. Even though the spec clearly says that it's a bad idea to serialize an unkown list, if a list is serializable some of the times but not others, it might hide bugs:
> In cases where the serializability of a collection is not specified, there is no guarantee about the serializability of such collections. In particular, many [view collections](https://docs.oracle.com/en/java/javase/25/docs/api/java.base/java/util/Collection.html#view) are not serializable, even if the original collection is serializable.
> default List<E> reversed()
> Returns a reverse-ordered view of this collection.
I defer this serializability issue to @stuart-marks.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/27406#issuecomment-3316042278
More information about the core-libs-dev
mailing list