My original thinking was that it not so much that 3rd party code doesn't care about nanoseconds it just that the code was built before the concept of nano time was added to LogRecords.
The use cases for LogRecord.getMillis are: 1.g - Formatters for rendering the event time. 2.g - Filters for throttling bursts of events. 3.g - Comparators for ordering events along with getting the sequence for tie breaking. 4.g - Consumer bridges of the java.util.logging framework. The use cases for LogRecord.setMillis are: 1.s - Unit tests. 2.s - Logging framework bridges. 3.s - Type conversions of LogRecords to another subclass of LogRecord (GlassFish) Note that even the logging framework itself doesn't call setMillis because the time is set in the LogRecord constructor. All of the uses of setMillis are closer to the producer of a log record and the uses of getMillis are closer to the consumer of a log record. Ideally if we are choose to lose precision it shouldn't impact other consumers down stream. The reason to keep setMillis deprecated is that calling this method makes the choice to lose precision ahead of other code that was interested in that extra precision. Out of the list of getMillis callers, 4.g is about the only reason to deprecate getMillis (assuming the bridge has support for nanos). Everyone else is choosing their fate without choosing the fate of others. Under the current proposal, I would have to suppress or add "some fine hackery" to unit tests which seems like a fair trade off since that keeps the code smell out of the released production code. Jason ________________________________________ From: Stuart Marks <stuart.ma...@oracle.com> Sent: Tuesday, December 1, 2015 7:13 PM To: Daniel Fuchs Cc: Jason Mehrens; Core-Libs-Dev Subject: Re: Deprecation of LogRecord.getMillis in JDK9 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 >> >
