Collections.synchronizedXXX() and internal mutex (aka SyncRoot)

[Stuart Marks]

On 5/1/20 2:02 AM, dmytro sheyko wrote:
> The checked, synchronized and unmodifiable wrappers in some cases store backing 
> collection in more than one fields.
> 
> Thus `UnmodifiableList<E>` has
> 1. its own field `List<? extends E> list` and
> 2. `Collection<? extends E> c`, which it inherits from `UnmodifiableCollection<E>`.
> 
> Also `UnmodifiableNavigableSet<E>` has even 3 duplicated fields:
> 1. its own `NavigableSet<E> ns`,
> 2. `SortedSet<E> ss` from `UnmodifiableSortedSet<E>` and
> 3. `Collection<? extends E> c` from `UnmodifiableCollection<E>`.
> 
> Isn't it worth to get rid of such duplication? (at least for unmodifiable 
> collections). This may affect serialization, but I believe it's still possible 
> preserve serialization backward compatible, if it's necessary.
> Or is it done intentionally?

Interesting. I went through some of the history here, in particular the 
core-libs-dev review threads of JDK-7129185 [1] which was the last time 
significant work was done on the wrappers. There was no mention of the duplicate 
references in any of the reviews. I suspect the wrappers introduced in this 
changeset (e.g., UnmodifiableNavigableSet) copied the style from existing 
wrappers, which also used this style.

I was able to look through the old (non open source) history of the JDK, and I 
found that this style of having a redundant field in a wrapper subtype was 
introduced in JDK 1.2 in 1998 along with the original collections implementation.

I suspect it was done this way for convenience. Certainly the wrapper subclasses 
have access to the field from the superclass. But to use it they there would 
have to be a cast at every call site that wanted to use a subclass method. This 
would certainly make the wrapper code more verbose, and it might even slow 
things down a bit with checkcast bytecodes and such.

While trying to save space is laudable, compatibility with existing serial forms 
needs to be preserved. Doing this would require adding serialPersistentFields 
arrays and readObject() and writeObject() methods to every one of the wrapper 
classes. This is fairly tedious and error-prone. I'm not sure it's worth it.

s'marks


[1] https://bugs.openjdk.java.net/browse/JDK-7129185