RFR: 8368178: Add specialization of SequencedCollection methods to standard list factories [v3]

Stuart Marks smarks at openjdk.org
Wed Sep 24 23:38:17 UTC 2025


On Tue, 23 Sep 2025 07:25:04 GMT, Tagir F. Valeev <tvaleev 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.
>
> Tagir F. Valeev has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - Add SequencedCollection specialization for immutable collections
>  - Fix indentation

OK, List12::getFirst and List12::getLast seem fine.

But yeah... AbstractImmutableList::reversed and List12::reversed are indeed debatable. I wrestled over this for a while. I don't think they're incorrect, but I'm also having trouble convincing myself they're cases worth optimizing. There are also weird tradeoffs.

With no override, calling reversed() on a List12 returns a ReverseOrderListView(orig) and reversed() on that returns orig again. But with the override, calling reversed() gives a new List12, and calling reversed() on that gives another new List12. Nothing here is really terrible, but it's also not clear to me this is an improvement.

With AbstractImmutableList I got kind of tangled up in analyzing where this would be used. It seems to me the optimization would take effect for the zero-size nulls-disallowed case, the zero-size nulls-allowed case, and the size-one nulls-allowed case. (The size-one nulls-disallowed case is covered by List12, unless we don't add the override there....) I don't think this is incorrect for any case, but I'm also having a hard time seeing the improvement. It might save the allocation of a small object, or it might save an indirection. And ... that's about it?

The reversed() overrides hardly seem worthwhile. Maybe don't include them.

Regarding the tests, is it necessary to add new test files for EmptyList and SingletonList? Maybe it's possible to add cases to test/jdk/java/util/Collection/MOAT.java. I'm also a bit surprised to see a new provider added to the ListFactories test. But I haven't looked too closely. If it's necessary, then of course we should include it.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/27406#issuecomment-3331019666


More information about the core-libs-dev mailing list