RFR: 8072645: java.util.logging should use java.time to get more precise time stamps

[Daniel Fuchs]

Hi Peter,

On 2/14/15 10:36 AM, Peter Levart wrote:
> Hi Daniel,
>
> The "millis" property in your proposal just changes one part of 
> LogRecord's notion of time (it doesn't change/reset the nanoAdjustment 
> part). From compatibility standpoint, your ptoposal is changing the 
> semantics of "millis" property. Previously "millis" was the sole 
> property of record's notion of time, now it is only a component of it. 
> Consider this legacy code:
>
> LogRecord r = new LogRecord(...); // timestamp1
> ...
> r.setMillis(System.currentTimeMillis()); // timestamp2
>
> What is the record's notion of time now? It is composed of "millis" 
> from timestamp2 and "nanoAdjustment" from timestamp1. Not something 
> that one would want, right?

Excellent observation. I had missed that.
> So what should be done instead is:
> - deprecate "millis" writable property and document that it sets the 
> record's time with the resolution of milliseconds and point to new 
> "instantUTC" property as a replacement.
> - add "instantUTC" writable property and document it as "the" method 
> to be used to set record's time with full resolution
> - optionally add read-only "nanoAdjustment" property (I don't think it 
> is needed, since users should start using new time API instead of 
> mangling with millis and nanos themselves)
>
> This lends itself to also change internal storage of LogRecord's time. 
> You could just replace the "millis" field with a reference to 
> "instantUTC". Serialization would have to be adjusted a bit (using 
> serialPersistentFields) to stay compatible.
>
> What do you think?

setMillis should definitely set the whole time for backward compatibility
reasons. That will make it more than a simple setter, and therefore 
deprecating
it is probably the best thing to do as the method name could then become
misleading (setTimeAsMillis() would be the expected name for such a
behaviour).

That lets me think that there shouldn't be a setNanoAdjustment() either.
getMillis() and getNanoAdjustment() are OK and can stay, since they
are consistent - are needed for compatibility reasons (at least 
getMillis is),
and will help with describing the serial form.

As you noted, the only correct way to set the LogRecord time is to do it 
in a
single method:
We could have either setInstant() or setTime(long millis, int nanos) - 
setInstant
being most probably the best alternative - since we already have a 
getInstant(),
and splitting the time into millis + nanos is a bit strange anyway - since
java.time favors seconds + nanos.

For serialization - I think we will need to keep serializing a number of 
milliseconds
and a nano second adjustment. Whether the time stamp should be stored 
internally
as an Instant or as a number of milliseconds + a nano adjustment can be 
discussed.

I might favor the second as it would make serialization easier 
(especially for
documenting the serial form).

Would you agree with that?

best regards,

-- daniel

>
> Regards, Peter
>
> On 02/13/2015 03:56 PM, Daniel Fuchs wrote:
>> Hi,
>>
>> Please find below a patch for:
>>
>> 8072645: java.util.logging should use java.time to get more
>>          precise time stamps
>>
>>
>> http://cr.openjdk.java.net/~dfuchs/webrev_8072645/webrev.00/
>>
>> specdiff:
>> http://cr.openjdk.java.net/~dfuchs/webrev_8072645/specdiff-logging-time/java/util/logging/package-summary.html 
>>
>>
>> Overview:
>> ---------
>>
>> The patch is made of the following pieces:
>>
>>  - LogRecord uses java.time.Clock's systemClock to get an
>>    Instant in the best available resolution.
>>
>>    The instant is split into a number of milliseconds (a long)
>>    and a nanosecond adjustment (an int).
>>    The number of milliseconds is the same than what would have
>>    been obtained by calling System.currentTimeMillis().
>>
>>  - LogRecord acquires a new serializable int nanoAdjustement field,
>>    which can be used together with the number of milliseconds
>>    to reconstruct the instant.
>>
>>  - SimpleFormatter is updated to pass a ZoneDateTime
>>    instance to String.format, instead of a Date.
>>
>>    The effect of that is that the format string can now
>>    be configure to print the full instant precision, if
>>    needed.
>>
>>  - XMLformatter will add a new <nanos> element after the
>>    <millis> element - if the value of the nanoAdjustment
>>    field is not 0.
>>
>>    The <date> string will also contain the nano second
>>    adjustment as well as the zone offset as formatted by
>>    DateTimeFormatter.ISO_OFFSET_DATE_TIME
>>
>> Compatibility considerations:
>> -----------------------------
>>
>> - The serial for of log record is backward/forward compatible.
>>   I added a test to verify that.
>>
>> - XMLFormatter has acquired a new configurable property
>>   '<FQCN>.printNanos' which allows to revert to the old
>>   XML format, should the new format cause issues in
>>   existing applications.
>>
>> - The logger.dtd will need to be updated, to support the
>>   new optional <nanos> element. And for this matter,
>>   should we update the logger.dtd or rather define a
>>   logger-v2.dtd?
>>   See planned modification:
>>
>> <http://cr.openjdk.java.net/~dfuchs/webrev_8072645/logger-dtd/logger.dtd.frames.html> 
>>
>>
>> best regards,
>>
>> -- daniel
>