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

[Stuart Marks]

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
>