RFR: 8266571: Sequenced Collections [v8]

[Nir Lisker]

On Fri, 21 Apr 2023 22:14:58 GMT, Stuart Marks <smarks at openjdk.org> wrote:

>> src/java.base/share/classes/java/util/SequencedMap.java line 152:
>> 
>>> 150:         var it = entrySet().iterator();
>>> 151:         return it.hasNext() ? Map.Entry.copyOf(it.next()) : null;
>>> 152:     }
>> 
>> Would is be better to first check `Map.isEmpty()` and only then obtain the iterator? That would eliminate also the `hasNext` check.
>> 
>> default Map.Entry<K,V> firstEntry() {
>>     return isEmpty() ? null : Map.Entry.copyOf(entrySet().iterator().next());
>> }
>> 
>> 
>> Same question for the other methods.
>
> Might not work if this map is concurrent. If isEmpty() returns false but then the map is emptied before the subsequent calls, next() might throw NSEE. The concurrent maps' iterators make sure to cache the element if hasNext() has returned true.

Then shouldn't `ConcurrentNavigableMap`, the only `ConcurrentMap` and `SequencedMap` supertype (there is no `ConcurrentSortedMap`), override its superinterface implementation with this thread-safe one?

I think that the question boils down to: should `SequencedMap` take into account the needs of its subtypes and give a conservative default implementation, or should it give a "minimal" implementation and let the subtypes override it according to their needs? I would choose the latter, both because `SequencedMap` specifies no guarantees on thread-safety (atomicity and synchronization), and because `Map`'s default implementation behave this way  - `compute`, `merge`... tell concurrent implementations how to override them.

Now if someone implements a `SequencedMap` that is also a `ConcurrentMap`, but is not a `NavigableMap`, say `ConcurrentLinkedHashMap`, they will override the `SequencedMap` defaults like they would the `Map` defaults.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/7387#discussion_r1174327323