On Thu, 23 Jan 2025 17:41:35 GMT, Naoto Sato <na...@openjdk.org> wrote:
>> Justin Lu has updated the pull request incrementally with one additional >> commit since the last revision: >> >> reflect Naoto's review > > src/java.base/share/classes/java/util/TimeZone.java line 652: > >> 650: * >> 651: * @param rawOffset the given time zone GMT offset in milliseconds. >> 652: * @return an array of IDs, where the time zone for that ID has > > array -> stream Thanks for the catch. Fixed this and bumped those Zone* file copyright years in https://github.com/openjdk/jdk/pull/23251/commits/48956fdd92aba3a3802e81cfbc8468296dbab777. I double checked that all the test files have 2025 copyright years. > src/java.base/share/classes/sun/util/calendar/ZoneInfoFile.java line 96: > >> 94: // specify it. Keep the same behavior for better >> 95: // compatibility. >> 96: String[] list = ids.toArray(new String[0]); > > Why is this change? Since the array has correct size, no extra array > allocation is needed? Regarding the switch from `toArray(new T[size])` to `toArray(new T[0])`, based off https://shipilev.net/blog/2016/arrays-wisdom-ancients/#_conclusion, the JVM performs the latter faster. I believe the article data/results still hold true now. > src/java.base/share/classes/sun/util/calendar/ZoneInfoFile.java line 122: > >> 120: return zoneIds() >> 121: .filter(id -> getZoneInfo(id).getRawOffset() == >> rawOffset) >> 122: .sorted(); // Sort the IDs, see getZoneIds(int) > > This is interesting. one with `rawOffset` is soreted, but the other is not. > It is inconsistent but probably we should align as you did here. Noticed that too. There is also the argument that since these stream methods are new, we can leave it un-sorted but we would have to specify the deviation. Although I agree it is best to align for consistency. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/23251#discussion_r1927539888 PR Review Comment: https://git.openjdk.org/jdk/pull/23251#discussion_r1927539958 PR Review Comment: https://git.openjdk.org/jdk/pull/23251#discussion_r1927540008
