[threeten-dev] Some observations of chrono ser

[roger riggs]

Hi,

On 3/14/2013 11:40 AM, Xueming Shen wrote:
> 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?
The intent was that Chronology would always handle serialization
and return the same (singleton) instances that are known to Chronology.

To make that work the writeReplace method needs to be "protected final"
and visible to subclasses and not be override-able.

>
> (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?

Roger

>
> -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
>>
>

-- 
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