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