Re: RFR: 8068958: Timestamp.from(Instant) should throw when conversion is not possible [v2]

2023-12-22 Thread Raffaello Giulietti
On Fri, 22 Dec 2023 22:55:09 GMT, Eamonn McManus wrote: >> Multiplying with `*` never produces `ArithmeticException`, so the catch in >> the existing code is never triggered. `Math.multiplyExact` does produce >> `ArithmeticException` if the multiplication overflows. So we can use that, >> and

Re: RFR: 8068958: Timestamp.from(Instant) should throw when conversion is not possible [v2]

2023-12-22 Thread Eamonn McManus
> Multiplying with `*` never produces `ArithmeticException`, so the catch in > the existing code is never triggered. `Math.multiplyExact` does produce > `ArithmeticException` if the multiplication overflows. So we can use that, > and rethrow `IllegalArgumentException` as the specification says.

Re: RFR: 8068958: Timestamp.from(Instant) should throw when conversion is not possible [v2]

2023-12-22 Thread Eamonn McManus
On Fri, 22 Dec 2023 09:07:31 GMT, Raffaello Giulietti wrote: >> Eamonn McManus has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Address review comments about the new test. > > test/jdk/java/sql/testng/test/sql/TimestampTests.java line

Re: RFR: 8322647: Short name for the `Europe/Lisbon` time zone is incorrect

2023-12-22 Thread Joe Wang
On Fri, 22 Dec 2023 21:36:32 GMT, Naoto Sato wrote: >> make/jdk/src/classes/build/tools/cldrconverter/CLDRConverter.java line 2: >> >>> 1: /* >>> 2: * Copyright (c) 2012, 2024, Oracle and/or its affiliates. All rights >>> reserved. >> >> 2024 already? ;-) > > Thanks, Joe. Not planning to

Re: RFR: 8322647: Short name for the `Europe/Lisbon` time zone is incorrect

2023-12-22 Thread Naoto Sato
On Fri, 22 Dec 2023 20:19:17 GMT, Joe Wang wrote: >> This is a regression caused by the fix to >> [JDK-8317979](https://bugs.openjdk.org/browse/JDK-8317979), where the >> parsing of TZDB files was incorrect. With this fix, `TimeZoneNames_en.java` >> which is generated during the build time

Re: RFR: 8322647: Short name for the `Europe/Lisbon` time zone is incorrect

2023-12-22 Thread Iris Clark
On Fri, 22 Dec 2023 18:59:20 GMT, Naoto Sato wrote: > This is a regression caused by the fix to > [JDK-8317979](https://bugs.openjdk.org/browse/JDK-8317979), where the parsing > of TZDB files was incorrect. With this fix, `TimeZoneNames_en.java` which is > generated during the build time has

Re: RFR: 8322647: Short name for the `Europe/Lisbon` time zone is incorrect

2023-12-22 Thread Joe Wang
On Fri, 22 Dec 2023 18:59:20 GMT, Naoto Sato wrote: > This is a regression caused by the fix to > [JDK-8317979](https://bugs.openjdk.org/browse/JDK-8317979), where the parsing > of TZDB files was incorrect. With this fix, `TimeZoneNames_en.java` which is > generated during the build time has

RFR: 8322647: Short name for the `Europe/Lisbon` time zone is incorrect

2023-12-22 Thread Naoto Sato
This is a regression caused by the fix to [JDK-8317979](https://bugs.openjdk.org/browse/JDK-8317979), where the parsing of TZDB files was incorrect. With this fix, `TimeZoneNames_en.java` which is generated during the build time has the following diffs from the previous (incorrect) one: ---

Re: RFR: 8322322: Support archived full module graph when -Xbootclasspath/a is used [v2]

2023-12-22 Thread Calvin Cheung
On Fri, 22 Dec 2023 08:40:08 GMT, Alan Bateman wrote: >> Calvin Cheung has updated the pull request incrementally with one additional >> commit since the last revision: >> >> comments from Alan and Ioi > > src/java.base/share/classes/jdk/internal/loader/ClassLoaders.java line 71: > >> 69:

Re: RFR: 8322322: Support archived full module graph when -Xbootclasspath/a is used [v2]

2023-12-22 Thread Calvin Cheung
> Please review this change for enabling full module graph even when > -Xbootclasspath/a is specified. The validation of -Xbootclasspath/a is > already being done in `FileMapInfo::validate_boot_class_paths()`. The full > module graph will be disabled if there's a mismatch in -Xbootclasspath/a

Integrated: 8321802: (zipfs) Add validation of incorrect LOC signature in ZipFileSystem

2023-12-22 Thread Eirik Bjørsnøs
On Mon, 11 Dec 2023 15:38:28 GMT, Eirik Bjørsnøs wrote: > Please review this PR which adds validation of incorrect LOC signatures in > `ZipFileSystem`. > > `ZipFile` already rejects the case where the offset pointed to from the CEN > header does not start with the expected LOC signature. It

Re: RFR: 8320971: Use BufferedInputStream.buf directly when param of implTransferTo() is trusted [v17]

2023-12-22 Thread Markus KARG
On Fri, 22 Dec 2023 14:37:26 GMT, Vladimir Sitnikov wrote: >> I think there is a misundertanding. This PR is not intendend to reduce the >> *amount* of allocated heap, it is about sparing time by not creating >> *temporary copies*. The latter should be rather easy to check: Invoke >>

Re: RFR: 8320971: Use BufferedInputStream.buf directly when param of implTransferTo() is trusted [v17]

2023-12-22 Thread Vladimir Sitnikov
On Fri, 22 Dec 2023 14:27:16 GMT, Markus KARG wrote: >> Can anybody give a hint how one can create assertions in OpenJDK test code >> that would check the amount of allocated heap for the tested method? >> >> Since the change here is "removal of an allocation", the assert in the code >>

Re: RFR: 8316662: Remove one allocation per conversion in Double.toString(double) and Float.toString(float) [v3]

2023-12-22 Thread Raffaello Giulietti
On Fri, 27 Oct 2023 12:10:07 GMT, Raffaello Giulietti wrote: >> By correctly sizing an intermediate `byte[]` and making use of the internal >> `newStringNoRepl()` method, one allocation per conversion can be avoided >> when the runtime uses compact strings. > > Raffaello Giulietti has updated

Re: RFR: 8320971: Use BufferedInputStream.buf directly when param of implTransferTo() is trusted [v17]

2023-12-22 Thread Markus KARG
On Fri, 22 Dec 2023 13:53:42 GMT, Vladimir Sitnikov wrote: >> test/jdk/java/io/BufferedInputStream/TransferToTrusted.java line 68: >> >>> 66: >>> 67: var bis = new BufferedInputStream(new >>> ByteArrayInputStream(dup)); >>> 68: bis.mark(dup.length); >> >> If you take a

Re: RFR: 8320971: Use BufferedInputStream.buf directly when param of implTransferTo() is trusted [v17]

2023-12-22 Thread Vladimir Sitnikov
On Fri, 22 Dec 2023 13:19:46 GMT, Markus KARG wrote: >> Sergey Tsypanov has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains 22 additional >>

Re: RFR: 8320971: Use BufferedInputStream.buf directly when param of implTransferTo() is trusted [v17]

2023-12-22 Thread Markus KARG
On Fri, 22 Dec 2023 09:55:15 GMT, Sergey Tsypanov wrote: >> It looks like we can skip copying of `byte[]` in >> `BufferedInputStream.implTransferTo()` for `OutputStreams` residing in >> `java.io`. >> >> See comment by @vlsi in >>

Re: RFR: 8322292: Rearrange comparison of fields in Record.equals() [v5]

2023-12-22 Thread Sergey Tsypanov
On Thu, 21 Dec 2023 19:43:50 GMT, ExE Boss wrote: >> src/java.base/share/classes/java/lang/runtime/ObjectMethods.java line 224: >> >>> 222: var rt2 = mh2.type().returnType(); >>> 223: return Integer.compare( >>> 224: rt1.isPrimitive() || rt1.isEnum() ||

Re: RFR: 8322292: Rearrange comparison of fields in Record.equals() [v6]

2023-12-22 Thread Sergey Tsypanov
> Currently if we create a record it's fields are compared in their declaration > order. This might be ineffective in cases when two objects have "heavy" > fields equals to each other, but different "lightweight" fields (heavy and > lightweight in terms of comparison) e.g. primitives, enums, >

Re: RFR: 8320971: Use BufferedInputStream.buf directly when param of implTransferTo() is trusted [v17]

2023-12-22 Thread Sergey Tsypanov
> It looks like we can skip copying of `byte[]` in > `BufferedInputStream.implTransferTo()` for `OutputStreams` residing in > `java.io`. > > See comment by @vlsi in > https://github.com/openjdk/jdk/pull/10525/files#diff-e19c508d1bb6ee78697ecca66947c395adda0d9c49a85bf696e677ecbd977af1R612

Re: RFR: 8320971: Use BufferedInputStream.buf directly when param of implTransferTo() is trusted [v16]

2023-12-22 Thread Sergey Tsypanov
On Thu, 21 Dec 2023 17:29:16 GMT, Brian Burkhalter wrote: >> Sergey Tsypanov has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains 20 additional >>

Re: RFR: 8068958: Timestamp.from(Instant) should throw when conversion is not possible

2023-12-22 Thread Raffaello Giulietti
On Thu, 21 Dec 2023 21:51:10 GMT, Eamonn McManus wrote: > Multiplying with `*` never produces `ArithmeticException`, so the catch in > the existing code is never triggered. `Math.multiplyExact` does produce > `ArithmeticException` if the multiplication overflows. So we can use that, > and

Re: RFR: 8322322: Support archived full module graph when -Xbootclasspath/a is used

2023-12-22 Thread Alan Bateman
On Thu, 21 Dec 2023 19:10:59 GMT, Calvin Cheung wrote: > Please review this change for enabling full module graph even when > -Xbootclasspath/a is specified. The validation of -Xbootclasspath/a is > already being done in `FileMapInfo::validate_boot_class_paths()`. The full > module graph will