Deprecation of LogRecord.getMillis in JDK9

[Stuart Marks]

I think #3 or a variation is best. Clearly, #1 is inconsistent and simply 
documenting it (#2) isn't much better.

I'd recommend making setInstant() be more explicit about the range of Instant 
values that are allowed, namely those created from Instant.ofEpochMilli(long), 
which allows +/- 292 million years from the epoch. Otherwise the reader is 
forced to try to understand when Instant.toEpochMilli() throws the exception.

Now that we've constrained the range of instants that a LogRecord can contain, 
is it still necessary for setMillis(long) to be deprecated? Every value is 
valid. It can set (almost) the full range of Instant values, just not with 
nanosecond resolution. If you don't care about nanosecond resolution, this seems 
like a perfectly fine method to use.

s'marks

On 12/1/15 10:48 AM, Daniel Fuchs wrote:
> Hi Jason, Stuart,
>
> Here is a potential fix for the issue:
>
> http://cr.openjdk.java.net/~dfuchs/webrev_8144262/webrev.00/src/java.logging/share/classes/java/util/logging/LogRecord.java.frames.html
>
>
> http://cr.openjdk.java.net/~dfuchs/webrev_8144262/specdiff-logging/java/util/logging/LogRecord.html
>
>
>
> As Stuart noted, java.time.Instant has a greater range than what can
> be constructed from a long milliseconds-since-epoch + a nano-time
> adjustment. This does not apply to instants returned by the system
> clock, since those are constructed precisely from such long
> milliseconds-since-epoch + nano-time adjustment.
>
> However - someone could conceivably construct such an Instant
> and pass it to a LogRecord. If that happens, then LogRecord.getMillis()
> could potentially throw an undocumented ArithmeticException.
>
> So we have at least 3 possibilities:
>
> 1. do nothing
> 2. document that getMillis() can throw ArithmeticException, with the
>     additional consequence that serializing a LogRecord thus constructed
>     would also throw an ArithmeticException.
> 3. modify setInstant() to validate that the instant will fit in a
>     long milliseconds-since-epoch.
>
>
> The above patch implements option 3 (which currently has my
> preference). Is that the best solution?
>
> I would very much like to hear your opinion.
> If it seems like the best then I'll add a unit test, send an RFR, and
> do the paper work for the spec change...
>
> best regards, and thanks for all the valuable feedback!
>
> -- daniel
>
>
>
> On 30/11/15 18:04, Jason Mehrens wrote:
>> Hi Daniel,
>>
>>
>> When JDK-8072645 - java.util.logging should use java.time to get more precise
>> time stamps was commited the LogRecord.getMillis() method was marked as
>> deprecated with the reason "To get the full nanosecond resolution event time,
>> use getInstant".  I can see marking LogRecord.setMillis as deprecated since
>> using that would be an untended loss of precision.  However, it seems
>> excessive to deprecate LogRecord.getMillis when it could be treated as a
>> convenience method that could simply note that if the caller wants nanosecond
>> resolution use getInstant.  It would be extremely helpful compatibility wise
>> to have this undeprecated for libs that have support pre-Java 9.  If it can't
>> be undeprecated what is the proper way to target support as low as JDK7 but
>> might end up executing on JDK9?
>>
>>
>> Thanks,
>>
>>
>> Jason
>>
>