RFR 8144262: LogRecord.getMillis() method is a convenience API that should not have been deprecated

[Daniel Fuchs]

Hi Stuart,

On 12/2/15 11:49 PM, Stuart Marks wrote:
> Hi Daniel,
>
> I'm ok with setMillis() remaining deprecated, but there should be an 
> explanation of why it's deprecated. What's there now says
>
>     To set event time with nanosecond resolution,
>     use {@link #setInstant(java.time.Instant)}.
>
> A better explanation might be along the lines of:
>
>     LogRecord maintains timestamps with nanosecond resolution, using
>     Instant values. For this reason, setInstant() should be used in
>     preference to setMillis().
>
> Wordsmith to taste.

Thanks for this suggestion. It does read better indeed.

>
> The explanation of exceptions in setInstant() is good.
>
> In the LogRecordWithNanosAPI.java test, I was confused by the Instant 
> calculations. I think the last Instant value for which toEpochMilli() 
> won't throw is
>
>     Instant max = 
> Instant.ofEpochMilli(Long.MAX_VALUE).plusNanos(999_999L);
>
> and the earliest Instant value for which toEpochMilli() will throw is
>
>     Instant toobig = max.plusNanos(1L);

Oh - yes that's a much more readable way to create these instant values :-)
I'll take that!

Thanks for the feedback.

-- daniel

>
> s'marks
>
>
>
> On 12/2/15 12:10 PM, Daniel Fuchs wrote:
>> Hi,
>>
>> Please find below a fix for
>> 8144262: LogRecord.getMillis() method is a convenience API that
>>           should not have been deprecated
>> https://bugs.openjdk.java.net/browse/JDK-8144262
>>
>>
>> webrev:
>> http://cr.openjdk.java.net/~dfuchs/webrev_8144262/webrev.00
>>
>> specdiff:
>> http://cr.openjdk.java.net/~dfuchs/webrev_8144262/specdiff-logging/java/util/logging/LogRecord.html 
>>
>>
>>
>>
>> When 8072645:java.util.logging should use java.time to get more
>>       precise time stamps
>> was implemented we decided to deprecate LogRecord.getMillis()
>> and LogRecord.setMillis() in favor of the new LogRecord.getInstant()
>> and LogRecord.setInstant().
>>
>> This may have been a bit hasty as LogRecord.getMillis() can in fact
>> be seen as a convenience method - a shortcut to
>> LogRecord.getInstant().toEpochMillis().
>> The only method we really wanted to deprecate was LogRecord.setMillis()
>> as that would truncate the instant to milliseconds:
>> in other words, LogRecord.setMillis(LogRecord.getMillis()) was no longer
>> idempotent.
>>
>> This changes proposes to restore LogRecord.getMillis() to its previous
>> status, and also fixes LogRecord.setInstant to reject instant values
>> which would not fit in a long milliseconds-since-epoch - which would
>> have caused serialization to fail and LogRecord.getMillis() to throw
>> an undocumented ArithmeticException.
>>
>> Rationale about this proposed change have also been discussed in this
>> thread:
>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-November/037039.html 
>>
>>
>> best regards,
>>
>> -- daniel
>>