[threeten-dev] Some observations of chrono ser

[Stephen Colebourne]

I have no requirement to allow the chronologies to be overridden by
the config file, only for new ones to be added. In fact, allowing
overriding looks like an unecessary risk.

I also have no requirement for the Ser class to be used in the chrono
package, however using it would undoubtably result in smaller
serialized sizes.

Stephen



On 15 March 2013 01:07, Roger Riggs <Roger.Riggs at oracle.com> wrote:
> Hi Sherman,
>
> Sorry for the race condition, I was trying out a few things...
>
>
> On 3/14/13 3:33 PM, Xueming Shen wrote:
>>
>> On 03/14/2013 10:42 AM, roger riggs wrote:
>>>
>>> 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.
>>>
>>
>> Something like the attached webrev?
>>
>> http://cr.openjdk.java.net/~sherman/jdk8_threeten/serChrono
>
> Yes, same as what I did.
>
>>
>> Do we really to make it "final" to enforce the abstract base class
>> Chronology to
>> always handle the serialization? Or be liberal to document that this is
>> encouraged,
>> but not enforced, with a "non-final" Chronology.writeReplace()
>
>
> I went with non-final because as final it would prevent subclassing a
> Chronology
> that had its own state to serialize, like the HijrahChronology that has
> variants.
> It seems inflexible to prevent that from anything but bundled Chronologies.
>
>>
>> Second question leads to my (2) in previous email. With the enforcement of
>> Chronology.writeReplace()
>>     ->Ser
>>         ->Chronology.readExternal()
>>             ->Chronology.of/of0(id)
>>                 ->cache.
>>
>> A Chronology instance may be serialized from a vm default impl to a
>> user-defined
>> Chronology in other vm, if there is overriding Chronology implementation,
>> however,
>> a built-in ChronoLocalDate, for example JapaneseDate, is always hard-wired
>> to the
>> built-in JapaneseChronology, so if you serialize and de-serialize a
>> JapaneseChronology
>> AND a JapaneseDate into other vm, you may get a user-defined
>> JapaneseChronology
>> and a built-in JapneseDate with bundled with a built-in
>> JapaneseChronology, something
>> undesired?
>
> I suppose there is little benefit from allowing the built-in chronologies to
> be overridden and it does open up some potential problems.
> To insure consistency, JapaneseDate could use Chronology.of("Japanese") to
> get
> the Chronology but that adds very little and will be slower.
>
> If we do away with the override then there should be at least a log
> message if the register of the ServiceLoader chronology fails.
>
> Also, note that the synchronization on whether initialization is complete
> is based on finding ISOChronology in the HashTable.  That would need
> a different mechanism. (Its ok for multiple threads to do the init
> concurrently
> only one will win and it avoids a single threaded synchronization.)
>
> Roger
>
>>
>> -Sherman
>>
>>
>