On Sat, 20 Sep 2025 20:36:43 GMT, Pavel Rappo <[email protected]> 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
