RFR: 8275386: Change nested classes in jdk.jlink to static nested classes
Non-static classes hold a link to their parent classes, which can be avoided. - Commit messages: - [PATCH] Change nested classes in jdk.jlink to static nested classes Changes: https://git.openjdk.java.net/jdk/pull/5983/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5983&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8275386 Stats: 6 lines in 3 files changed: 0 ins; 0 del; 6 mod Patch: https://git.openjdk.java.net/jdk/pull/5983.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5983/head:pull/5983 PR: https://git.openjdk.java.net/jdk/pull/5983
Re: RFR: 8266936: Add a finalization JFR event [v19]
> 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 incrementally with one additional commit since the last revision: relax memory ordering constraint - Changes: - all: https://git.openjdk.java.net/jdk/pull/4731/files - new: https://git.openjdk.java.net/jdk/pull/4731/files/d10eb309..85a46263 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4731&range=18 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4731&range=17-18 Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 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
Re: RFR: 8233674: JarURLConnection.getJarFile throws Exception if some process is holding the file
On Mon, 27 Sep 2021 08:04:23 GMT, Alan Bateman wrote: >> It’s a good idea to ask the Microsoft folks about that, but I don't know the >> way to ask. Could you tell me how to do it? >> >> As you say, CreateFile function is used in other parts of the JDK, but it >> have a significant impact to fix all of that. Therefore, I fixed the problem >> about opening zip files and jar files which is reported in the JBS. > >> It’s a good idea to ask the Microsoft folks about that, but I don't know the >> way to ask. Could you tell me how to do it? >> >> As you say, CreateFile function is used in other parts of the JDK, but it >> have a significant impact to fix all of that. Therefore, I fixed the problem >> about opening zip files and jar files which is reported in the JBS. > > I think we need to find out if the issue you are working around is a > bug/issue with the virus checker or that Microsoft recommends that all > applications should put a retry to work around these issues. As regards the > patch then it is incomplete. If we are forced to put a workaround into the > JDK code then I think it will have to do everywhere, not just for zip/JAR > files. @AlanBateman Could you reply to the above comment? - PR: https://git.openjdk.java.net/jdk/pull/5460
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 (@AlanBateman ) Could you tell me how many more days do you need for your confirmation? - PR: https://git.openjdk.java.net/jdk/pull/5679
Re: RFR: 8233674: JarURLConnection.getJarFile throws Exception if some process is holding the file
On Mon, 27 Sep 2021 08:04:23 GMT, Alan Bateman wrote: >> It’s a good idea to ask the Microsoft folks about that, but I don't know the >> way to ask. Could you tell me how to do it? >> >> As you say, CreateFile function is used in other parts of the JDK, but it >> have a significant impact to fix all of that. Therefore, I fixed the problem >> about opening zip files and jar files which is reported in the JBS. > >> It’s a good idea to ask the Microsoft folks about that, but I don't know the >> way to ask. Could you tell me how to do it? >> >> As you say, CreateFile function is used in other parts of the JDK, but it >> have a significant impact to fix all of that. Therefore, I fixed the problem >> about opening zip files and jar files which is reported in the JBS. > > I think we need to find out if the issue you are working around is a > bug/issue with the virus checker or that Microsoft recommends that all > applications should put a retry to work around these issues. As regards the > patch then it is incomplete. If we are forced to put a workaround into the > JDK code then I think it will have to do everywhere, not just for zip/JAR > files. > @AlanBateman Could you reply to the above comment? I view this issue as something for the anti-virus vendor to fix. It would be very disappointing to force the JDK to put in a workaround for this. A workaround would be very intrusive and require changes in many areas of the JDK. - PR: https://git.openjdk.java.net/jdk/pull/5460
Re: RFR: 8245095: Implementation of JEP 408: Simple Web Server [v9]
On Tue, 12 Oct 2021 10:40:15 GMT, Julia Boes wrote:
>> This change implements a simple web server that can be run on the
>> command-line with `java -m jdk.httpserver`.
>>
>> This is facilitated by adding an entry point for the `jdk.httpserver`
>> module, an implementation class whose main method is run when the above
>> command is executed. This is the first such module entry point in the JDK.
>>
>> The server is a minimal HTTP server that serves the static files of a given
>> directory, similar to existing alternatives on other platforms and
>> convenient for testing, development, and debugging.
>>
>> Additionally, a small API is introduced for programmatic creation and
>> customization.
>>
>> Testing: tier1-3.
>
> Julia Boes has updated the pull request with a new target base due to a merge
> or a rebase. The pull request now contains 24 commits:
>
> - Minor rewording of bind address output
> - Merge branch 'master' into simpleserver
> - Merge branch 'master' into simpleserver
> - update output for all interfaces
> - use ipv4/ipv6 specific loopback address and add add how-to output for all
> interfaces
> - Merge branch 'master' into simpleserver
> - change default bind address from anylocal to loopback
> - address PR comments
> - Merge branch 'master' into simpleserver
> - Merge remote-tracking branch 'origin/simpleserver' into simpleserver
> - ... and 14 more:
> https://git.openjdk.java.net/jdk/compare/b1b66965...e86609d0
test/jdk/com/sun/net/httpserver/simpleserver/ServerMimeTypesResolutionTest.java
line 200:
> 198: */
> 199: //@Test(dataProvider = "commonExtensions")
> 200: public static void testCommonExtensions(String extension) {
Is this test and the one below supposed to be commented out?
-
PR: https://git.openjdk.java.net/jdk/pull/5505
Re: RFR: 8245095: Implementation of JEP 408: Simple Web Server [v9]
On Mon, 18 Oct 2021 11:38:22 GMT, Michael McMahon wrote:
>> Julia Boes has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains 24 commits:
>>
>> - Minor rewording of bind address output
>> - Merge branch 'master' into simpleserver
>> - Merge branch 'master' into simpleserver
>> - update output for all interfaces
>> - use ipv4/ipv6 specific loopback address and add add how-to output for all
>> interfaces
>> - Merge branch 'master' into simpleserver
>> - change default bind address from anylocal to loopback
>> - address PR comments
>> - Merge branch 'master' into simpleserver
>> - Merge remote-tracking branch 'origin/simpleserver' into simpleserver
>> - ... and 14 more:
>> https://git.openjdk.java.net/jdk/compare/b1b66965...e86609d0
>
> test/jdk/com/sun/net/httpserver/simpleserver/ServerMimeTypesResolutionTest.java
> line 200:
>
>> 198: */
>> 199: //@Test(dataProvider = "commonExtensions")
>> 200: public static void testCommonExtensions(String extension) {
>
> Is this test and the one below supposed to be commented out?
Yes, `testCommonExtensions` is a manual test to find common mime types that are
currently not supported, `printUnsupportedFileExtensions` prints the list of
types found.
-
PR: https://git.openjdk.java.net/jdk/pull/5505
Integrated: 8274346: Support for additional content in an app-image.
On Thu, 30 Sep 2021 18:51:49 GMT, Andy Herrick wrote: > 8274346: Support for additional content in an app-image. This pull request has now been integrated. Changeset: d548f2fc Author:Andy Herrick URL: https://git.openjdk.java.net/jdk/commit/d548f2fc0dbc9e7864dd1701873bbf3d12a75ecb Stats: 167 lines in 10 files changed: 157 ins; 0 del; 10 mod 8274346: Support for additional content in an app-image. Reviewed-by: asemenyuk, almatvee - PR: https://git.openjdk.java.net/jdk/pull/5780
RFR: 8269336: Malformed jdk.serialFilter incorrectly handled
Can I please get a review for this change which addresses https://bugs.openjdk.java.net/browse/JDK-8269336? As noted in that issue, this change will now propagate any exception that occurred during parsing and creation of the filter configured through the `jdk.serialFilter` system property. It will also continue to log those errors, like it previously did. A new jtreg test has been introduced to reproduce this issue and verify the fix. Given that invalid values for this system property will now start throwing exception, will this change need a CSR? - Commit messages: - 8269336: Malformed jdk.serialFilter incorrectly handled Changes: https://git.openjdk.java.net/jdk/pull/5988/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5988&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8269336 Stats: 90 lines in 2 files changed: 89 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/5988.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5988/head:pull/5988 PR: https://git.openjdk.java.net/jdk/pull/5988
Re: RFR: 8245095: Implementation of JEP 408: Simple Web Server [v9]
On Tue, 12 Oct 2021 10:40:15 GMT, Julia Boes wrote: >> This change implements a simple web server that can be run on the >> command-line with `java -m jdk.httpserver`. >> >> This is facilitated by adding an entry point for the `jdk.httpserver` >> module, an implementation class whose main method is run when the above >> command is executed. This is the first such module entry point in the JDK. >> >> The server is a minimal HTTP server that serves the static files of a given >> directory, similar to existing alternatives on other platforms and >> convenient for testing, development, and debugging. >> >> Additionally, a small API is introduced for programmatic creation and >> customization. >> >> Testing: tier1-3. > > Julia Boes has updated the pull request with a new target base due to a merge > or a rebase. The pull request now contains 24 commits: > > - Minor rewording of bind address output > - Merge branch 'master' into simpleserver > - Merge branch 'master' into simpleserver > - update output for all interfaces > - use ipv4/ipv6 specific loopback address and add add how-to output for all > interfaces > - Merge branch 'master' into simpleserver > - change default bind address from anylocal to loopback > - address PR comments > - Merge branch 'master' into simpleserver > - Merge remote-tracking branch 'origin/simpleserver' into simpleserver > - ... and 14 more: > https://git.openjdk.java.net/jdk/compare/b1b66965...e86609d0 Marked as reviewed by michaelm (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5505
Re: RFR: 8271820: Implementation of JEP 416: Reimplement Core Reflection with Method Handle [v14]
> 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&page=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: Separate paramFlgas into paramCount and flags fields - Changes: - all: https://git.openjdk.java.net/jdk/pull/5027/files - new: https://git.openjdk.java.net/jdk/pull/5027/files/c25d651a..c1bb48fe Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5027&range=13 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5027&range=12-13 Stats: 36 lines in 2 files changed: 9 ins; 13 del; 14 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: 8266936: Add a finalization JFR event [v10]
On Fri, 27 Aug 2021 15:23:35 GMT, Markus Grönlund wrote:
>> 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 incrementally with three
> additional commits since the last revision:
>
> - localize
> - cleanup
> - FinalizerStatistics
Marked as reviewed by egahlin (Reviewer).
src/java.base/share/classes/java/lang/ref/Finalizer.java line 71:
> 69: }
> 70:
> 71:
extraneous whitespace?
test/jdk/jdk/jfr/event/runtime/TestFinalizerStatisticsEvent.java line 98:
> 96: case TEST_CLASS_NAME: {
> 97:
> Asserts.assertTrue(event.getString("codeSource").startsWith("file://"));
> 98: foundTestClassName = true;
Could we (sanity) check "objects" and "totalFinalzersRun" fields as well?
-
PR: https://git.openjdk.java.net/jdk/pull/4731
Re: RFR: 8266936: Add a finalization JFR event [v10]
On Tue, 31 Aug 2021 08:42:58 GMT, Erik Gahlin wrote:
>> Markus Grönlund has updated the pull request incrementally with three
>> additional commits since the last revision:
>>
>> - localize
>> - cleanup
>> - FinalizerStatistics
>
> test/jdk/jdk/jfr/event/runtime/TestFinalizerStatisticsEvent.java line 98:
>
>> 96: case TEST_CLASS_NAME: {
>> 97:
>> Asserts.assertTrue(event.getString("codeSource").startsWith("file://"));
>> 98: foundTestClassName = true;
>
> Could we (sanity) check "objects" and "totalFinalzersRun" fields as well?
It's risky to do because of the non-deterministic nature of when the Finalizer
thread runs (if at all). The best I could think of is to check if either field
is 0 or more, but that becomes so weak it's not much of a sanity check.
-
PR: https://git.openjdk.java.net/jdk/pull/4731
Re: RFR: 8266936: Add a finalization JFR event [v10]
On Tue, 31 Aug 2021 08:45:54 GMT, Erik Gahlin wrote: >> Markus Grönlund has updated the pull request incrementally with three >> additional commits since the last revision: >> >> - localize >> - cleanup >> - FinalizerStatistics > > src/java.base/share/classes/java/lang/ref/Finalizer.java line 71: > >> 69: } >> 70: >> 71: > > extraneous whitespace? I think this version is outdated, and the extra whitespace was removed in later versions. - PR: https://git.openjdk.java.net/jdk/pull/4731
Re: RFR: 8266936: Add a finalization JFR event [v12]
On Fri, 24 Sep 2021 22:31:18 GMT, Mandy Chung wrote: >> Markus Grönlund has updated the pull request incrementally with one >> additional commit since the last revision: >> >> no precompiled headers and mtServiceability nmt classification > > Hi Markus, > > It's a little surprised to see Finalizer.c to depend JMM interface which is > used by `java.management` and `jdk.management` modules only. It's more > appropriate for it to be a JVM_* entry point for Finalizer to report > completion of the finalization instead.I understand that you want to make > FinalizerService to be a conditional feature which is a good idea. Such JVM > entry can be made as a nop if not INCLUDE_SERVICES. Thank you for staying around with this protracted PR. Thanks @mlchung , @coleenp and @egahlin for your reviews! - PR: https://git.openjdk.java.net/jdk/pull/4731
Re: RFR: 8266936: Add a finalization JFR event [v20]
> 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 incrementally with one additional commit since the last revision: no constexpr for constant values - Changes: - all: https://git.openjdk.java.net/jdk/pull/4731/files - new: https://git.openjdk.java.net/jdk/pull/4731/files/85a46263..b3268c90 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4731&range=19 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4731&range=18-19 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 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: 8266936: Add a finalization JFR event
On Thu, 8 Jul 2021 19:47:26 GMT, Markus Grönlund wrote: > 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 This pull request has now been integrated. Changeset: 72a976ef Author:Markus Grönlund URL: https://git.openjdk.java.net/jdk/commit/72a976ef05fc2c62657920a560a0abc60b27c852 Stats: 1917 lines in 36 files changed: 1375 ins; 409 del; 133 mod 8266936: Add a finalization JFR event Reviewed-by: coleenp, mchung, egahlin - PR: https://git.openjdk.java.net/jdk/pull/4731
Re: RFR: 8274544: Langtools command's usage were garbled on Japanese Windows
On Thu, 14 Oct 2021 23:43:47 GMT, Naoto Sato wrote:
>> @takiguc - if JShell is still an issue, is there a chance you could try this:
>> https://github.com/lahodaj/jdk/commit/cfa6b3eebbc22c5a48d31cfd692ff98690653686
>>
>> Not sure if it will help, but it might (this won't change the default
>> charset, but could send the output in a sensible encoding for the console.
>>
>> Thanks!
>
> I believe both @lahodaj and my proposed changes are needed, which I provided
> here:
> https://github.com/openjdk/jdk/commit/82a9033953655042480c90d388fcbc8053fc5ff5
> Can you check this works?
Hello @naotoj .
[82a9033](https://github.com/openjdk/jdk/commit/82a9033953655042480c90d388fcbc8053fc5ff5)
worked on Windows platform, then I got your point.
To minimize changes, I think I should create new method and call from
JShellToolBuilder.java and JShellToolProvider.java.
static PrintStream derivePrintStream(PrintStream ps) {
if (Charset.defaultCharset().equals(NATIVE_CHARSET)) {
return ps;
} else {
return new PrintStream(new WriterOutputStream(
new OutputStreamWriter(ps, NATIVE_CHARSET),
Charset.defaultCharset()));
}
}
Additionally, I tested this issue on non-UTF-8 platform like RHEL7/CentOS7 EUC
locale (ja_JP.eucjp).
It seemed JLine's terminal's encoding was still UTF-8.
I could not input Japanese character as expected, also Cut & Paste with
Japanese string did not work.
In my investigation, Charset.defaultCharset() was used on AbstractTerminal.java
https://github.com/openjdk/jdk/blob/master/src/jdk.internal.le/share/classes/jdk/internal/org/jline/terminal/impl/AbstractTerminal.java#L55-L62
I should be like,
this.encoding = encoding != null ? encoding : nativeCharset;
(JLine was upgraded by b19, I need to rebase if it's required)
One more thing,
For Inter-Process Communication between jshell front-end and agent, I think
default charset should be same for non UTF-8 environment.
I think JdiInitiator.java should be modified.
Please give me your suggestion.
-
PR: https://git.openjdk.java.net/jdk/pull/5771
RFR: 7008363: TEST_BUG: test/java/lang/StringCoding/CheckEncodings.sh does nothing and is very slow at that
Removing a problem-listed test case, which has little value in itself. Confirmed it did succeed on all platforms before the removal. - Commit messages: - 7008363: TEST_BUG: test/java/lang/StringCoding/CheckEncodings.sh does nothing and is very slow at that Changes: https://git.openjdk.java.net/jdk/pull/5996/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5996&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-7008363 Stats: 96 lines in 3 files changed: 0 ins; 96 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/5996.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5996/head:pull/5996 PR: https://git.openjdk.java.net/jdk/pull/5996
Re: RFR: 7008363: TEST_BUG: test/java/lang/StringCoding/CheckEncodings.sh does nothing and is very slow at that
On Mon, 18 Oct 2021 20:49:07 GMT, Naoto Sato wrote: > Removing a problem-listed test case, which has little value in itself. > Confirmed it did succeed on all platforms before the removal. Marked as reviewed by iris (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5996
Re: RFR: 7008363: TEST_BUG: test/java/lang/StringCoding/CheckEncodings.sh does nothing and is very slow at that
On Mon, 18 Oct 2021 20:49:07 GMT, Naoto Sato wrote: > Removing a problem-listed test case, which has little value in itself. > Confirmed it did succeed on all platforms before the removal. Marked as reviewed by lancea (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5996
Re: RFR: 7008363: TEST_BUG: test/java/lang/StringCoding/CheckEncodings.sh does nothing and is very slow at that
On Mon, 18 Oct 2021 20:49:07 GMT, Naoto Sato wrote: > Removing a problem-listed test case, which has little value in itself. > Confirmed it did succeed on all platforms before the removal. Marked as reviewed by bpb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5996
Re: RFR: 8274544: Langtools command's usage were garbled on Japanese Windows [v3]
On Wed, 6 Oct 2021 05:04:28 GMT, Ichiroh Takiguchi wrote: >> JEP-400 (UTF-8 by Default) was eabled on JDK18-b13. >> After JDK18-b13, javac and some other langtool command's usage were garbled >> on Japanese Windows. >> These commands use PrintWriter instead of standard out/err with PrintStream. > > 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 this.encoding = encoding != null ? encoding : nativeCharset; I am not familiar with `JLine`, but if it is working in a Console, it should refer to `Console.charset()` first. As to `JdiInitiator`, not sure I understand the situation. Would you mind providing some examples that would cause problems? - PR: https://git.openjdk.java.net/jdk/pull/5771
Re: RFR: 8271515: Integration of JEP 417: Vector API (Third Incubator) [v3]
On Sat, 16 Oct 2021 00:56:14 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.
>
> Paul Sandoz has updated the pull request with a new target base due to a
> merge or a rebase. The pull request now contains seven commits:
>
> - Merge branch 'master' into JDK-8271515-vector-api
> - Apply patch from https://github.com/openjdk/panama-vector/pull/152
> - 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)
src/hotspot/share/utilities/globalDefinitions.hpp line 36:
> 34:
> 35: #include COMPILER_HEADER(utilities/globalDefinitions)
> 36: #include "utilities/globalDefinitions_vecApi.hpp"
This change is not needed.
src/hotspot/share/utilities/globalDefinitions_vecApi.hpp line 29:
> 27: // the intent of this file to provide a header that can be included in .s
> files.
> 28:
> 29: #ifndef SHARE_VM_UTILITIES_GLOBALDEFINITIONS_VECAPI_HPP
The file src/hotspot/share/utilities/globalDefinitions_vecApi.hpp is not needed.
src/jdk.incubator.vector/share/classes/jdk/incubator/vector/AbstractMask.java
line 67:
> 65:
> 66: @Override
> 67: public boolean laneIsSet(int i) {
Missing ForceInline.
src/jdk.incubator.vector/share/classes/jdk/incubator/vector/Byte128Vector.java
line 278:
> 276: @Override
> 277: @ForceInline
> 278: public Byte128Vector lanewise(Unary op, VectorMask m) {
Should this method be final as well?
src/jdk.incubator.vector/share/classes/jdk/incubator/vector/Byte128Vector.java
line 290:
> 288: @Override
> 289: @ForceInline
> 290: public Byte128Vector lanewise(Binary op, Vector v,
> VectorMask m) {
Should this method be final as well?
src/jdk.incubator.vector/share/classes/jdk/incubator/vector/Byte128Vector.java
line 313:
> 311: public final
> 312: Byte128Vector
> 313: lanewise(Ternary op, Vector v1, Vector v2) {
For unary and binary operator above, we use VectorOperators.Unary and
VectorOperators.Binary.
Should we use VectorOperators.Ternary here as well then?
src/jdk.incubator.vector/share/classes/jdk/incubator/vector/Byte128Vector.java
line 321:
> 319: public final
> 320: Byte128Vector
> 321: lanewise(Ternary op, Vector v1, Vector v2,
> VectorMask m) {
Should we use VectorOperators.Ternary here?
src/jdk.incubator.vector/share/classes/jdk/incubator/vector/Byte128Vector.java
line 731:
> 729: @Override
> 730: @ForceInline
> 731: public long toLong() {
Should this and other mask operation methods be final methods?
src/jdk.incubator.vector/share/classes/jdk/incubator/vector/ByteVector.java
line 603:
> 601: if (opKind(op, VO_SPECIAL)) {
> 602: if (op == ZOMO) {
> 603: return blend(broadcast(-1), compare(NE, 0, m));
This doesn't look correct. The lanes where mask is false should get the
original lane value in this vector.
-
PR: https://git.openjdk.java.net/jdk/pull/5873
Re: RFR: 8274544: Langtools command's usage were garbled on Japanese Windows [v3]
On Wed, 6 Oct 2021 05:04:28 GMT, Ichiroh Takiguchi wrote: >> JEP-400 (UTF-8 by Default) was eabled on JDK18-b13. >> After JDK18-b13, javac and some other langtool command's usage were garbled >> on Japanese Windows. >> These commands use PrintWriter instead of standard out/err with PrintStream. > > 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 This is pretty ugly code to be replicating so many times. What if the tools have been run in an environment where `System.out` and `System.err` have already been redirected in some manner, with `System.setOut` or `System.setErr`? You should not assume that `System.out` and `System.err` will always refer to the console. - PR: https://git.openjdk.java.net/jdk/pull/5771
Re: RFR: 8271515: Integration of JEP 417: Vector API (Third Incubator) [v3]
On Wed, 13 Oct 2021 19:33:12 GMT, Vladimir Kozlov wrote: > C2 and x86 changes seems fine. Could any reviewer please help to review AArch64 changes in this patch? @theRealAph @adinn @nick-arm ? The AArch64 changes include: 1. SVE scalable predicate register allocation support 2. SVE backend support for vector masking operations with predicate feature - PR: https://git.openjdk.java.net/jdk/pull/5873
Re: RFR: 8275063: Implementation of Foreign Function & Memory API (Second incubator) [v6]
On Sat, 16 Oct 2021 11:11:59 GMT, Maurizio Cimadamore wrote: >> 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 two > additional commits since the last revision: > > - * use `invokeWithArguments` to simplify new test > - Add test for liveness check with high-aririty downcalls >(make sure that if an exception occurs in a downcall because of liveness, >ref count of other resources are left intact). Hotspot changes seem fine. Thanks, David - PR: https://git.openjdk.java.net/jdk/pull/5907
Re: RFR: 8273111: Default timezone should return zone ID if /etc/localtime is valid but not canonicalization on linux [v5]
> 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: fix code style - Changes: - all: https://git.openjdk.java.net/jdk/pull/5327/files - new: https://git.openjdk.java.net/jdk/pull/5327/files/93cff3d1..ba9cc656 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5327&range=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5327&range=03-04 Stats: 9 lines in 1 file changed: 0 ins; 6 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: 8273111: Default timezone should return zone ID if /etc/localtime is valid but not canonicalization on linux [v4]
On Fri, 15 Oct 2021 07:03:18 GMT, Hamlin Li wrote:
>> 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
>
> src/java.base/unix/native/libjava/TimeZone_md.c line 82:
>
>> 80:
>> 81: /*
>> 82: * remove repeated path separators ('/') in the giving 'path'.
>
> given?
Thanks, fix it.
> src/java.base/unix/native/libjava/TimeZone_md.c line 89:
>
>> 87: char *left = path;
>> 88: char *right = path;
>> 89: char *end = path + strlen(path);
>
> "char* end"? better to align with existing code style
OK, fixed it.
> src/java.base/unix/native/libjava/TimeZone_md.c line 95:
>
>> 93: for (; right < end; right++) {
>> 94: if (*right == '/' && *(right + 1) == '/') break;
>> 95: }
>
> Is this for loop necessary? Seems it's ok to merge with the nested loop below.
Thanks for your suggestion, this for loop is indeed unnecessary. Removed it.
-
PR: https://git.openjdk.java.net/jdk/pull/5327
Re: RFR: 8153490: Cannot setBytes() if incoming buffer's length is bigger than number of elements we want to insert. [v5]
On Fri, 15 Oct 2021 09:30:18 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. > > Mitsuru Kariya has updated the pull request incrementally with one additional > commit since the last revision: > > Follow the comment The pre-submit test seems to have failed because the compiler was not found in some environments. Should I take any action? Or should I issue the /integrate pull request command? - PR: https://git.openjdk.java.net/jdk/pull/4001
