Hi Claes,
Thanks for filing JDK-8209003
(https://bugs.openjdk.java.net/browse/JDK-8209003)! I'll look into
archiving the immutable singletons in ImmutableCollections.
Thanks again!!
Jiangli
On 8/4/18 9:22 AM, Claes Redestad wrote:
Hi,
Jiangli and I discussed a subtle issue with the archiving changes here
last week (she's currently on vacation, and I was at JVMLS this week,
so it was never brought to the list, sorry!)
Basically: the empty configuration uses things like
Collections.emptySet(), which means it will end up deserializing an
object from the archive that doesn't (necessarily) have the same
identity as the object retrieved from direct uses of
Collection.emptySet().
Same goes for the Set.of()/Map.of()/List.of() singletons which might
now already exist at runtime in two versions since the change to
archive ModuleDescriptors. Oops!
This glitch is probably fine semantically for most intents and
purposes, but could lead to some unnecessary inefficiencies and
theoretically real surprises/bugs, so we probably need to ensure that
all singletons we serialize retain the property that they are
identical to the object a caller of the respective getter would receive.
I think this can be deferred to a must-fix follow-up, mainly since
there's some benefit in first cleaning up a few things here first:
- currently we have uses of both Collections.emptyFoo() and Foo.of()
in the ModuleDescriptor/Configuration code
- to be correct, we thus need to archive the Collections.emptyFoo()
singletons *and* the various java.util.ImmutableCollections ones
- if we first consolidated all uses of Collections.emptyFoo() to
Foo.of() (in java.lang.module), we'd only need to archive the
immutable singletons in java.util.ImmutableCollections
Consolidating to Foo.of() was actually already being considered
anyway, so I'll go ahead with a separate RFE..
Thanks!
/Claes
On 2018-07-20 20:31, Jiangli Zhou wrote:
Many thanks to Alan and Claes for discussions and contributions to
this change!
Webrev: http://cr.openjdk.java.net/~jiangli/8207263/webrev.00/
