[threeten-dev] Some observations of chrono ser

[Xueming Shen]

On 3/14/13 7:29 AM, roger riggs wrote:
> Hi  Sherman,
>
> On 3/12/2013 5:52 PM, Xueming Shen wrote:
>> Hi,
>>
>> (1) Chronology.writeReplace() is private, so the proxy Ser mechanism
>> is actually disabled for all Chronology subclasses. Is it 
>> intentional? Given
>> all JDK subclasses all have readResolve() defined (3 out of 4 to return
>> the singleton) I would assume the Ser delegation might not be necessary,
>> though it might be still desirable to "enforce" the Ser pattern here?
> Not quite, but still broken.  The writeReplace method can be private but
> is used only if the Chronology class is Serializable.
> http://docs.oracle.com/javase/7/docs/platform/serialization/spec/output.html#5324
>
> The subclasses are serializable and that is triggering the default 
> serialization.
>
> Is there an issue for this?

I think a private writeReplace method don't have an effect on how its 
serializable
subclass serializes itself. So I believe the existing 4 jdk Chronology 
subclasses
are using the "jdk default mechanism" (not the Ser delegation/proxy), so 
I was
asking if it is an intentional design/implementation. For any other 
potential
user defined Chronology subclass, does the abstract Chronology have anything
really need to be "serializable" and via the Ser mechanism? Two possible 
designs
here

(1) we want all the Chronology subclasses to be serialized via "Ser" 
(then it can
be done via the Chronology.of(...) when read in), then the 
writeReplace() probably
should be declared as non-private?

(2) if we don't want to enforce the "Ser" serialization for all its 
subclasses, then
the writeReplace() may not need to be defined at all in Chronology?

-Sherman
>>
>> (2) Chronology.initCache() tries to load the Chronology implementation
>> from the ServiceLoader before initializing/registering the built-ins, 
>> is it
>> intentional? If yes, the current implementation as described in (1) may
>> have an un-desired impact to this design intention. For example, if a
>> Japanese Chronology in a VM with built-in ja chronology is transferred
>> to a VM that has a "customized" Japanese Chronology registered, the
>> current implementation may force to use the built-in (as the Ser 
>> mechanism
>> is off/out now), the Chronology.of(id) logic in 
>> Chronology.readExternal()
>> will never kick in and the "registered/customized" Japanese chronology
>> will not be used for the deser-ed chronology.
> It was intentional that the ServiceLoader defined Chronologies would
> supercede the built-in ones allowing replacement if as part of 
> customization.
> It may deserve more discussion.
>
>>
>> (3) ChronoDateImpl also declares "Serializable", given all its 
>> subclasses have
>> their own declaration of Serializable() and the 
>> writeReplace()/write/read
>> External(), this might not be necessary?
> It does seem unnecessary duplication.
>
> Roger
>
>>
>> -Sherman
>
> -- 
> Thanks, Roger
>
> Oracle Java Platform Group
>
> Green Oracle <http://www.oracle.com/commitment> Oracle is committed to 
> developing practices and products that help protect the environment
>