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

[Stuart Marks]

BTW Daniel, if you're ok with my suggestions, go ahead and push them. I don't 
need to see another webrev.

Thanks,

s'marks

On 12/2/15 2: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.
>
> 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);
>
> 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
>>