On Fri, 25 Mar 2022 14:52:09 GMT, Roger Riggs <rri...@openjdk.org> wrote:
>> Done. >> >> Sadly it seems the smaller improvement I got on >> `getYearFromMillisZoneRegion/-UTC` was due avoiding the added arithmetic in >> `Instant.ofEpochSecond(sec, nanos)`: >> >> Benchmark Mode Cnt Score Error >> Units >> GetYearBench.getYearFromMillisZoneOffset thrpt 15 27.579 ? 0.030 >> ops/ms >> GetYearBench.getYearFromMillisZoneRegion thrpt 15 9.570 ? 0.091 >> ops/ms >> GetYearBench.getYearFromMillisZoneRegionUTC thrpt 15 28.063 ? 0.030 >> ops/ms >> >> Benchmark Mode Cnt Score Error >> Units >> GetYearBench.getYearFromMillisZoneOffset thrpt 15 34.791 ? 0.030 >> ops/ms >> GetYearBench.getYearFromMillisZoneRegion thrpt 15 9.526 ? 0.122 >> ops/ms >> GetYearBench.getYearFromMillisZoneRegionUTC thrpt 15 28.056 ? 0.040 >> ops/ms >> >> >> `getYearFromMillisZoneOffset` is still good. > > I would expect that `nanoAdjustment` is zero in most cases, would it hurt > performance to test for zero and skip the math? Actually I think it might be fairly common with a `nanoAdjustment` (e.g. timestamps with milliseconds), so not sure such a test is profitable. But I think it was correct to omit the nano parts for the `ZonedDateTime` constructor, since it's validating that the `nanoOfSecond` parameter is in the range 0-999999999. I'll revert the change to add the 2nd parameter to the new, internal getOffset method. ------------- PR: https://git.openjdk.java.net/jdk/pull/7957
