java.time.ZoneId.systemDefalut() overhead

[Peter Levart]

On 02/23/2015 12:24 PM, Daniel Fuchs wrote:
> Hi Peter,
>
> On 23/02/15 11:29, Peter Levart wrote:
>> Caching of ZoneId in the defensive clone of defualt TimeZone object
>> would not have a desired effect as the clone is thrown away in each call
>> to ZoneId.systemDefault(). We must get hold of the TimeZone instance
>> that is cached. Another way to do that would be to take the route of
>> reflection objects (Field, Method, Constructor) and put a pointer to
>> 'root' TimeZone instance in the clones, so it would be accessible
>> through the clone.
>
> Ah I see. You would have to initialize the zoneId field in the
> root object, i.e. in setDefaultZone - and possibly in
> TimeZone.setDefault as well. So that might not be as good an
> idea after all...

I think the logic stays more or less the same, just one dereference more 
('root' if not null) is needed to get to the TimeZone instance that 
caches ZoneId. I would still initialize it lazily.

>
>>> Finally, I also wonder whether a better idea would be to add
>>> a variant of dosToJavaTime/javaToDosTime that would take an
>>> additional zoneId as parameter.
>>
>> For ZipEntry it would not make much sense as there's only one time value
>> that has to be converted only once in each ZipEntry.
>
> If I'm not mistaken dosToJavaTime is only called from ZipFile
> and ZipInputStream, and javaToDosTime from ZipOutputStream.

It is as of now, but it will not be any more (see Claes' RFR(S): 
8073497: Lazy conversion of ZipEntry time) that is just being reviewed.

>
>>> You probably don't want to support changing the time zone in
>>> the middle of a Zip. Do you?
>>
>> Ok, in that case the ZoneId would have to be attached to ZipFile and
>> passed to each ZipEntry. I agree that per ZipFile default zone atomicity
>> is a desired property. Speeding up ZoneId.systemDefault() retrieval is
>> also generally desirable, don't you think?

In case we go with LocalDateTime (most probably), to support atomicity 
for the whole ZipFile/ZipInput(Output)Stream, we actually need to get 
hold of ZoneRules object - not ZoneId. With dynamic ZoneRulesProvider(s) 
the rules can still change dynamically underneath the same ZoneId 
instance...

>
> Agreed. I'm just not a big fan of using SharedSecrets if that can
> be helped. I just wanted to see if there was other alternatives to
> explore.

There is this 'root' pointer alternative. We still do a clone 
unnecessarily in this case, but the allocation will hopefully be 
optimized away as the methods are in-lined and analyzed for clone's escape.

Regards, Peter

>
> best regards,
>
> -- daniel
>
>>
>