Integrated: 8269559: AArch64: Implement string_compare intrinsic in SVE
On Mon, 16 Aug 2021 20:59:55 GMT, TatWai Chong wrote: > This patch implements string_compare intrinsic in SVE. > It supports all LL, LU, UL and UU comparisons. > > As we haven't found an existing benchmark to measure performance impact, > we created a benchmark derived from the test [1] for this evaluation. > This benchmark is attached to this patch. > > Besides, remove the unused temporary register `vtmp3` from the existing > match rules for StrCmp. > > The result below shows all varients can be benefited largely. > Command: make exploded-test TEST="micro:StringCompareToDifferentLength" > > Benchmark(size) Mode Cnt Score Speedup Units > compareToLL 24 avgt 10 1.0x ms/op > compareToLL 36 avgt 10 1.0x ms/op > compareToLL 72 avgt 10 1.0x ms/op > compareToLL 128 avgt 10 1.4x ms/op > compareToLL 256 avgt 10 1.8x ms/op > compareToLL 512 avgt 10 2.7x ms/op > compareToLU 24 avgt 10 1.6x ms/op > compareToLU 36 avgt 10 1.8x ms/op > compareToLU 72 avgt 10 2.3x ms/op > compareToLU 128 avgt 10 3.8x ms/op > compareToLU 256 avgt 10 4.7x ms/op > compareToLU 512 avgt 10 6.3x ms/op > compareToUL 24 avgt 10 1.6x ms/op > compareToUL 36 avgt 10 1.7x ms/op > compareToUL 72 avgt 10 2.2x ms/op > compareToUL 128 avgt 10 3.3x ms/op > compareToUL 256 avgt 10 4.4x ms/op > compareToUL 512 avgt 10 6.1x ms/op > compareToUU 24 avgt 10 1.0x ms/op > compareToUU 36 avgt 10 1.0x ms/op > compareToUU 72 avgt 10 1.4x ms/op > compareToUU 128 avgt 10 2.2x ms/op > compareToUU 256 avgt 10 2.6x ms/op > compareToUU 512 avgt 10 3.7x ms/op > > [1] > https://github.com/openjdk/jdk/blob/master/test/hotspot/jtreg/compiler/intrinsics/string/TestStringCompareToDifferentLength.java This pull request has now been integrated. Changeset: 8b1b6f9f Author:TatWai Chong Committer: Nick Gasson URL: https://git.openjdk.java.net/jdk/commit/8b1b6f9fb375bbc2de339ad8f526ca4d5f83dc70 Stats: 517 lines in 11 files changed: 421 ins; 1 del; 95 mod 8269559: AArch64: Implement string_compare intrinsic in SVE Reviewed-by: ngasson, aph - PR: https://git.openjdk.java.net/jdk/pull/5129
Re: RFR: 8273111: Default timezone should return zone ID if /etc/localtime is valid but not canonicalization on linux [v4]
> Hi, > Please help me review the change to enhance getting time zone ID from > /etc/localtime on linux. > > We use `realpath` instead of `readlink` to obtain the link name of > /etc/localtime, because `readlink` can only read the value of a symbolic of > link, not the canonicalized absolute pathname. > > For example, the value of /etc/localtime is > "../usr/share/zoneinfo//Asia/Shanghai", then the linkbuf obtained by > `readlink` is "../usr/share/zoneinfo//Asia/Shanghai", and then the call of > `getZoneName(linkbuf)` will get "/Asia/Shanghai", not "Asia/Shanghai", which > consider as invalid in `ZoneInfoFile.getZoneInfo()`. Using `realpath`, you > can get “/usr/share/zoneinfo/Asia/Shanghai“ directly from “/etc/localtime“. > > Thanks, > wuyan Wu Yan 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 four additional commits since the last revision: - Merge branch 'master' into timezone - change functions to be static - replace realpath - 8273111: Default timezone should return zone ID if /etc/localtime is valid but not canonicalization on linux - Changes: - all: https://git.openjdk.java.net/jdk/pull/5327/files - new: https://git.openjdk.java.net/jdk/pull/5327/files/38177cd0..93cff3d1 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5327=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5327=02-03 Stats: 1449026 lines in 13982 files changed: 735454 ins; 659127 del; 54445 mod Patch: https://git.openjdk.java.net/jdk/pull/5327.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5327/head:pull/5327 PR: https://git.openjdk.java.net/jdk/pull/5327
Re: RFR: 8271820: Implementation of JEP 416: Reimplement Core Reflection with Method Handle [v13]
On Wed, 13 Oct 2021 23:49:19 GMT, Maurizio Cimadamore
wrote:
>> Mandy Chung has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Minor cleanup. Improve javadoc in CallerSensitiveAdapter
>
> src/java.base/share/classes/jdk/internal/reflect/DirectMethodHandleAccessor.java
> line 58:
>
>> 56: * Creates a MethodAccessorImpl for a caller-sensitive method.
>> 57: */
>> 58: static MethodAccessorImpl callerSensitiveMethodAccessor(Method
>> method, MethodHandle dmh) {
>
> This method and the one above are identical - they just call `new
> DirectMethodHandleAccessor` with same parameters. Is the distinction between
> these two factories still relevant? (besides the different asserts)
Good catch! It no longer needs this distinction in this new version. Will
remove it.
-
PR: https://git.openjdk.java.net/jdk/pull/5027
Re: RFR: 8153490: Cannot setBytes() if incoming buffer's length is bigger than number of elements we want to insert. [v4]
On Wed, 15 Sep 2021 17:57:35 GMT, Lance Andersen wrote:
>> Mitsuru Kariya has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Modify javadoc for consistency
>
> src/java.sql.rowset/share/classes/javax/sql/rowset/serial/SerialBlob.java
> line 337:
>
>> 335: * @param bytes the array of bytes to be written to the {@code BLOB}
>> 336: * value
>> 337: * @param offset the offset into the array {@code bytes} at which
>
> Please change all occurrences of `{@code bytes}` to `{@code byte}s` as this
> was caught as part of the CSR review.
Sorry for my very slow response.
These `{@code bytes}` point to the `bytes` argument, but should I change it to
`{@code byte}s`?
-
PR: https://git.openjdk.java.net/jdk/pull/4001
Re: RFR: 8269559: AArch64: Implement string_compare intrinsic in SVE [v4]
> This patch implements string_compare intrinsic in SVE. > It supports all LL, LU, UL and UU comparisons. > > As we haven't found an existing benchmark to measure performance impact, > we created a benchmark derived from the test [1] for this evaluation. > This benchmark is attached to this patch. > > Besides, remove the unused temporary register `vtmp3` from the existing > match rules for StrCmp. > > The result below shows all varients can be benefited largely. > Command: make exploded-test TEST="micro:StringCompareToDifferentLength" > > Benchmark(size) Mode Cnt Score Speedup Units > compareToLL 24 avgt 10 1.0x ms/op > compareToLL 36 avgt 10 1.0x ms/op > compareToLL 72 avgt 10 1.0x ms/op > compareToLL 128 avgt 10 1.4x ms/op > compareToLL 256 avgt 10 1.8x ms/op > compareToLL 512 avgt 10 2.7x ms/op > compareToLU 24 avgt 10 1.6x ms/op > compareToLU 36 avgt 10 1.8x ms/op > compareToLU 72 avgt 10 2.3x ms/op > compareToLU 128 avgt 10 3.8x ms/op > compareToLU 256 avgt 10 4.7x ms/op > compareToLU 512 avgt 10 6.3x ms/op > compareToUL 24 avgt 10 1.6x ms/op > compareToUL 36 avgt 10 1.7x ms/op > compareToUL 72 avgt 10 2.2x ms/op > compareToUL 128 avgt 10 3.3x ms/op > compareToUL 256 avgt 10 4.4x ms/op > compareToUL 512 avgt 10 6.1x ms/op > compareToUU 24 avgt 10 1.0x ms/op > compareToUU 36 avgt 10 1.0x ms/op > compareToUU 72 avgt 10 1.4x ms/op > compareToUU 128 avgt 10 2.2x ms/op > compareToUU 256 avgt 10 2.6x ms/op > compareToUU 512 avgt 10 3.7x ms/op > > [1] > https://github.com/openjdk/jdk/blob/master/test/hotspot/jtreg/compiler/intrinsics/string/TestStringCompareToDifferentLength.java TatWai Chong has updated the pull request incrementally with one additional commit since the last revision: Replace `sve_cmpne` with up-to-date `sve_cmp`. - Changes: - all: https://git.openjdk.java.net/jdk/pull/5129/files - new: https://git.openjdk.java.net/jdk/pull/5129/files/4a584089..7799f934 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5129=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5129=02-03 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/5129.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5129/head:pull/5129 PR: https://git.openjdk.java.net/jdk/pull/5129
Re: RFR: 8271820: Implementation of JEP 416: Reimplement Core Reflection with Method Handle [v13]
On Thu, 14 Oct 2021 00:10:50 GMT, Maurizio Cimadamore
wrote:
>> Mandy Chung has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Minor cleanup. Improve javadoc in CallerSensitiveAdapter
>
> src/java.base/share/classes/jdk/internal/reflect/MethodHandleAccessorFactory.java
> line 151:
>
>> 149: var setter = isReadOnly ? null : JLIA.unreflectField(field,
>> true);
>> 150: Class type = field.getType();
>> 151: if (type == Boolean.TYPE) {
>
> dumb question: any reason why `boolean.class` (which is compiled to a LDC) is
> not used?
I only see `boolean.class` compiled to `getstatic Boolean.TYPE`. Is there a
javac flag to compile it to a LDC?
-
PR: https://git.openjdk.java.net/jdk/pull/5027
Re: RFR: 8271820: Implementation of JEP 416: Reimplement Core Reflection with Method Handle [v13]
On Wed, 13 Oct 2021 18:53:22 GMT, Mandy Chung wrote:
>> This reimplements core reflection with method handles.
>>
>> For `Constructor::newInstance` and `Method::invoke`, the new implementation
>> uses `MethodHandle`. For `Field` accessor, the new implementation uses
>> `VarHandle`.For the first few invocations of one of these reflective
>> methods on a specific reflective object we invoke the corresponding method
>> handle directly. After that we spin a dynamic bytecode stub defined in a
>> hidden class which loads the target `MethodHandle` or `VarHandle` from its
>> class data as a dynamically computed constant. Loading the method handle
>> from a constant allows JIT to inline the method-handle invocation in order
>> to achieve good performance.
>>
>> The VM's native reflection methods are needed during early startup, before
>> the method-handle mechanism is initialized. That happens soon after
>> System::initPhase1 and before System::initPhase2, after which we switch to
>> using method handles exclusively.
>>
>> The core reflection and method handle implementation are updated to handle
>> chained caller-sensitive method calls [1] properly. A caller-sensitive
>> method can define with a caller-sensitive adapter method that will take an
>> additional caller class parameter and the adapter method will be annotated
>> with `@CallerSensitiveAdapter` for better auditing. See the detailed
>> description from [2].
>>
>> Ran tier1-tier8 tests.
>>
>> [1] https://bugs.openjdk.java.net/browse/JDK-8013527
>> [2]
>> https://bugs.openjdk.java.net/browse/JDK-8271820?focusedCommentId=14439430=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14439430
>
> Mandy Chung has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Minor cleanup. Improve javadoc in CallerSensitiveAdapter
Looks a very good simplification. It's great that in the new `poly` benchmarks
the regression is so contained (I was frankly expecting more), and I agree with
the comments (super interesting discussion btw!) that Poly is probably the most
relevant case out there.
I noted that in the static case, Poly does regress for fields - do we know why
instance Poly is better than static Poly? That seems surprising.
src/java.base/share/classes/jdk/internal/reflect/DirectMethodHandleAccessor.java
line 58:
> 56: * Creates a MethodAccessorImpl for a caller-sensitive method.
> 57: */
> 58: static MethodAccessorImpl callerSensitiveMethodAccessor(Method
> method, MethodHandle dmh) {
This method and the one above are identical - they just call `new
DirectMethodHandleAccessor` with same parameters. Is the distinction between
these two factories still relevant? (besides the different asserts)
src/java.base/share/classes/jdk/internal/reflect/DirectMethodHandleAccessor.java
line 86:
> 84: }
> 85:
> 86: private static final int PARAM_COUNT_MASK = 0x00FF;
Is this packing logic required? I get it that it saves footprint - but then you
have to always unmask bits to get the argument count (see invoke and other
places). If you keep this, it might be worth documenting what the bit layout is
supposed to be?
src/java.base/share/classes/jdk/internal/reflect/MethodHandleAccessorFactory.java
line 151:
> 149: var setter = isReadOnly ? null : JLIA.unreflectField(field,
> true);
> 150: Class type = field.getType();
> 151: if (type == Boolean.TYPE) {
dumb question: any reason why `boolean.class` (which is compiled to a LDC) is
not used?
-
Marked as reviewed by mcimadamore (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/5027
Withdrawn: 8153490: Cannot setBytes() if incoming buffer's length is bigger than number of elements we want to insert.
On Wed, 12 May 2021 17:48:50 GMT, Mitsuru Kariya wrote: > Fix `SerialBlob.setBytes(long pos, byte[] bytes, int offset, int length)` in > the following cases: > > 1. `pos - 1 + bytes.length - offset > this.length() && pos - 1 + length <= > this.length()` >The original implementation throws `ArrayIndexOutOfBoundsException` but > this case should end successfully. >(test31) > > 2. `pos - 1 + length > this.length()` >The original implementation throws `ArrayIndexOutOfBoundsException` but > this case should end successfully. *1 >(test32) > > 3. `pos == this.length() + 1` >The original implementation throws `SerialException` but this case should > end successfully. *2 >(test33) > > 4. `length < 0` >The original implementation throws `ArrayIndexOutOfBoundsException` but > this case should throw `SerialException`. >(test34) > > 5. `offset + length > Integer.MAX_VALUE` >The original implementation throws `ArrayIndexOutOfBoundsException` (or > `OutOfMemoryError` in most cases) but this case should throw > `SerialException`. >(test35) > > Additionally, fix `SerialClob.setString(long pos, String str, int offset, int > length)` in the following cases: > > 1. `offset > str.length()` >The original implementaion throws `StringIndexOutOfBoundsException` but > this case should throw `SerialException`. >(test39) > > 2. `pos - 1 + str.length() - offset > this.length() && pos - 1 + length <= > this.length()` >The original implementation throws `ArrayIndexOutOfBoundsException` but > this case should end successfully. >(test32) > > 3. `pos - 1 + length > this.length()` >The original implementaion throws `SerialException` but this case should > end successfully. *3 >(test40) > > 4. `pos == this.length() + 1` >The original implementaion throws `SerialException` but this case should > end successfully. *4 >(test41) > > 5. `length < 0` >The original implementation throws `StringIndexOutOfBoundsException` but > this case should throw `SerialException`. >(test42) > > 6. `offset + length > Integer.MAX_VALUE` >The original implementation throws `ArrayIndexOutOfBoundsException` (or > `OutOfMemoryError` in most cases) but this case should throw > `SerialException`. >(test43) > > > The javadoc has also been modified according to the above. > > *1 The documentation of `Blob.setBytes()` says, "If the end of the Blob value > is reached while writing the array of bytes, then the length of the Blob > value will be increased to accommodate the extra bytes." > > *2 The documentation of `Blob.setBytes()` says, "If the value specified for > pos is greater than the length+1 of the BLOB value then the behavior is > undefined." >So, it should work correctly when pos == length+1 of the BLOB value. > > *3 The documentation of `Clob.setString()` says, "If the end of the Clob > value is eached while writing the given string, then the length of the Clob > value will be increased to accommodate the extra characters." > > *4 The documentation of `Clob.setString()` says, "If the value specified for > pos is greater than the length+1 of the CLOB value then the behavior is > undefined." >So, it should work correctly when pos == length+1 of the CLOB value. This pull request has been closed without being integrated. - PR: https://git.openjdk.java.net/jdk/pull/4001
Re: RFR: 8269559: AArch64: Implement string_compare intrinsic in SVE [v3]
> This patch implements string_compare intrinsic in SVE. > It supports all LL, LU, UL and UU comparisons. > > As we haven't found an existing benchmark to measure performance impact, > we created a benchmark derived from the test [1] for this evaluation. > This benchmark is attached to this patch. > > Besides, remove the unused temporary register `vtmp3` from the existing > match rules for StrCmp. > > The result below shows all varients can be benefited largely. > Command: make exploded-test TEST="micro:StringCompareToDifferentLength" > > Benchmark(size) Mode Cnt Score Speedup Units > compareToLL 24 avgt 10 1.0x ms/op > compareToLL 36 avgt 10 1.0x ms/op > compareToLL 72 avgt 10 1.0x ms/op > compareToLL 128 avgt 10 1.4x ms/op > compareToLL 256 avgt 10 1.8x ms/op > compareToLL 512 avgt 10 2.7x ms/op > compareToLU 24 avgt 10 1.6x ms/op > compareToLU 36 avgt 10 1.8x ms/op > compareToLU 72 avgt 10 2.3x ms/op > compareToLU 128 avgt 10 3.8x ms/op > compareToLU 256 avgt 10 4.7x ms/op > compareToLU 512 avgt 10 6.3x ms/op > compareToUL 24 avgt 10 1.6x ms/op > compareToUL 36 avgt 10 1.7x ms/op > compareToUL 72 avgt 10 2.2x ms/op > compareToUL 128 avgt 10 3.3x ms/op > compareToUL 256 avgt 10 4.4x ms/op > compareToUL 512 avgt 10 6.1x ms/op > compareToUU 24 avgt 10 1.0x ms/op > compareToUU 36 avgt 10 1.0x ms/op > compareToUU 72 avgt 10 1.4x ms/op > compareToUU 128 avgt 10 2.2x ms/op > compareToUU 256 avgt 10 2.6x ms/op > compareToUU 512 avgt 10 3.7x ms/op > > [1] > https://github.com/openjdk/jdk/blob/master/test/hotspot/jtreg/compiler/intrinsics/string/TestStringCompareToDifferentLength.java TatWai Chong has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains three commits: - Merge master - Restore the removal of vtmp3 (=V2) as it is still used by the non-SVE compare-long-strings stub. And remove the assertion in `string_compare` since it won't help as the registers used in the stub are fixed. - 8269559: AArch64: Implement string_compare intrinsic in SVE This patch implements string_compare intrinsic in SVE. It supports all LL, LU, UL and UU comparisons. As we haven't found an existing benchmark to measure performance impact, we created a benchmark derived from the test [1] for this evaluation. This benchmark is attached to this patch. Besides, remove the unused temporary register `vtmp3` from the existing match rules for StrCmp. The result below shows all varients can be benefited largely. Command: make exploded-test TEST="micro:StringCompareToDifferentLength" Benchmark(size) Mode Cnt Score Speedup Units compareToLL 24 avgt 10 1.0x ms/op compareToLL 36 avgt 10 1.0x ms/op compareToLL 72 avgt 10 1.0x ms/op compareToLL 128 avgt 10 1.4x ms/op compareToLL 256 avgt 10 1.8x ms/op compareToLL 512 avgt 10 2.7x ms/op compareToLU 24 avgt 10 1.6x ms/op compareToLU 36 avgt 10 1.8x ms/op compareToLU 72 avgt 10 2.3x ms/op compareToLU 128 avgt 10 3.8x ms/op compareToLU 256 avgt 10 4.7x ms/op compareToLU 512 avgt 10 6.3x ms/op compareToUL 24 avgt 10 1.6x ms/op compareToUL 36 avgt 10 1.7x ms/op compareToUL 72 avgt 10 2.2x ms/op compareToUL 128 avgt 10 3.3x ms/op compareToUL 256 avgt 10 4.4x ms/op compareToUL 512 avgt 10 6.1x ms/op compareToUU 24 avgt 10 1.0x ms/op compareToUU 36 avgt 10 1.0x ms/op compareToUU 72 avgt 10 1.4x ms/op compareToUU 128 avgt 10 2.2x ms/op compareToUU 256 avgt 10 2.6x ms/op compareToUU 512 avgt 10 3.7x ms/op [1] https://github.com/openjdk/jdk/blob/master/test/hotspot/jtreg/compiler/intrinsics/string/TestStringCompareToDifferentLength.java - Changes: https://git.openjdk.java.net/jdk/pull/5129/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5129=02 Stats: 517 lines in 11 files changed: 421 ins; 1 del; 95 mod Patch: https://git.openjdk.java.net/jdk/pull/5129.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5129/head:pull/5129 PR: https://git.openjdk.java.net/jdk/pull/5129
Re: RFR: JDK-8275249: Suppress warnings on non-serializable array component types in jdk.jlink
On Wed, 13 Oct 2021 21:08:41 GMT, Joe Darcy wrote: > After a refinement to the checks under development in #5709, the new checks > examine array types of serial fields and warn if the underlying component > type is not serializable. Per the JLS, all array types are serializable, but > if the base component is not serializable, the serialization process can > throw an exception. > > From "Java Object Serialization Specification: 2 - Object Output Classes": > > "If the object is an array, writeObject is called recursively to write the > ObjectStreamClass of the array. The handle for the array is assigned. It is > followed by the length of the array. Each element of the array is then > written to the stream, after which writeObject returns." > > The jdk.jlink module has one instance of this coding pattern that needs > suppression to compile successfully under the future warning. Marked as reviewed by iris (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5936
RFR: JDK-8275249: Suppress warnings on non-serializable array component types in jdk.jlink
After a refinement to the checks under development in #5709, the new checks examine array types of serial fields and warn if the underlying component type is not serializable. Per the JLS, all array types are serializable, but if the base component is not serializable, the serialization process can throw an exception. >From "Java Object Serialization Specification: 2 - Object Output Classes": "If the object is an array, writeObject is called recursively to write the ObjectStreamClass of the array. The handle for the array is assigned. It is followed by the length of the array. Each element of the array is then written to the stream, after which writeObject returns." The jdk.jlink module has one instance of this coding pattern that needs suppression to compile successfully under the future warning. - Commit messages: - JDK-8275249: Suppress warnings on non-serializable array component types in jdk.jlink Changes: https://git.openjdk.java.net/jdk/pull/5936/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5936=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8275249 Stats: 2 lines in 1 file changed: 1 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/5936.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5936/head:pull/5936 PR: https://git.openjdk.java.net/jdk/pull/5936
Re: RFR: 8271515: Integration of JEP 417: Vector API (Third Incubator)
On Fri, 8 Oct 2021 21:25:26 GMT, Paul Sandoz wrote: > This PR improves the performance of vector operations that accept masks on > architectures that support masking in hardware, specifically Intel AVX512 and > ARM SVE. > > On architectures that do not support masking in hardware the same technique > as before is applied to most operations, specifically composition using blend. > > Masked loads/stores are a special form of masked operation that require > additional care to ensure out-of-bounds access throw exceptions. The range > checking has not been fully optimized and will require further work. > > No API enhancements were required and only a few additional tests were needed. C2 and x86 changes seems fine. - Marked as reviewed by kvn (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5873
Re: RFR: 8271820: Implementation of JEP 416: Reimplement Core Reflection with Method Handle [v13]
On Wed, 13 Oct 2021 18:53:22 GMT, Mandy Chung wrote: >> This reimplements core reflection with method handles. >> >> For `Constructor::newInstance` and `Method::invoke`, the new implementation >> uses `MethodHandle`. For `Field` accessor, the new implementation uses >> `VarHandle`.For the first few invocations of one of these reflective >> methods on a specific reflective object we invoke the corresponding method >> handle directly. After that we spin a dynamic bytecode stub defined in a >> hidden class which loads the target `MethodHandle` or `VarHandle` from its >> class data as a dynamically computed constant. Loading the method handle >> from a constant allows JIT to inline the method-handle invocation in order >> to achieve good performance. >> >> The VM's native reflection methods are needed during early startup, before >> the method-handle mechanism is initialized. That happens soon after >> System::initPhase1 and before System::initPhase2, after which we switch to >> using method handles exclusively. >> >> The core reflection and method handle implementation are updated to handle >> chained caller-sensitive method calls [1] properly. A caller-sensitive >> method can define with a caller-sensitive adapter method that will take an >> additional caller class parameter and the adapter method will be annotated >> with `@CallerSensitiveAdapter` for better auditing. See the detailed >> description from [2]. >> >> Ran tier1-tier8 tests. >> >> [1] https://bugs.openjdk.java.net/browse/JDK-8013527 >> [2] >> https://bugs.openjdk.java.net/browse/JDK-8271820?focusedCommentId=14439430=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14439430 > > Mandy Chung has updated the pull request incrementally with one additional > commit since the last revision: > > Minor cleanup. Improve javadoc in CallerSensitiveAdapter The JDI problem list and EATest.java changes are approved. test/hotspot/jtreg/ProblemList.txt line 43: > 41: vmTestbase/nsk/jvmti/AttachOnDemand/attach002a/TestDescription.java > 8265795 generic-all > 42: vmTestbase/nsk/jvmti/AttachOnDemand/attach022/TestDescription.java > 8265795 generic-all > 43: > vmTestbase/nsk/jdi/ObjectReference/referringObjects/referringObjects002/referringObjects002.java > 8265796 generic-all Approved. test/jdk/com/sun/jdi/EATests.java line 2203: > 2201: // the method depth in debuggee is 11 as it includes all hidden > frames > 2202: // the expected method depth is 6 excluding 5 hidden frames > 2203: testMethodDepth = 11-5; Approved. - Marked as reviewed by cjplummer (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5027
Re: RFR: 8271820: Implementation of JEP 416: Reimplement Core Reflection with Method Handle [v13]
> This reimplements core reflection with method handles. > > For `Constructor::newInstance` and `Method::invoke`, the new implementation > uses `MethodHandle`. For `Field` accessor, the new implementation uses > `VarHandle`.For the first few invocations of one of these reflective > methods on a specific reflective object we invoke the corresponding method > handle directly. After that we spin a dynamic bytecode stub defined in a > hidden class which loads the target `MethodHandle` or `VarHandle` from its > class data as a dynamically computed constant. Loading the method handle from > a constant allows JIT to inline the method-handle invocation in order to > achieve good performance. > > The VM's native reflection methods are needed during early startup, before > the method-handle mechanism is initialized. That happens soon after > System::initPhase1 and before System::initPhase2, after which we switch to > using method handles exclusively. > > The core reflection and method handle implementation are updated to handle > chained caller-sensitive method calls [1] properly. A caller-sensitive method > can define with a caller-sensitive adapter method that will take an > additional caller class parameter and the adapter method will be annotated > with `@CallerSensitiveAdapter` for better auditing. See the detailed > description from [2]. > > Ran tier1-tier8 tests. > > [1] https://bugs.openjdk.java.net/browse/JDK-8013527 > [2] > https://bugs.openjdk.java.net/browse/JDK-8271820?focusedCommentId=14439430=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14439430 Mandy Chung has updated the pull request incrementally with one additional commit since the last revision: Minor cleanup. Improve javadoc in CallerSensitiveAdapter - Changes: - all: https://git.openjdk.java.net/jdk/pull/5027/files - new: https://git.openjdk.java.net/jdk/pull/5027/files/86d34f48..c25d651a Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5027=12 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5027=11-12 Stats: 95 lines in 3 files changed: 83 ins; 0 del; 12 mod Patch: https://git.openjdk.java.net/jdk/pull/5027.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5027/head:pull/5027 PR: https://git.openjdk.java.net/jdk/pull/5027
Re: RFR: 8275197: Remove unused fields in ThaiBuddhistChronology
On Tue, 12 Oct 2021 21:10:12 GMT, Andrey Turbanov wrote: > Remove 3 unused HashMap's. > Reported here > https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-September/081866.html > I did the similar PR to treetenbp and it was merged > https://github.com/ThreeTen/threetenbp/pull/155 Marked as reviewed by naoto (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5917
RFR: 8271515: Integration of JEP 417: Vector API (Third Incubator)
This PR improves the performance of vector operations that accept masks on architectures that support masking in hardware, specifically Intel AVX512 and ARM SVE. On architectures that do not support masking in hardware the same technique as before is applied to most operations, specifically composition using blend. Masked loads/stores are a special form of masked operation that require additional care to ensure out-of-bounds access throw exceptions. The range checking has not been fully optimized and will require further work. No API enhancements were required and only a few additional tests were needed. - Commit messages: - Apply patch from https://github.com/openjdk/panama-vector/pull/142 - Apply patch from https://github.com/openjdk/panama-vector/pull/139 - Apply patch from https://github.com/openjdk/panama-vector/pull/151 - Add new files. - 8271515: Integration of JEP 417: Vector API (Third Incubator) Changes: https://git.openjdk.java.net/jdk/pull/5873/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5873=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8271515 Stats: 21999 lines in 106 files changed: 16228 ins; 2077 del; 3694 mod Patch: https://git.openjdk.java.net/jdk/pull/5873.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5873/head:pull/5873 PR: https://git.openjdk.java.net/jdk/pull/5873
Re: RFR: 8275163: Deflater::deflate methods omit javadoc for ReadOnlyBufferException
On Wed, 13 Oct 2021 17:43:29 GMT, Lance Andersen wrote: > Hi all, > > Please review the fix to address a javadoc issue for the Deflater::deflate > methods that were added as part of JDK-6341887 that could throw a > ReadOnlyBufferException. > > The` @throws ` clause for `ReadOnlyBufferException` was inadvertently > omitted from the javadoc for these new methods. > > A CSR, JDK-8275164, has also been created for this issue. > > Best > Lance Marked as reviewed by naoto (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5931
Re: RFR: 8275163: Deflater::deflate methods omit javadoc for ReadOnlyBufferException
On Wed, 13 Oct 2021 18:20:03 GMT, Brian Burkhalter wrote: >> Hi all, >> >> Please review the fix to address a javadoc issue for the Deflater::deflate >> methods that were added as part of JDK-6341887 that could throw a >> ReadOnlyBufferException. >> >> The` @throws ` clause for `ReadOnlyBufferException` was inadvertently >> omitted from the javadoc for these new methods. >> >> A CSR, JDK-8275164, has also been created for this issue. >> >> Best >> Lance > > src/java.base/share/classes/java/util/zip/Deflater.java line 498: > >> 496: * @return the actual number of bytes of compressed data written to >> the >> 497: * output buffer >> 498: * @throws ReadOnlyBufferException if the given output buffer is >> read-only > > It could equally well simply state > > if the buffer is read-only > > but this is fine. I thought about that but I decided to go with the wording used for the change to Inflater::inflate that was added as part of JDK-6341887. If I were to change it, I would want to change Inflater::inflate as well. - PR: https://git.openjdk.java.net/jdk/pull/5931
Re: RFR: 8275163: Deflater::deflate methods omit javadoc for ReadOnlyBufferException
On Wed, 13 Oct 2021 18:24:39 GMT, Lance Andersen wrote: >> src/java.base/share/classes/java/util/zip/Deflater.java line 498: >> >>> 496: * @return the actual number of bytes of compressed data written >>> to the >>> 497: * output buffer >>> 498: * @throws ReadOnlyBufferException if the given output buffer is >>> read-only >> >> It could equally well simply state >> >> if the buffer is read-only >> >> but this is fine. > > I thought about that but I decided to go with the wording used for the > change to Inflater::inflate that was added as part of JDK-6341887. If I were > to change it, I would want to change Inflater::inflate as well. I retract my previous comment: I think you made the correct decision. - PR: https://git.openjdk.java.net/jdk/pull/5931
Re: RFR: 8275163: Deflater::deflate methods omit javadoc for ReadOnlyBufferException
On Wed, 13 Oct 2021 17:43:29 GMT, Lance Andersen wrote: > Hi all, > > Please review the fix to address a javadoc issue for the Deflater::deflate > methods that were added as part of JDK-6341887 that could throw a > ReadOnlyBufferException. > > The` @throws ` clause for `ReadOnlyBufferException` was inadvertently > omitted from the javadoc for these new methods. > > A CSR, JDK-8275164, has also been created for this issue. > > Best > Lance Associated CSR also Reviewed. - Marked as reviewed by iris (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5931
Re: RFR: 8275163: Deflater::deflate methods omit javadoc for ReadOnlyBufferException
On Wed, 13 Oct 2021 17:43:29 GMT, Lance Andersen wrote: > Hi all, > > Please review the fix to address a javadoc issue for the Deflater::deflate > methods that were added as part of JDK-6341887 that could throw a > ReadOnlyBufferException. > > The` @throws ` clause for `ReadOnlyBufferException` was inadvertently > omitted from the javadoc for these new methods. > > A CSR, JDK-8275164, has also been created for this issue. > > Best > Lance Marked as reviewed by bpb (Reviewer). src/java.base/share/classes/java/util/zip/Deflater.java line 498: > 496: * @return the actual number of bytes of compressed data written to > the > 497: * output buffer > 498: * @throws ReadOnlyBufferException if the given output buffer is > read-only It could equally well simply state if the buffer is read-only but this is fine. - PR: https://git.openjdk.java.net/jdk/pull/5931
Re: RFR: 8274544: Langtools command's usage were garbled on Japanese Windows [v3]
On Fri, 8 Oct 2021 21:07:32 GMT, Naoto Sato wrote:
>> Ichiroh Takiguchi has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> 8274544: Langtools command's usage were garbled on Japanese Windows
>
> BTW, does the PoC in the jshell bug report really causing the issue?
>
> System.out.println("\u3042")
>
> This is ASCII, so save/restore does not seem to cause any issues across JDKs
> with and without JEP400. Did you mean `Systemout.println("あ")` instead?
Hello @naotoj .
Sorry I'm late.
I'd like to show you jshell issue step by step.
1. `System.out.println(...)` did not work if non-ASCII character was printed on
JDK18-b13.
Because jshell output encoding was MS932, jshell agent output encoding was UTF-8
2. Above fix was applied against `JShellToolProvider.java` only.
The issue was not fixed.
3. Just applied lahodaj's fix `JShellToolBuilder.java`.
It worked fine as expected
4. I checked compatibility between JDK17 and this fix by using `/save` and
`/open`
It seemed saved a.jsh's encoding was MS932
5. I think jshell and agent should use same `file.encoding` system property. I
applied following fix.
--- a/src/jdk.jshell/share/classes/jdk/jshell/execution/JdiInitiator.java
+++ b/src/jdk.jshell/share/classes/jdk/jshell/execution/JdiInitiator.java
@@ -27,6 +27,7 @@ package jdk.jshell.execution;
import java.io.File;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
+import java.nio.charset.Charset;
import java.nio.file.Files;
import java.util.ArrayList;
import java.util.HashMap;
@@ -86,6 +87,17 @@ public class JdiInitiator {
Map customConnectorArgs) {
this.remoteAgent = remoteAgent;
this.connectTimeout = (int) (timeout * CONNECT_TIMEOUT_FACTOR);
+if (!StandardCharsets.UTF_8.equals(Charset.defaultCharset())) {
+boolean encodingFlag = true;
+for (String s : remoteVMOptions.toArray(new String[0])) {
+if (s.startsWith("-Dfile.encoding="))
+encodingFlag = false;
+}
+if (encodingFlag) {
+remoteVMOptions.add("-Dfile.encoding="
++Charset.defaultCharset().name());
+}
+}
String connectorName
= isLaunch
? "com.sun.jdi.CommandLineLaunch"
I think `JShellToolBuilder.java` and `JdiInitiator.java`.
Could you give me some suggestions ?
-
PR: https://git.openjdk.java.net/jdk/pull/5771
Re: RFR: 8266936: Add a finalization JFR event [v16]
> Greetings, > > Object.finalize() was deprecated in JDK9. There is an ongoing effort to > replace and mitigate Object.finalize() uses in the JDK libraries; please see > https://bugs.openjdk.java.net/browse/JDK-8253568 for more information. > > We also like to assist users in replacing and mitigating uses in non-JDK code. > > Hence, this changeset adds a periodic JFR event to help identify which > classes are overriding Object.finalize(). > > Thanks > Markus Markus Grönlund has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains two new commits since the last revision: - cleanup - rebased and adjusted for new lock rankings - Changes: - all: https://git.openjdk.java.net/jdk/pull/4731/files - new: https://git.openjdk.java.net/jdk/pull/4731/files/5359a793..96a9d6bf Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4731=15 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4731=14-15 Stats: 0 lines in 0 files changed: 0 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/4731.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4731/head:pull/4731 PR: https://git.openjdk.java.net/jdk/pull/4731
RFR: 8275163: Deflater::deflate methods omit javadoc for ReadOnlyBufferException
Hi all, Please review the fix to address a javadoc issue for the Deflater::deflate methods that were added as part of JDK-6341887 that could throw a ReadOnlyBufferException. The` @throws ` clause for `ReadOnlyBufferException` was inadvertently omitted from the javadoc for these new methods. A CSR, JDK-8275164, has also been created for this issue. Best Lance - Commit messages: - Add missing @throws for ReadOnlyBufferException Changes: https://git.openjdk.java.net/jdk/pull/5931/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5931=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8275163 Stats: 2 lines in 1 file changed: 2 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/5931.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5931/head:pull/5931 PR: https://git.openjdk.java.net/jdk/pull/5931
Re: RFR: 8266936: Add a finalization JFR event [v15]
> Greetings, > > Object.finalize() was deprecated in JDK9. There is an ongoing effort to > replace and mitigate Object.finalize() uses in the JDK libraries; please see > https://bugs.openjdk.java.net/browse/JDK-8253568 for more information. > > We also like to assist users in replacing and mitigating uses in non-JDK code. > > Hence, this changeset adds a periodic JFR event to help identify which > classes are overriding Object.finalize(). > > Thanks > Markus Markus Grönlund has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 44 commits: - Merge branch '8266936-alt' of github.com:mgronlun/jdk into 8266936-alt - symbols-unix - jvm.h and JVM_Entry - no precompiled headers and mtServiceability nmt classification - remove rehashing and rely on default grow_hint for table resize - mtStatistics - localize - cleanup - FinalizerStatistics - Model as finalizerService - ... and 34 more: https://git.openjdk.java.net/jdk/compare/124f8237...5359a793 - Changes: https://git.openjdk.java.net/jdk/pull/4731/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4731=14 Stats: 1893 lines in 36 files changed: 1375 ins; 409 del; 109 mod Patch: https://git.openjdk.java.net/jdk/pull/4731.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4731/head:pull/4731 PR: https://git.openjdk.java.net/jdk/pull/4731
Integrated: JDK-8275187: Suppress warnings on non-serializable array component types in java.sql.rowset
On Wed, 13 Oct 2021 05:02:15 GMT, Joe Darcy wrote: > After a refinement to the checks under development in #5709, the new checks > examine array types of serial fields and warn if the underlying component > type is not serializable. Per the JLS, all array types are serializable, but > if the base component is not serializable, the serialization process can > throw an exception. > > From "Java Object Serialization Specification: 2 - Object Output Classes": > > "If the object is an array, writeObject is called recursively to write the > ObjectStreamClass of the array. The handle for the array is assigned. It is > followed by the length of the array. Each element of the array is then > written to the stream, after which writeObject returns." > > The java.sql.rowset module has several instances of this coding pattern that > need suppression to compile successfully under the future warning. This pull request has now been integrated. Changeset: d15fbc28 Author:Joe Darcy URL: https://git.openjdk.java.net/jdk/commit/d15fbc28afc3f2d509b4e46e70877a4650fafdc2 Stats: 5 lines in 3 files changed: 3 ins; 0 del; 2 mod 8275187: Suppress warnings on non-serializable array component types in java.sql.rowset Reviewed-by: lancea - PR: https://git.openjdk.java.net/jdk/pull/5925
Re: RFR: 8273111: Default timezone should return zone ID if /etc/localtime is valid but not canonicalization on linux [v3]
On Wed, 13 Oct 2021 09:15:25 GMT, Wu Yan wrote: >> Hi, >> Please help me review the change to enhance getting time zone ID from >> /etc/localtime on linux. >> >> We use `realpath` instead of `readlink` to obtain the link name of >> /etc/localtime, because `readlink` can only read the value of a symbolic of >> link, not the canonicalized absolute pathname. >> >> For example, the value of /etc/localtime is >> "../usr/share/zoneinfo//Asia/Shanghai", then the linkbuf obtained by >> `readlink` is "../usr/share/zoneinfo//Asia/Shanghai", and then the call of >> `getZoneName(linkbuf)` will get "/Asia/Shanghai", not "Asia/Shanghai", which >> consider as invalid in `ZoneInfoFile.getZoneInfo()`. Using `realpath`, you >> can get “/usr/share/zoneinfo/Asia/Shanghai“ directly from “/etc/localtime“. >> >> Thanks, >> wuyan > > Wu Yan has updated the pull request incrementally with one additional commit > since the last revision: > > change functions to be static Looks good. Thanks for the fix. - Marked as reviewed by naoto (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5327
Re: RFR: 8275063: Implementation of Foreign Function & Memory API (Second incubator) [v4]
> This PR contains the API and implementation changes for JEP-419 [1]. A more > detailed description of such changes, to avoid repetitions during the review > process, is included as a separate comment. > > [1] - https://openjdk.java.net/jeps/419 Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision: Address review comments - Changes: - all: https://git.openjdk.java.net/jdk/pull/5907/files - new: https://git.openjdk.java.net/jdk/pull/5907/files/d6bf27ff..27e71ee7 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5907=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5907=02-03 Stats: 35 lines in 4 files changed: 8 ins; 11 del; 16 mod Patch: https://git.openjdk.java.net/jdk/pull/5907.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5907/head:pull/5907 PR: https://git.openjdk.java.net/jdk/pull/5907
Re: RFR: 8271820: Implementation of JEP 416: Reimplement Core Reflection with Method Handle [v12]
On Fri, 8 Oct 2021 23:31:32 GMT, Mandy Chung wrote: >> This reimplements core reflection with method handles. >> >> For `Constructor::newInstance` and `Method::invoke`, the new implementation >> uses `MethodHandle`. For `Field` accessor, the new implementation uses >> `VarHandle`.For the first few invocations of one of these reflective >> methods on a specific reflective object we invoke the corresponding method >> handle directly. After that we spin a dynamic bytecode stub defined in a >> hidden class which loads the target `MethodHandle` or `VarHandle` from its >> class data as a dynamically computed constant. Loading the method handle >> from a constant allows JIT to inline the method-handle invocation in order >> to achieve good performance. >> >> The VM's native reflection methods are needed during early startup, before >> the method-handle mechanism is initialized. That happens soon after >> System::initPhase1 and before System::initPhase2, after which we switch to >> using method handles exclusively. >> >> The core reflection and method handle implementation are updated to handle >> chained caller-sensitive method calls [1] properly. A caller-sensitive >> method can define with a caller-sensitive adapter method that will take an >> additional caller class parameter and the adapter method will be annotated >> with `@CallerSensitiveAdapter` for better auditing. See the detailed >> description from [2]. >> >> Ran tier1-tier8 tests. >> >> [1] https://bugs.openjdk.java.net/browse/JDK-8013527 >> [2] >> https://bugs.openjdk.java.net/browse/JDK-8271820?focusedCommentId=14439430=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14439430 > > Mandy Chung has updated the pull request incrementally with one additional > commit since the last revision: > > Fix left-over assignment I agree that this is in a good state for integration now, though it might be necessary to do some follow-up work to address different performance concerns: - While performance in the `*Const` micros are on the same level as after #5694 there is some added overhead in non-const cases - Cold start overheads leaves a few things to be desired, though the effects we can measure on targeted tests appear to be small in practice on larger apps. Some possible simplifications like using `asSpreader` always is currently unattractive due the added startup overheads. - Marked as reviewed by redestad (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5027
Re: RFR: 8275063: Implementation of Foreign Function & Memory API (Second incubator) [v3]
> This PR contains the API and implementation changes for JEP-419 [1]. A more > detailed description of such changes, to avoid repetitions during the review > process, is included as a separate comment. > > [1] - https://openjdk.java.net/jeps/419 Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision: Apply suggestions from code review Co-authored-by: Paul Sandoz - Changes: - all: https://git.openjdk.java.net/jdk/pull/5907/files - new: https://git.openjdk.java.net/jdk/pull/5907/files/23f69054..d6bf27ff Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5907=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5907=01-02 Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/5907.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5907/head:pull/5907 PR: https://git.openjdk.java.net/jdk/pull/5907
Re: RFR: JDK-8275187: Suppress warnings on non-serializable array component types in java.sql.rowset
On Wed, 13 Oct 2021 05:02:15 GMT, Joe Darcy wrote: > After a refinement to the checks under development in #5709, the new checks > examine array types of serial fields and warn if the underlying component > type is not serializable. Per the JLS, all array types are serializable, but > if the base component is not serializable, the serialization process can > throw an exception. > > From "Java Object Serialization Specification: 2 - Object Output Classes": > > "If the object is an array, writeObject is called recursively to write the > ObjectStreamClass of the array. The handle for the array is assigned. It is > followed by the length of the array. Each element of the array is then > written to the stream, after which writeObject returns." > > The java.sql.rowset module has several instances of this coding pattern that > need suppression to compile successfully under the future warning. Marked as reviewed by lancea (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5925
Integrated: 8273682: Upgrade Jline to 3.20.0
On Thu, 23 Sep 2021 14:43:21 GMT, Jan Lahoda wrote: > I'd like to upgrade the internal JLine to 3.20.0, to support the rxvt > terminal (see JDK-8270943), and to generally use a newer version of the > library. This patch is basically a application of relevant parts of the diff > between JLine 3.14.0 and 3.20.0, with merge fixes as needed. > > Thanks! This pull request has now been integrated. Changeset: b8cb76ad Author:Jan Lahoda URL: https://git.openjdk.java.net/jdk/commit/b8cb76ad210cb3e7524c7f5b13cfe57746ac05d4 Stats: 2667 lines in 47 files changed: 1830 ins; 473 del; 364 mod 8273682: Upgrade Jline to 3.20.0 Reviewed-by: sundar - PR: https://git.openjdk.java.net/jdk/pull/5655
Re: RFR: 8273111: Default timezone should return zone ID if /etc/localtime is valid but not canonicalization on linux [v3]
> Hi, > Please help me review the change to enhance getting time zone ID from > /etc/localtime on linux. > > We use `realpath` instead of `readlink` to obtain the link name of > /etc/localtime, because `readlink` can only read the value of a symbolic of > link, not the canonicalized absolute pathname. > > For example, the value of /etc/localtime is > "../usr/share/zoneinfo//Asia/Shanghai", then the linkbuf obtained by > `readlink` is "../usr/share/zoneinfo//Asia/Shanghai", and then the call of > `getZoneName(linkbuf)` will get "/Asia/Shanghai", not "Asia/Shanghai", which > consider as invalid in `ZoneInfoFile.getZoneInfo()`. Using `realpath`, you > can get “/usr/share/zoneinfo/Asia/Shanghai“ directly from “/etc/localtime“. > > Thanks, > wuyan Wu Yan has updated the pull request incrementally with one additional commit since the last revision: change functions to be static - Changes: - all: https://git.openjdk.java.net/jdk/pull/5327/files - new: https://git.openjdk.java.net/jdk/pull/5327/files/19cc634d..38177cd0 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5327=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5327=01-02 Stats: 6 lines in 2 files changed: 0 ins; 3 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/5327.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5327/head:pull/5327 PR: https://git.openjdk.java.net/jdk/pull/5327
Re: RFR: 8250678: ModuleDescriptor.Version parsing treats empty segments inconsistently
On Wed, 6 Oct 2021 18:40:36 GMT, Mark Reinhold wrote: >> @mbreinhold Could you comment on this pull request? > >> @mbreinhold Could you comment on this pull request? > > This is somewhat tricky code. I’ll take a look, but give me a few days. @mbreinhold I waited for about a week, but I haven't received an answer yet. Could you comment on this? - PR: https://git.openjdk.java.net/jdk/pull/5679
Re: RFR: 8273111: Default timezone should return zone ID if /etc/localtime is valid but not canonicalization on linux [v2]
On Mon, 11 Oct 2021 18:18:02 GMT, Naoto Sato wrote: >> Wu Yan has updated the pull request incrementally with one additional commit >> since the last revision: >> >> replace realpath > > src/java.base/unix/native/libjava/path_util.h line 31: > >> 29: int collapsible(char *names); >> 30: void splitNames(char *names, char **ix); >> 31: void joinNames(char *names, int nc, char **ix); > > Are these functions, `collapsible`, `splitNames` and `joinNames` have to be > non-static? You are right, thanks for your suggestions, I'll change these functions to static in next commit. - PR: https://git.openjdk.java.net/jdk/pull/5327
Re: RFR: 8273111: Default timezone should return zone ID if /etc/localtime is valid but not canonicalization on linux [v2]
On Mon, 11 Oct 2021 18:16:28 GMT, Naoto Sato wrote: >> Wu Yan has updated the pull request incrementally with one additional commit >> since the last revision: >> >> replace realpath > > src/java.base/unix/native/libjava/TimeZone_md.c line 113: > >> 111: } >> 112: } >> 113: > > There are a few `*(right + 1)` references in the loops. Is there any > possibility that it would run over the buffer? It wouldn't run over the buffer. Here `end` points to `'\0'`. At line 94 and line 99, `right` is less than `end`, so `right + 1` is at most `end`. At line 103, if `right` equals `end`, `*right != '\0'` will be false and `!(*right == '/' && *(right + 1) == '/')` will not executed. - PR: https://git.openjdk.java.net/jdk/pull/5327
Re: RFR: 4511638: Double.toString(double) sometimes produces incorrect results [v2]
On Fri, 16 Apr 2021 11:30:32 GMT, Raffaello Giulietti wrote: >> Hello, >> >> here's a PR for a patch submitted on March 2020 >> [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was >> a thing. >> >> The patch has been edited to adhere to OpenJDK code conventions about >> multi-line (block) comments. Nothing in the code proper has changed, except >> for the addition of redundant but clarifying parentheses in some expressions. >> >> >> Greetings >> Raffaello > > Raffaello Giulietti has updated the pull request incrementally with one > additional commit since the last revision: > > 4511638: Double.toString(double) sometimes produces incorrect results Email from GuySteele follows: (my reply) Hi Guy, for some reason your comments still appear garbled on the GitHub PR page and don't make it to the core-libs-dev mailing list at all. Luckily, they appear intelligible in my mailbox, so I'll keep going with prepending your comments in my replies: not ideal but good enough. Thanks so much for re-reading my "paper". printf() There are some issues to consider when trying to apply Schubfach to printf(), the main one being that printf() allows to specify an arbitrary length for the resulting decimal. This means, for example, that unlimited precision arithmetic is unavoidable. But it might be worthwhile to investigate using Schubfach for lengths up to H (9 and 17 for float and double, resp.) and fall back to unlimited precision beyond that. Before that, however, I would prefer to finally push Schubfach in the OpenJDK codebase for the toString() cases and close this PR. I completely agree that using Schubfach to solve only the toString() problems would be a _major_ improvement in the situation, and this should not wait for exploration of the printf problem. But I suspect that using Schubfach for lengths up to H would cover a very large fraction of actual usage, and would improve both quality and speed, and therefore would be worth exploring later. Tests Below, by "extensive test" I mean not only that the outcomes convert back without loss of information, but that they fully meet the spec about minimal number of digits, closeness, correct formatting (normal viz scientific), character set, etc. All currently available tests are in the contributed code of this PR and will be part of the OpenJDK once integrated. * All powers of 2 and the extreme values are already extensively tested. * All values closest to powers of 10 are extensively tested. * All values proposed by Paxson [1] are extensively tested. I have now read through the Paxson paper. Does this refer to the values listed in his Tables 3 and 4, or to other values instead or in addition? * A configurable number of random values are tested at each round (currently 4 * 1'000'000 random values). Should a value fail, there's enough diagnostic information for further investigation. I'll add extensive tests for the values you propose in point (1) and (2), setting Z = Y = 1024. I do think that would lend further confidence. As for comparison with the current JDK behavior, there are already a bunch of values for which extensive tests fail on the current JDK but pass with Schubfach. Yes, thanks for supplying some of those. It would be cumbersome, if possible at all, to have both the current JDK and Schubfach implementations in the same OpenJDK codebase to be able to compare the outcomes. I performed comparisons in a different constellation, with Schubfach as an external library, but this is hardly a possibility in the core-libs. Needless to say, Schubfach outcomes are either the same as in JDK or better (shorter or closest to the fp value). Okay. I will mention here, for the record, that there is one other sort of test that could be performed that I think I have not yet seen you mention: a monotonicity test of the kind used by David Hough’s Testbase (mentioned by Paxson). However, a little thought reveals that such a test made unnecessary by the round-trip test. So a monotonicity test would be a good idea when testing printf case, but is not needed for the toString case. Therefore, if you add the few tests I have suggested, I think that we can say with the best certainty we can have, short of separately testing every possible double value, that Schubfach is extremely well tested and ready for adoption into Java. Peer reviewed publication Shortening my 27 pages writing and re-formating it to meet a journal standards for publication would require investing yet another substantial amount of time. I'm not sure I'm prepared for that, given that I've no personal interest in a journal publication: I'm not an academic, I'm not pursuing a career... But I promise I'll think about reconsidering my position on this ;-) Please do think about reconsidering. There are several reasons to publish an “academic” paper: - Earning “merit badges” that lead to academic tenure -
Re: RFR: 4511638: Double.toString(double) sometimes produces incorrect results [v2]
On Fri, 16 Apr 2021 11:30:32 GMT, Raffaello Giulietti wrote: >> Hello, >> >> here's a PR for a patch submitted on March 2020 >> [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was >> a thing. >> >> The patch has been edited to adhere to OpenJDK code conventions about >> multi-line (block) comments. Nothing in the code proper has changed, except >> for the addition of redundant but clarifying parentheses in some expressions. >> >> >> Greetings >> Raffaello > > Raffaello Giulietti has updated the pull request incrementally with one > additional commit since the last revision: > > 4511638: Double.toString(double) sometimes produces incorrect results Hi Guy, > I have now read through the Paxson paper. Does this refer to the values > listed in his Tables 3 and 4, or to other values instead or in addition? Right. >> Shortening my 27 pages writing and re-formating it to meet a journal >> standards for publication would require investing yet another substantial >> amount of time. I'm not sure I'm prepared for that, given that I've no >> personal interest in a journal publication: I'm not an academic, I'm not >> pursuing a career... >> But I promise I'll think about reconsidering my position on this ;-) > > Please do think about reconsidering. OK, thanks for the useful suggestions. Greetings Raffaello - PR: https://git.openjdk.java.net/jdk/pull/3402
Re: RFR: 4511638: Double.toString(double) sometimes produces incorrect results [v2]
On Fri, 16 Apr 2021 11:30:32 GMT, Raffaello Giulietti wrote: >> Hello, >> >> here's a PR for a patch submitted on March 2020 >> [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was >> a thing. >> >> The patch has been edited to adhere to OpenJDK code conventions about >> multi-line (block) comments. Nothing in the code proper has changed, except >> for the addition of redundant but clarifying parentheses in some expressions. >> >> >> Greetings >> Raffaello > > Raffaello Giulietti has updated the pull request incrementally with one > additional commit since the last revision: > > 4511638: Double.toString(double) sometimes produces incorrect results (Replies from Guy Steele) > Hi Guy, > > for some reason your comments still appear garbled on the GitHub PR page and > don't make it to the core-libs-dev mailing list at all. Luckily, they appear > intelligible in my mailbox, so I'll keep going with prepending your comments > in my replies: not ideal but good enough. > > Thanks so much for re-reading my "paper". > > printf() > > There are some issues to consider when trying to apply Schubfach to printf(), > the main one being that printf() allows to specify an arbitrary length for > the resulting decimal. This means, for example, that unlimited precision > arithmetic is unavoidable. But it might be worthwhile to investigate using > Schubfach for lengths up to H (9 and 17 for float and double, resp.) and fall > back to unlimited precision beyond that. > Before that, however, I would prefer to finally push Schubfach in the OpenJDK > codebase for the toString() cases and close this PR. I completely agree that using Schubfach to solve only the toString() problems would be a _major_ improvement in the situation, and this should not wait for exploration of the printf problem. But I suspect that using Schubfach for lengths up to H would cover a very large fraction of actual usage, and would improve both quality and speed, and therefore would be worth exploring later. > Tests > > Below, by "extensive test" I mean not only that the outcomes convert back > without loss of information, but that they fully meet the spec about minimal > number of digits, closeness, correct formatting (normal viz scientific), > character set, etc. > > All currently available tests are in the contributed code of this PR and will > be part of the OpenJDK once integrated. > > * All powers of 2 and the extreme values are already extensively tested. > * All values closest to powers of 10 are extensively tested. > * All values proposed by Paxson [1] are extensively tested. I have now read through the Paxson paper. Does this refer to the values listed in his Tables 3 and 4, or to other values instead or in addition? > * A configurable number of random values are tested at each round (currently > 4 * 1'000'000 random values). Should a value fail, there's enough diagnostic > information for further investigation. > > I'll add extensive tests for the values you propose in point (1) and (2), > setting Z = Y = 1024. > I do think that would lend further confidence. > As for comparison with the current JDK behavior, there are already a bunch of > values for which extensive tests fail on the current JDK but pass with > Schubfach. Yes, thanks for supplying some of those. > It would be cumbersome, if possible at all, to have both the current JDK and > Schubfach implementations in the same OpenJDK codebase to be able to compare > the outcomes. I performed comparisons in a different constellation, with > Schubfach as an external library, but this is hardly a possibility in the > core-libs. Needless to say, Schubfach outcomes are either the same as in JDK > or better (shorter or closest to the fp value). Okay. I will mention here, for the record, that there is one other sort of test that could be performed that I think I have not yet seen you mention: a monotonicity test of the kind used by David Hough’s Testbase (mentioned by Paxson). However, a little thought reveals that such a test made unnecessary by the round-trip test. So a monotonicity test would be a good idea when testing printf case, but is not needed for the toString case. Therefore, if you add the few tests I have suggested, I think that we can say with the best certainty we can have, short of separately testing every possible double value, that Schubfach is extremely well tested and ready for adoption into Java. > Peer reviewed publication > > Shortening my 27 pages writing and re-formating it to meet a journal > standards for publication would require investing yet another substantial > amount of time. I'm not sure I'm prepared for that, given that I've no > personal interest in a journal publication: I'm not an academic, I'm not > pursuing a career... > But I promise I'll think about reconsidering my position on this ;-) > Please do think about reconsidering. There are several reasons to
Re: RFR: 4511638: Double.toString(double) sometimes produces incorrect results [v2]
On Fri, 16 Apr 2021 11:30:32 GMT, Raffaello Giulietti wrote: >> Hello, >> >> here's a PR for a patch submitted on March 2020 >> [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was >> a thing. >> >> The patch has been edited to adhere to OpenJDK code conventions about >> multi-line (block) comments. Nothing in the code proper has changed, except >> for the addition of redundant but clarifying parentheses in some expressions. >> >> >> Greetings >> Raffaello > > Raffaello Giulietti has updated the pull request incrementally with one > additional commit since the last revision: > > 4511638: Double.toString(double) sometimes produces incorrect results (Guy Steele reply to a previous comment of mine) Yes, thank you, I stated my suggested criterion incorrectly. —Guy On Oct 11, 2021, at 4:16 AM, Raffaello Giulietti ***@***.**@***.***>> wrote:> Hi Guy, > > while implementing the additional test recommended in your point (2), it > occurred to me that the numbers of the form y 10^n, y in D_k (k = 1, 2, 3, 4) > end up being of the form y' 10^n', where y' = y / 10^k, n' = n + k, plus the > 2 * Y values around these. Such numbers do not seem to show any special > structure worth a dedicated test, so I'm wondering if you mean something else > instead. > > Perhaps you mean y to have at most 4 digits, i.e., 0 <= y < 10^4? > > Greetings > Raffaello > > P.S. The test recommended in point (1) pass successfully. > - PR: https://git.openjdk.java.net/jdk/pull/3402
RFR: 8275197: Remove unused fields in ThaiBuddhistChronology
Remove 3 unused HashMap's. Reported here https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-September/081866.html I did the similar PR to treetenbp and it was merged https://github.com/ThreeTen/threetenbp/pull/155 - Commit messages: - [PATCH] Remove unused fields in ThaiBuddhistChronology Changes: https://git.openjdk.java.net/jdk/pull/5917/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5917=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8275197 Stats: 37 lines in 1 file changed: 0 ins; 36 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/5917.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5917/head:pull/5917 PR: https://git.openjdk.java.net/jdk/pull/5917
