Re: RFR: JDK-8284858: Start of release updates for JDK 20 [v6]
On Thu, 9 Jun 2022 01:03:47 GMT, Joe Darcy wrote: >> Time to start getting ready for JDK 20... > > Joe Darcy 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 44 additional commits since > the last revision: > > - Merge branch 'master' into JDK-8284858 > - Merge branch 'master' into JDK-8284858 > - Merge branch 'master' into JDK-8284858 > - Merge branch 'master' into JDK-8284858 > - Update symbols for JDK 19 b25. > - Merge branch 'master' into JDK-8284858 > - Adjust deprecated options test. > - Merge branch 'master' into JDK-8284858 > - Update deprecated options test. > - Merge branch 'master' into JDK-8284858 > - ... and 34 more: > https://git.openjdk.java.net/jdk/compare/705cfa36...1b29a17c Marked as reviewed by iris (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8236
Re: RFR: 8287971: Throw exception for missing values in .jpackage.xml
On Thu, 9 Jun 2022 01:21:30 GMT, Alexander Matveev wrote: > - Warning will be printed if .jpackage.xml has old version number. > - Error will be thrown if main-class and app-store values are missing from > .jpackage.xml. > - Both main-class and app-store are required values for JDK-8286850 and they > might be missing if user uses app images generated by previous jpackage > version. If some fields of .jpackage.xml are not valid, the entire file must be considered invalid, not only the invalid fields. This is the idea of `AppImageFile.isValid()` method. We do not support backward compatibility for .jpackage.xml for simplicity. We do support only the case when an application image supplied in `--app-image` parameter to jpackage was created by the same jpackage, and not by jpackage from another JDK or from the same JDK but on another platform. I'd discard this fix and create a new one that will: 1. Make `AppImageFile.getVersion()` return the value of `java.version` system property instead of hardcoded "1.0" value. 2. If `AppImageFile.isValid()` returns false, throw exception saying that .jpackage.xml is malformed instead of returning empty AppImageFile instance from `AppImageFile.load()`. - PR: https://git.openjdk.java.net/jdk/pull/9098
Integrated: 8284780: Need methods to create pre-sized HashSet and LinkedHashSet
On Tue, 19 Apr 2022 18:00:19 GMT, XenoAmess wrote: > as title. This pull request has now been integrated. Changeset: e01cd7c3 Author:XenoAmess Committer: Stuart Marks URL: https://git.openjdk.java.net/jdk/commit/e01cd7c3ed923cd19509fc972ba6e4aa2991289f Stats: 154 lines in 29 files changed: 107 ins; 7 del; 40 mod 8284780: Need methods to create pre-sized HashSet and LinkedHashSet Reviewed-by: naoto, bpb, dfuchs, ascarpino - PR: https://git.openjdk.java.net/jdk/pull/8302
RFR: 8287971: Throw exception for missing values in .jpackage.xml
- Warning will be printed if .jpackage.xml has old version number. - Error will be thrown if main-class and app-store values are missing from .jpackage.xml. - Both main-class and app-store are required values for JDK-8286850 and they might be missing if user uses app images generated by previous jpackage version. - Commit messages: - 8287971: Throw exception for missing values in .jpackage.xml Changes: https://git.openjdk.java.net/jdk/pull/9098/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=9098=00 Issue: https://bugs.openjdk.org/browse/JDK-8287971 Stats: 73 lines in 6 files changed: 56 ins; 9 del; 8 mod Patch: https://git.openjdk.java.net/jdk/pull/9098.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/9098/head:pull/9098 PR: https://git.openjdk.java.net/jdk/pull/9098
Re: RFR: JDK-8284858: Start of release updates for JDK 20 [v6]
> Time to start getting ready for JDK 20... Joe Darcy 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 44 additional commits since the last revision: - Merge branch 'master' into JDK-8284858 - Merge branch 'master' into JDK-8284858 - Merge branch 'master' into JDK-8284858 - Merge branch 'master' into JDK-8284858 - Update symbols for JDK 19 b25. - Merge branch 'master' into JDK-8284858 - Adjust deprecated options test. - Merge branch 'master' into JDK-8284858 - Update deprecated options test. - Merge branch 'master' into JDK-8284858 - ... and 34 more: https://git.openjdk.java.net/jdk/compare/530d16c1...1b29a17c - Changes: - all: https://git.openjdk.java.net/jdk/pull/8236/files - new: https://git.openjdk.java.net/jdk/pull/8236/files/e495a579..1b29a17c Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8236=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8236=04-05 Stats: 16423 lines in 539 files changed: 12382 ins; 2143 del; 1898 mod Patch: https://git.openjdk.java.net/jdk/pull/8236.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8236/head:pull/8236 PR: https://git.openjdk.java.net/jdk/pull/8236
Integrated: 8288068: Javadoc contains spurious reference to CLinker
On Wed, 8 Jun 2022 21:07:13 GMT, Maurizio Cimadamore wrote: > This simple patch fixes a bunch of references to "CLinker" and replaces them > with "Linker", following the API name changes. This pull request has now been integrated. Changeset: 65f0829d Author:Maurizio Cimadamore URL: https://git.openjdk.java.net/jdk/commit/65f0829d645fd988c6a208622b1f34bf9de08e60 Stats: 5 lines in 2 files changed: 0 ins; 0 del; 5 mod 8288068: Javadoc contains spurious reference to CLinker Reviewed-by: iris - PR: https://git.openjdk.java.net/jdk/pull/9094
Re: RFR: 8283726: x86_64 intrinsics for compareUnsigned method in Integer and Long [v2]
On Wed, 8 Jun 2022 09:39:23 GMT, Quan Anh Mai wrote: >> Hi, >> >> This patch implements intrinsics for `Integer/Long::compareUnsigned` using >> the same approach as the JVM does for long and floating-point comparisons. >> This allows efficient and reliable usage of unsigned comparison in Java, >> which is a basic operation and is important for range checks such as >> discussed in #8620 . >> >> Thank you very much. > > Quan Anh Mai has updated the pull request incrementally with two additional > commits since the last revision: > > - remove comments > - review comments Tier1-4 testing passed - no new failures. I suggest to push it into JDK 20 after fork and after you get second review. - PR: https://git.openjdk.java.net/jdk/pull/9068
Re: RFR: 8288068: Javadoc contains spurious reference to CLinker
On Wed, 8 Jun 2022 21:07:13 GMT, Maurizio Cimadamore wrote: > This simple patch fixes a bunch of references to "CLinker" and replaces them > with "Linker", following the API name changes. Marked as reviewed by iris (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/9094
Re: RFR: 8287186: JDK modules participating in preview [v2]
On Wed, 8 Jun 2022 18:24:35 GMT, Paul Sandoz wrote: >> Allow JDK modules that use preview features (preview language features or >> preview API features from dependent modules) to participate without the need >> to compile with `--enable-preview`. >> >> It's difficult to enable participation using an annotation due to the nature >> in which symbols are encountered when processing source as there is no >> guaranteed order to the processing of certain symbols. >> >> Instead a JDK module participates if the `java.base` package >> `jdk.internal.javac` is exported to that module (@lahodaj clever idea!). An >> internal annotation `jdk.internal.javac.ParticipatesInPreview` can be >> declared on the module. Such a declaration cannot be enforced but does by >> its use require the `jdk.internal.javac`'s export list to be updated. >> >> The modules `jdk.incubator.vector` and `jdk.incubator.concurrent` have been >> updated accordingly, both of which participate in preview APIs (APIs in >> `java.lang.foreign` and `Thread.ofVirtual`, respectively). > > Paul Sandoz has updated the pull request incrementally with one additional > commit since the last revision: > > Let java.management participate in preview features. Just curious, is it still up to incubator modules' discretion to avoid accidental user access to preview content via the modules without enabling preview, like the `PreviewFeatures.ensureEnabled()` in `StructuredTaskScope`? - PR: https://git.openjdk.java.net/jdk/pull/9087
RFR: 8288068: Javadoc contains spurious reference to CLinker
This simple patch fixes a bunch of references to "CLinker" and replaces them with "Linker", following the API name changes. - Commit messages: - Initial push Changes: https://git.openjdk.java.net/jdk/pull/9094/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=9094=00 Issue: https://bugs.openjdk.org/browse/JDK-8288068 Stats: 5 lines in 2 files changed: 0 ins; 0 del; 5 mod Patch: https://git.openjdk.java.net/jdk/pull/9094.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/9094/head:pull/9094 PR: https://git.openjdk.java.net/jdk/pull/9094
Re: RFR: 8284780: Need methods to create pre-sized HashSet and LinkedHashSet [v19]
On Wed, 8 Jun 2022 17:49:38 GMT, XenoAmess wrote: >> as title. > > XenoAmess has updated the pull request incrementally with one additional > commit since the last revision: > > clean up Calendar I gave a quick look at the security files touched and seems straightforward. I didn't see any problems - Marked as reviewed by ascarpino (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8302
Re: RFR: 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved flags is used [v5]
On Wed, 8 Jun 2022 14:51:59 GMT, Thiago Henrique Hüpner wrote: >> 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved >> flags is used > > Thiago Henrique Hüpner has updated the pull request incrementally with one > additional commit since the last revision: > > Fix test message Marked as reviewed by lancea (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/9049
Re: RFR: 8284780: Need methods to create pre-sized HashSet and LinkedHashSet [v19]
On Wed, 8 Jun 2022 17:49:38 GMT, XenoAmess wrote: >> as title. > > XenoAmess has updated the pull request incrementally with one additional > commit since the last revision: > > clean up Calendar Running tests and awaiting review from security team. Our internal test system is backlogged and tests might not complete in time to get into JDK 19. - PR: https://git.openjdk.java.net/jdk/pull/8302
Re: RFR: 8287186: JDK modules participating in preview [v2]
On Wed, 8 Jun 2022 18:24:35 GMT, Paul Sandoz wrote: >> Allow JDK modules that use preview features (preview language features or >> preview API features from dependent modules) to participate without the need >> to compile with `--enable-preview`. >> >> It's difficult to enable participation using an annotation due to the nature >> in which symbols are encountered when processing source as there is no >> guaranteed order to the processing of certain symbols. >> >> Instead a JDK module participates if the `java.base` package >> `jdk.internal.javac` is exported to that module (@lahodaj clever idea!). An >> internal annotation `jdk.internal.javac.ParticipatesInPreview` can be >> declared on the module. Such a declaration cannot be enforced but does by >> its use require the `jdk.internal.javac`'s export list to be updated. >> >> The modules `jdk.incubator.vector` and `jdk.incubator.concurrent` have been >> updated accordingly, both of which participate in preview APIs (APIs in >> `java.lang.foreign` and `Thread.ofVirtual`, respectively). > > Paul Sandoz has updated the pull request incrementally with one additional > commit since the last revision: > > Let java.management participate in preview features. javac + `jdk.internal.javac` changes look good to me. - Marked as reviewed by jlahoda (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/9087
Re: RFR: JDK-8266670: Better modeling of access flags in core reflection [v20]
On Wed, 25 May 2022 00:35:24 GMT, Joe Darcy wrote:
>> This is an early review of changes to better model JVM access flags, that is
>> "modifiers" like public, protected, etc. but explicitly at a VM level.
>>
>> Language level modifiers and JVM level access flags are closely related, but
>> distinct. There are concepts that overlap in the two domains (public,
>> private, etc.), others that only have a language-level modifier (sealed),
>> and still others that only have an access flag (synthetic).
>>
>> The existing java.lang.reflect.Modifier class is inadequate to model these
>> subtleties. For example, the bit positions used by access flags on different
>> kinds of elements overlap (such as "volatile" for fields and "bridge" for
>> methods. Just having a raw integer does not provide sufficient context to
>> decode the corresponding language-level string. Methods like
>> Modifier.methodModifiers() were introduced to cope with this situation.
>>
>> With additional modifiers and flags on the horizon with projects like
>> Valhalla, addressing the existent modeling deficiency now ahead of time is
>> reasonable before further strain is introduced.
>>
>> This PR in its current form is meant to give the overall shape of the API.
>> It is missing implementations to map from, say, method modifiers to access
>> flags, taking into account overlaps in bit positions.
>>
>> The CSR https://bugs.openjdk.java.net/browse/JDK-8281660 will be filled in
>> once the API is further along.
>
> Joe Darcy 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 32 additional commits since
> the last revision:
>
> - Target JDK 20 rather than 19.
> - Merge branch 'master' into JDK-8266670
> - Add mask values to constants' javadoc.
> - Implement review feedback from mlchung.
> - Fix type in @throws tag.
> - Merge branch 'master' into JDK-8266670
> - Respond to review feedback.
> - Merge branch 'master' into JDK-8266670
> - Make workding changes suggested in review feedback.
> - Merge branch 'master' into JDK-8266670
> - ... and 22 more:
> https://git.openjdk.java.net/jdk/compare/1b91dbd2...05cf2d8b
Testing a mask for one AccessFlag looks like:
if ((modifiers & AccessFlag.STRICTFP.mask()) != 0) { ... }
A method on AccessFlag to test if its mask value is present in a 32-bit value
would/could make the code easier to read and write.
if (AccessFlag.STRICTFP.isPresent(modifiers)) { ...}
The method name should be chosen to make the test clear and readable.
-
PR: https://git.openjdk.java.net/jdk/pull/7445
Re: RFR: 8287186: JDK modules participating in preview [v2]
On Wed, 8 Jun 2022 18:24:35 GMT, Paul Sandoz wrote: >> Allow JDK modules that use preview features (preview language features or >> preview API features from dependent modules) to participate without the need >> to compile with `--enable-preview`. >> >> It's difficult to enable participation using an annotation due to the nature >> in which symbols are encountered when processing source as there is no >> guaranteed order to the processing of certain symbols. >> >> Instead a JDK module participates if the `java.base` package >> `jdk.internal.javac` is exported to that module (@lahodaj clever idea!). An >> internal annotation `jdk.internal.javac.ParticipatesInPreview` can be >> declared on the module. Such a declaration cannot be enforced but does by >> its use require the `jdk.internal.javac`'s export list to be updated. >> >> The modules `jdk.incubator.vector` and `jdk.incubator.concurrent` have been >> updated accordingly, both of which participate in preview APIs (APIs in >> `java.lang.foreign` and `Thread.ofVirtual`, respectively). > > Paul Sandoz has updated the pull request incrementally with one additional > commit since the last revision: > > Let java.management participate in preview features. The updates to java.base, java.management, and jdk.incubator.* looks fine, it's good to have the reflection code go away. - Marked as reviewed by alanb (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/9087
Re: RFR: 8287186: JDK modules participating in preview
On Wed, 8 Jun 2022 17:21:08 GMT, Alan Bateman wrote: > Can java.management participate too? It would allow > sun.management.Util.isVirtual(Thread) to go away (lots of methods in > sun.management.ThreadImpl need to test if a thread is virtual). Pushed update. - PR: https://git.openjdk.java.net/jdk/pull/9087
Re: RFR: 8287186: JDK modules participating in preview [v2]
> Allow JDK modules that use preview features (preview language features or > preview API features from dependent modules) to participate without the need > to compile with `--enable-preview`. > > It's difficult to enable participation using an annotation due to the nature > in which symbols are encountered when processing source as there is no > guaranteed order to the processing of certain symbols. > > Instead a JDK module participates if the `java.base` package > `jdk.internal.javac` is exported to that module (@lahodaj clever idea!). An > internal annotation `jdk.internal.javac.ParticipatesInPreview` can be > declared on the module. Such a declaration cannot be enforced but does by its > use require the `jdk.internal.javac`'s export list to be updated. > > The modules `jdk.incubator.vector` and `jdk.incubator.concurrent` have been > updated accordingly, both of which participate in preview APIs (APIs in > `java.lang.foreign` and `Thread.ofVirtual`, respectively). Paul Sandoz has updated the pull request incrementally with one additional commit since the last revision: Let java.management participate in preview features. - Changes: - all: https://git.openjdk.java.net/jdk/pull/9087/files - new: https://git.openjdk.java.net/jdk/pull/9087/files/5e7ca855..9defdf23 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=9087=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=9087=00-01 Stats: 44 lines in 5 files changed: 4 ins; 29 del; 11 mod Patch: https://git.openjdk.java.net/jdk/pull/9087.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/9087/head:pull/9087 PR: https://git.openjdk.java.net/jdk/pull/9087
Re: RFR: 8283726: x86_64 intrinsics for compareUnsigned method in Integer and Long [v2]
On Wed, 8 Jun 2022 09:39:23 GMT, Quan Anh Mai wrote: >> Hi, >> >> This patch implements intrinsics for `Integer/Long::compareUnsigned` using >> the same approach as the JVM does for long and floating-point comparisons. >> This allows efficient and reliable usage of unsigned comparison in Java, >> which is a basic operation and is important for range checks such as >> discussed in #8620 . >> >> Thank you very much. > > Quan Anh Mai has updated the pull request incrementally with two additional > commits since the last revision: > > - remove comments > - review comments Good. I submitted testing. You need second review. - Marked as reviewed by kvn (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/9068
Re: RFR: 8284780: Need methods to create pre-sized HashSet and LinkedHashSet [v19]
> as title. XenoAmess has updated the pull request incrementally with one additional commit since the last revision: clean up Calendar - Changes: - all: https://git.openjdk.java.net/jdk/pull/8302/files - new: https://git.openjdk.java.net/jdk/pull/8302/files/bacc9ca8..95d59b97 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8302=18 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8302=17-18 Stats: 8 lines in 1 file changed: 0 ins; 7 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/8302.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8302/head:pull/8302 PR: https://git.openjdk.java.net/jdk/pull/8302
Re: RFR: 8284780: Need methods to create pre-sized HashSet and LinkedHashSet [v17]
On Wed, 1 Jun 2022 18:26:17 GMT, Naoto Sato wrote:
>> XenoAmess has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> do it as naotoj said
>
> src/java.base/share/classes/java/util/Calendar.java line 2648:
>
>> 2646: set.add("gregory");
>> 2647: set.add("buddhist");
>> 2648: set.add("japanese");
>
> This can be replaced with `SET = Set.of("gregory", "buddhist", "japanese");`.
@naotoj Yes it can. I did a further clean up to it, please have a look.
-
PR: https://git.openjdk.java.net/jdk/pull/8302
Re: RFR: 8287902: UnreadableRB case in MissingResourceCauseTest is not working reliably on Windows
On Tue, 7 Jun 2022 12:19:29 GMT, Magnus Ihse Bursie wrote: > The test > `test/jdk/java/util/ResourceBundle/Control/MissingResourceCauseTest.java` > verifies different failure modes of resource bundles. One of the failures is > that the runner class, `MissingResourceCauseTestRun.java`, creates a file > `UnreadableRB`, and runs `chmod 000` on it, to make it unreadable by the > test. Then MissingResourceCauseTest is called, and the `UnreadableRB` file is > removed. > > This does not work reliably on Windows. On msys2, `chmod` is essentially a > no-op, so the file is not made unreadable, and hence the test fails. In my > personal cygwin test environment, the chmod command does have some effect, > but it is still not enough to make the file unreadable, and so the test fails. > > The test was originally a shell script test that got converted to Java in > [JDK-4354216](https://bugs.openjdk.org/browse/JDK-8213127). The original > shell script code explicitly excluded Windows from testing. This was changed > in the rewrite, for reasons I cannot determine. > > What suprises me, though, is the "how can this ever has worked???" factor. > Apparently the test passes on the current Cygwin setup on GHA. I have failed > to reproduce the working conditions that makes a file actually unreadable for > the owner on Windows, neither on my GHA test repo, nor locally. I've searched > the web to figure out how to properly set file permissions on Windows to make > the file unreadable, using Windows native tools, to no avail. I've even asked > a [Stack Overflow > question](https://stackoverflow.com/questions/72528318/what-file-permissions-make-a-file-unreadable-by-owner-in-windows); > which as of yet is still unanswered. > > Since I feel I've spent far more time than reasonable trying to get this to > work properly, I suggest we instead skip the unreadable test on Windows. It > is clearly unstable and highly depending on the Windows environment, the test > was never originally supported or intended for Windows, and at the of the > day, testing file unreadability is not an important regression test for > [JDK-4354216](https://bugs.openjdk.org/browse/JDK-4354216). Thanks for the investigation, Magnus. I believe it is a bug to include the Windows environment in refactoring the shell script to Java. Why it's been working? No idea. Could be `IOException` has been thrown not by `unreadable` but for some other reason? - PR: https://git.openjdk.java.net/jdk/pull/9061
Re: RFR: 8284780: Need methods to create pre-sized HashSet and LinkedHashSet [v17]
On Wed, 1 Jun 2022 17:34:04 GMT, Stuart Marks wrote: >> XenoAmess has updated the pull request incrementally with one additional >> commit since the last revision: >> >> do it as naotoj said > > test/jdk/java/util/HashMap/WhiteBoxResizeTest.java line 441: > >> 439: } >> 440: } >> 441: > > This unifies the test cases between the Set and Map factories, which > accomplishes the primary goal. Good. > > The unification is achieved through classic object-oriented polymorphism, > which works fine, though which is rather verbose. This could probably be > reduced with some tinkering on the model, but it's probably reaching the > point where additional tinkering on the model isn't worth it. I'm ok with > sticking with this approach for now. Maybe we can clean it up later, or maybe > not -- it's at least fairly straightforward. > > One issue that contributes to the verbosity is the repeated null checking. > The null checking enables the test code to proceed and end up with -1 as the > capacity if there's a null in there somewhere. This will cause the assertion > to fail. This is good in that it will call attention to itself (as opposed to > silently passing or something). However, if the test cases are set up > properly, they should never run into null. If the null checking weren't done, > an unexpected null will throw NPE, which will be caught be the framework and > reported as an error. > > That seems perfectly fine to me, so I'd suggest simply removing the null > checking. That would also reduce the bulkiness of infrastructure. @stuart-marks done. - PR: https://git.openjdk.java.net/jdk/pull/8302
Re: RFR: 8284780: Need methods to create pre-sized HashSet and LinkedHashSet [v18]
> as title. XenoAmess has updated the pull request incrementally with two additional commits since the last revision: - remove null check for Capacitiable in WhiteBoxResizeTest - Rename type variable per CSR request; minor spec wording change. - Changes: - all: https://git.openjdk.java.net/jdk/pull/8302/files - new: https://git.openjdk.java.net/jdk/pull/8302/files/98bfb0e1..bacc9ca8 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8302=17 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8302=16-17 Stats: 19 lines in 3 files changed: 0 ins; 13 del; 6 mod Patch: https://git.openjdk.java.net/jdk/pull/8302.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8302/head:pull/8302 PR: https://git.openjdk.java.net/jdk/pull/8302
Re: RFR: 8287186: JDK modules participating in preview
On Wed, 8 Jun 2022 15:46:24 GMT, Paul Sandoz wrote: > Allow JDK modules that use preview features (preview language features or > preview API features from dependent modules) to participate without the need > to compile with `--enable-preview`. > > It's difficult to enable participation using an annotation due to the nature > in which symbols are encountered when processing source as there is no > guaranteed order to the processing of certain symbols. > > Instead a JDK module participates if the `java.base` package > `jdk.internal.javac` is exported to that module (@lahodaj clever idea!). An > internal annotation `jdk.internal.javac.ParticipatesInPreview` can be > declared on the module. Such a declaration cannot be enforced but does by its > use require the `jdk.internal.javac`'s export list to be updated. > > The modules `jdk.incubator.vector` and `jdk.incubator.concurrent` have been > updated accordingly, both of which participate in preview APIs (APIs in > `java.lang.foreign` and `Thread.ofVirtual`, respectively). Can java.management participate too? It would allow sun.management.Util.isVirtual(Thread) to go away (lots of methods in sun.management.ThreadImpl need to test if a thread is virtual). - PR: https://git.openjdk.java.net/jdk/pull/9087
Re: RFR: 8285405: add test and check for negative argument to HashMap::newHashMap et al
On Mon, 6 Jun 2022 06:57:23 GMT, Jaikiran Pai wrote: > Can I please get a review of this change which addresses > https://bugs.openjdk.java.net/browse/JDK-8285405? > > I've added the test for `LinkedHashMap.newLinkedHashMap(int)` in the existing > `test/jdk/java/util/LinkedHashMap/Basic.java` since that test has tests for > various APIs of this class. > > For `WeakHashMap.newWeakHashMap` and `HashMap.newHashMap`, I have created new > test classes under relevant locations, since these classes already have test > classes (almost) per API/feature in those locations. Marked as reviewed by lancea (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/9036
Re: RFR: 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved flags is used [v6]
> 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved > flags is used Thiago Henrique Hüpner has updated the pull request incrementally with one additional commit since the last revision: Simplify test names - Changes: - all: https://git.openjdk.java.net/jdk/pull/9049/files - new: https://git.openjdk.java.net/jdk/pull/9049/files/c88bc754..7362da92 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=9049=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk=9049=04-05 Stats: 30 lines in 1 file changed: 10 ins; 3 del; 17 mod Patch: https://git.openjdk.java.net/jdk/pull/9049.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/9049/head:pull/9049 PR: https://git.openjdk.java.net/jdk/pull/9049
Integrated: 8202449: overflow handling in Random.doubles
On Thu, 19 May 2022 15:54:06 GMT, Raffaello Giulietti wrote: > Extend the range of Random.doubles(double, double) and similar methods. This pull request has now been integrated. Changeset: c8cff1bd Author:Raffaello Giulietti Committer: Joe Darcy URL: https://git.openjdk.java.net/jdk/commit/c8cff1bd6f9807e90a6992ad3e181fe0d94397b8 Stats: 118 lines in 5 files changed: 47 ins; 36 del; 35 mod 8202449: overflow handling in Random.doubles Reviewed-by: darcy - PR: https://git.openjdk.java.net/jdk/pull/8791
Integrated: 8233760: Result of BigDecimal.toString throws overflow exception on new BigDecimal(str)
On Thu, 26 May 2022 18:02:14 GMT, Raffaello Giulietti wrote: > BigDecimal(String) currently fails to accept some strings produced by > BigDecimal.toString(). This PR removes this limitation. This pull request has now been integrated. Changeset: c15e10fb Author:Raffaello Giulietti Committer: Joe Darcy URL: https://git.openjdk.java.net/jdk/commit/c15e10fb6c35a99e80009f0a7b6a252fcbb549b7 Stats: 70 lines in 2 files changed: 23 ins; 26 del; 21 mod 8233760: Result of BigDecimal.toString throws overflow exception on new BigDecimal(str) Reviewed-by: darcy - PR: https://git.openjdk.java.net/jdk/pull/8905
Integrated: JDK-8281001 Class::forName(String) defaults to system class loader if the caller is null
On Sat, 14 May 2022 00:30:00 GMT, Tim Prinzing wrote: > The Class::forName behavior change to match JNI FindClass is a compatible > change and seems pretty attractive as it would be expected that > Class::forName would give the same behavior as FindClass which uses the > system classloader. The test for 8281006 was enhanced to test for this > change. Merged master to pick up fixes to unrelated test failures to reduce > noise. This pull request has now been integrated. Changeset: b92ce269 Author:Tim Prinzing Committer: Mandy Chung URL: https://git.openjdk.java.net/jdk/commit/b92ce2699b604cff638db583215863da8e253db8 Stats: 15 lines in 4 files changed: 8 ins; 3 del; 4 mod 8281001: Class::forName(String) defaults to system class loader if the caller is null Reviewed-by: mchung - PR: https://git.openjdk.java.net/jdk/pull/8711
Re: RFR: JDK-8281001 Class::forName(String) defaults to system class loader if the caller is null [v6]
On Wed, 8 Jun 2022 13:07:20 GMT, Tim Prinzing wrote: >> The Class::forName behavior change to match JNI FindClass is a compatible >> change and seems pretty attractive as it would be expected that >> Class::forName would give the same behavior as FindClass which uses the >> system classloader. The test for 8281006 was enhanced to test for this >> change. Merged master to pick up fixes to unrelated test failures to reduce >> noise. > > Tim Prinzing has updated the pull request incrementally with one additional > commit since the last revision: > > append bug id being fixed rather than sorted insert Marked as reviewed by mchung (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8711
RFR: 8287186: JDK modules participating in preview
Allow JDK modules that use preview features (preview language features or preview API features from dependent modules) to participate without the need to compile with `--enable-preview`. It's difficult to enable participation using an annotation due to the nature in which symbols are encountered when processing source as there is no guaranteed order to the processing of certain symbols. Instead a JDK module participates if the `java.base` package `jdk.internal.javac` is exported to that module (@lahodaj clever idea!). An internal annotation `jdk.internal.javac.ParticipatesInPreview` can be declared on the module. Such a declaration cannot be enforced but does by its use require the `jdk.internal.javac`'s export list to be updated. The modules `jdk.incubator.vector` and `jdk.incubator.concurrent` have been updated accordingly, both of which participate in preview APIs (APIs in `java.lang.foreign` and `Thread.ofVirtual`, respectively). - Commit messages: - Unused import. - Generalize the pariticipating in preview APIs. Changes: https://git.openjdk.java.net/jdk/pull/9087/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=9087=00 Issue: https://bugs.openjdk.org/browse/JDK-8287186 Stats: 98 lines in 7 files changed: 62 ins; 28 del; 8 mod Patch: https://git.openjdk.java.net/jdk/pull/9087.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/9087/head:pull/9087 PR: https://git.openjdk.java.net/jdk/pull/9087
Integrated: 8287541: Files.writeString fails to throw IOException for charset "windows-1252"
On Fri, 3 Jun 2022 16:48:46 GMT, Naoto Sato wrote: > The code path calls `String.getBytesNoRepl()`, but it blindly replaces > unmappable characters with replacements if the encoder is an `ArrayEncoder`. > Changed only to do so if `doReplace` is `true` in > `String.encodeWithEncoder()`. This pull request has now been integrated. Changeset: 6fb84e2c Author:Naoto Sato URL: https://git.openjdk.java.net/jdk/commit/6fb84e2c9119bdb9c66dd49422bcab637bbd4008 Stats: 22 lines in 2 files changed: 4 ins; 11 del; 7 mod 8287541: Files.writeString fails to throw IOException for charset "windows-1252" Reviewed-by: iris, bpb, alanb, jpai, lancea, aturbanov - PR: https://git.openjdk.java.net/jdk/pull/9019
Re: RFR: 8287902: UnreadableRB case in MissingResourceCauseTest is not working reliably on Windows
On Tue, 7 Jun 2022 12:19:29 GMT, Magnus Ihse Bursie wrote:
> I have failed to reproduce the working conditions that makes a file actually
> unreadable for the owner on Windows, neither on my GHA test repo, nor
> locally.
I had quick look at the CI setup we have access to. It appears that this test
passes there successfully on Windows. Could it be something to do with Windows
version being used?
test/jdk/java/util/ResourceBundle/Control/MissingResourceCauseTest.java line 48:
> 46: private static void callGetBundle(String baseName, Locale locale,
> 47: Class
> expectedCause) {
> 48: if (baseName.equals("UnreadableRB") &&
> System.getProperty("os.name").startsWith("Windows")) {
Hello @magicus, perhaps we could do this check in the `main` of this class
(line 37) before calling the `callGetBundle` for the `UnreadableRB` base name?
That way this method doesn't have to bother about which `baseName` is being
passed.
I had a quick look at how other tests do the OS name checks and they use the
`Platform.isWindows()` API which is part of the test library. I suspect you may
not be able to use it here though since this `MissingResourceCauseTest` class
gets launched manually through the `MissingResourceCauseTestRun`. So using
`System.getProperty` I think is fine. Nit - perhaps we could match this check
to do whatever `Platform.isWindows()` is doing (which seems to be checking that
the os name starts with `win`, ignoring the case).
-
PR: https://git.openjdk.java.net/jdk/pull/9061
Re: RFR: 8288011: StringConcatFactory: Split application of stringifiers
On Wed, 8 Jun 2022 14:44:33 GMT, Jim Laskey wrote: >> To take optimal advantage of the pre-existing optimization for repeated >> filters we could split the application of different types of stringifiers. >> >> The resulting difference in order of evaluation is not observable by >> conventional means since all reference type share the same object >> stringifier, and the others are filtering primitives (floats and doubles) >> which have been passed by value already. >> >> This change neutral on many concatenation expression shapes, but for any >> complex expressions with interleaving float/double and reference parameters >> it brings a straightforward reduction in rebinds and underlying LFs >> generated. For example on the >> [MixedStringCombinations.java](https://gist.github.com/cl4es/08fb581dece3a73e89bfa52337bc4248) >> test there's a modest 2% reduction in total classes loaded with this change >> (from 16209 to 15872) > > I make heavy use of the `mh = filterArgument(baseMH, 0, arrayOfFilters);` > pattern. I assumed it generated fewer LF than making individual calls. Am I > wrong? @JimLaskey no, you're generally right to assume that bunching them up reduces the number of LFs created. There's a flaw in the `makeRepeatedFilterForm` transform that means the theoretical total amount of forms can surpass the baseline if you do _both_ one-by-one and all-at-once transforms from the same root MH. That's a very big if, but it's something I've felt slight unease about since I did that optimization. - PR: https://git.openjdk.java.net/jdk/pull/9082
Re: RFR: 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved flags is used [v5]
On Wed, 8 Jun 2022 14:51:59 GMT, Thiago Henrique Hüpner
wrote:
>> 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved
>> flags is used
>
> Thiago Henrique Hüpner has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Fix test message
test/jdk/tools/jar/modularJar/Basic.java line 1050:
> 1048: */
> 1049: @Test(dataProvider = "resolutionNames")
> 1050: public void
> updateFooModuleResolutionDoNotResolveByDefaultAndWarnIfResolved(String
> resolutionName, Predicate hasWarning) throws IOException {
Update the existing tests is good but overly long lines make it too hard to
read (it will make future side-by-side view impossible). Can you trim all these
lines down to something closer to the existing tests in this file?
-
PR: https://git.openjdk.java.net/jdk/pull/9049
Re: RFR: 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved flags is used [v5]
> 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved > flags is used Thiago Henrique Hüpner has updated the pull request incrementally with one additional commit since the last revision: Fix test message - Changes: - all: https://git.openjdk.java.net/jdk/pull/9049/files - new: https://git.openjdk.java.net/jdk/pull/9049/files/c4a719e3..c88bc754 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=9049=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=9049=03-04 Stats: 4 lines in 1 file changed: 0 ins; 0 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/9049.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/9049/head:pull/9049 PR: https://git.openjdk.java.net/jdk/pull/9049
Re: RFR: 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved flags is used [v4]
> 8287760: --do-not-resolve-by-default gets overwritten if --warn-if-resolved > flags is used Thiago Henrique Hüpner has updated the pull request incrementally with one additional commit since the last revision: Add tests - Changes: - all: https://git.openjdk.java.net/jdk/pull/9049/files - new: https://git.openjdk.java.net/jdk/pull/9049/files/2acbedb1..c4a719e3 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=9049=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=9049=02-03 Stats: 129 lines in 1 file changed: 129 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/9049.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/9049/head:pull/9049 PR: https://git.openjdk.java.net/jdk/pull/9049
Re: RFR: 8288011: StringConcatFactory: Split application of stringifiers
On Wed, 8 Jun 2022 10:20:37 GMT, Claes Redestad wrote: > To take optimal advantage of the pre-existing optimization for repeated > filters we could split the application of different types of stringifiers. > > The resulting difference in order of evaluation is not observable by > conventional means since all reference type share the same object > stringifier, and the others are filtering primitives (floats and doubles) > which have been passed by value already. > > This change neutral on many concatenation expression shapes, but for any > complex expressions with interleaving float/double and reference parameters > it brings a straightforward reduction in rebinds and underlying LFs > generated. For example on the > [MixedStringCombinations.java](https://gist.github.com/cl4es/08fb581dece3a73e89bfa52337bc4248) > test there's a modest 2% reduction in total classes loaded with this change > (from 16209 to 15872) I make heavy use of the `mh = filterArgument(baseMH, 0, arrayOfFilters);` pattern. I assumed it generated fewer LF than making individual calls. Am I wrong? - PR: https://git.openjdk.java.net/jdk/pull/9082
Re: RFR: 8288011: StringConcatFactory: Split application of stringifiers
On Wed, 8 Jun 2022 13:42:11 GMT, Jorn Vernee wrote: > Looking at the code in MethodHandles::filterArguments, and this optimization. > It seems that for multiple filters we could potentially always generate just > a single new lambda form if we wanted, not just for repeated filters. > > It would be similar to `LambdaFormEditor::makeRepeatedFilterForm`. Just need > to extend the species data with one new `L_TYPE` field for each unique > filter, generate a new getCombiner `Name` for each unique combiner, pass in > an array of combiner types instead of just a single one, and pass in an > additional array of ints that marks the combiner used by each argument > position. Then do similar wiring up as we do now, but calling the > corresponding combiner for each argument pos. Right? > > (If I'm right about that, maybe that's even a more interesting direction to > explore, since it would benefit everyone, and the code in StringConcatFactory > could stay the same) You're right, but I think such a transform would add quite a bit of complexity. Back when I added the `makeRepeatedFilterForm` optimization (for the benefit of `StringConcatFactory` ) I sought to strike a balance between the simple repeatedly-create-`filterArgumentForm`s transform we would do before and more complex forms that would remove more intermediates. I was also a bit concerned that such "shortcutting" transforms could mean we eliminate sharing, and the more complex we make things the harder it gets to reason about. Example: ... mh1 = filterArgument(baseMH, 0, filter); mh1 = filterArgument(mh1, 0, filter); mh2 = filterArgument(baseMH, 0, filter, filter); mh1.form() == mh2.form(); // false The two different pathways end up with two different LFs, since `makeRepeatedFilterForm` shortcuts. In practice this might be a non-issue, but I figured it'd be nice if we could have a reasonable pathway to resolve mh1.form() and mh2.form() to the same thing in the future. - PR: https://git.openjdk.java.net/jdk/pull/9082
Re: RFR: 8288011: StringConcatFactory: Split application of stringifiers
On Wed, 8 Jun 2022 10:20:37 GMT, Claes Redestad wrote: > To take optimal advantage of the pre-existing optimization for repeated > filters we could split the application of different types of stringifiers. > > The resulting difference in order of evaluation is not observable by > conventional means since all reference type share the same object > stringifier, and the others are filtering primitives (floats and doubles) > which have been passed by value already. > > This change neutral on many concatenation expression shapes, but for any > complex expressions with interleaving float/double and reference parameters > it brings a straightforward reduction in rebinds and underlying LFs > generated. For example on the > [MixedStringCombinations.java](https://gist.github.com/cl4es/08fb581dece3a73e89bfa52337bc4248) > test there's a modest 2% reduction in total classes loaded with this change > (from 16209 to 15872) This looks good to me, but... Looking at the code in MethodHandles::filterArguments, and this optimization. It seems that for multiple filters we could potentially always generate just a single new lambda form if we wanted, not just for repeated filters. It would be similar to `LambdaFormEditor::makeRepeatedFilterForm`. Just need to extend the species data with one new `L_TYPE` field for each unique filter, generate a new getCombiner `Name` for each unique combiner, pass in an array of combiner types instead of just a single one, and pass in an additional array of ints that marks the combiner used by each argument position. Then do similar wiring up as we do now, but calling the corresponding combiner for each argument pos. Right? (If I'm right about that, maybe that's even a more interesting direction to explore, since it would benefit everyone, and the code in StringConcatFactory could stay the same) - Marked as reviewed by jvernee (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/9082
RFR: 8288021: Add hard test cases to jdk.internal.math.DoubleToDecimalChecker
These 19'545 doubles were generated on purpose by Paul Zimmermann of INRIA as hard test cases. - Commit messages: - 8288021: Add hard test cases to jdk.internal.math.DoubleToDecimalChecker Changes: https://git.openjdk.java.net/jdk/pull/9084/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=9084=00 Issue: https://bugs.openjdk.org/browse/JDK-8288021 Stats: 19594 lines in 1 file changed: 19594 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/9084.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/9084/head:pull/9084 PR: https://git.openjdk.java.net/jdk/pull/9084
Re: RFR: 8288011: StringConcatFactory: Split application of stringifiers
On Wed, 8 Jun 2022 12:11:30 GMT, Сергей Цыпанов wrote: >> To take optimal advantage of the pre-existing optimization for repeated >> filters we could split the application of different types of stringifiers. >> >> The resulting difference in order of evaluation is not observable by >> conventional means since all reference type share the same object >> stringifier, and the others are filtering primitives (floats and doubles) >> which have been passed by value already. >> >> This change neutral on many concatenation expression shapes, but for any >> complex expressions with interleaving float/double and reference parameters >> it brings a straightforward reduction in rebinds and underlying LFs >> generated. For example on the >> [MixedStringCombinations.java](https://gist.github.com/cl4es/08fb581dece3a73e89bfa52337bc4248) >> test there's a modest 2% reduction in total classes loaded with this change >> (from 16209 to 15872) > > src/java.base/share/classes/java/lang/invoke/StringConcatFactory.java line > 509: > >> 507: doubleFilters = new MethodHandle[ptypes.length]; >> 508: } >> 509: doubleFilters[i] = doubleStringifier(); > > Why do we have `ptypes[i] = int.class` for `int` and `short`, `ptypes[i] = > String.class` for `float`, `double` and `Object` and none for `boolean` and > `char`? 類 `Object`, `float` and `double` arguments are all eagerly transformed to `String` (this is what the "stringifiers" do). Since we apply these as filters as a last step (which means that's the first thing that will run) we then must change the internal type to `String`. Ultimately means we have fewer types to worry about in the combinator. We also widen integral subword types to `int` (`short` and `byte`). This doesn't need to be adapted by filtering but can lean on implicit conversion (subword primitives are widened to int anyhow, what we care about is matching the correct logic). This mainly matters insofar that the same direct method handle will be used for these three types, as it can (and will) be adapted without modification by the `viewAsType` call that ensures the `MH` returned from the BSM has the exact type asked for. `boolean` and `char` types can't use the `int` mixers/prependers and instead have specialized methods that will be adapted into the concat MH tree. This means the parameter type stays `boolean` and `char` throughout, respectively. - PR: https://git.openjdk.java.net/jdk/pull/9082
Re: RFR: JDK-8281001 Class::forName(String) defaults to system class loader if the caller is null [v6]
> The Class::forName behavior change to match JNI FindClass is a compatible > change and seems pretty attractive as it would be expected that > Class::forName would give the same behavior as FindClass which uses the > system classloader. The test for 8281006 was enhanced to test for this > change. Merged master to pick up fixes to unrelated test failures to reduce > noise. Tim Prinzing has updated the pull request incrementally with one additional commit since the last revision: append bug id being fixed rather than sorted insert - Changes: - all: https://git.openjdk.java.net/jdk/pull/8711/files - new: https://git.openjdk.java.net/jdk/pull/8711/files/3ce3a61a..95d7044d Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8711=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8711=04-05 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/8711.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8711/head:pull/8711 PR: https://git.openjdk.java.net/jdk/pull/8711
Re: RFR: 8288011: StringConcatFactory: Split application of stringifiers
On Wed, 8 Jun 2022 10:20:37 GMT, Claes Redestad wrote: > To take optimal advantage of the pre-existing optimization for repeated > filters we could split the application of different types of stringifiers. > > The resulting difference in order of evaluation is not observable by > conventional means since all reference type share the same object > stringifier, and the others are filtering primitives (floats and doubles) > which have been passed by value already. > > This change neutral on many concatenation expression shapes, but for any > complex expressions with interleaving float/double and reference parameters > it brings a straightforward reduction in rebinds and underlying LFs > generated. For example on the > [MixedStringCombinations.java](https://gist.github.com/cl4es/08fb581dece3a73e89bfa52337bc4248) > test there's a modest 2% reduction in total classes loaded with this change > (from 16209 to 15872) src/java.base/share/classes/java/lang/invoke/StringConcatFactory.java line 509: > 507: doubleFilters = new MethodHandle[ptypes.length]; > 508: } > 509: doubleFilters[i] = doubleStringifier(); Why do we have `ptypes[i] = int.class` for `int` and `short`, `ptypes[i] = String.class` for `float`, `double` and `Object` and none for `boolean` and `char`? 類 - PR: https://git.openjdk.java.net/jdk/pull/9082
Re: RFR: 8287007: [cgroups] Consistently use stringStream throughout parsing code [v2]
On Wed, 8 Jun 2022 08:25:21 GMT, Severin Gehwolf wrote:
>> src/hotspot/os/linux/cgroupV1Subsystem_linux.cpp line 54:
>>
>>> 52: } else {
>>> 53: char *p = strstr(cgroup_path, _root);
>>> 54: if (p != NULL && p == cgroup_path) {
>>
>> I think this change should be done in a separate bug, because it will cause
>> the `if` block to be executed. Previously the `if` block is never executed
>> (unless `cgroup_path == _root` ??, but then it will just set `_path` to the
>> same string as `_mount_point`) -- so we don't even know if this change of
>> behavior might do something harmful.
>
> OK. Then this will remain to be dead code and I'll remove the corresponding
> test case in the new `gtests` too (as they'd otherwise fail in contrast to
> the Java code).
Done.
-
PR: https://git.openjdk.java.net/jdk/pull/8969
Re: RFR: 8287007: [cgroups] Consistently use stringStream throughout parsing code [v3]
> Please review this cleanup change in the cgroup subsystem which used to use > hard-coded stack allocated > buffers for concatenating strings in memory. We can use `stringStream` > instead which doesn't have the issue > of hard-coding maximum lengths (and related checks) and makes the code, thus, > easier to read. > > While at it, I've written basic `gtests` verifying current behaviour (and the > same for the JDK code). From > a functionality point of view this is a no-op (modulo the one bug in the > substring case which seems to be > there since day one). I'd prefer if we could defer any change in > functionality to a separate bug as this is > meant to only use stringStream throughout the cgroups code. > > Testing: > - [x] Container tests on Linux x86_64 cgroups v1 and cgroups v2 > - [x] Added tests, which I've verified also pass before the stringStream > change > > Thoughts? Severin Gehwolf has updated the pull request incrementally with one additional commit since the last revision: Remove fix for substring match - Changes: - all: https://git.openjdk.java.net/jdk/pull/8969/files - new: https://git.openjdk.java.net/jdk/pull/8969/files/8047fe37..84d952b8 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8969=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8969=01-02 Stats: 10 lines in 2 files changed: 0 ins; 7 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/8969.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8969/head:pull/8969 PR: https://git.openjdk.java.net/jdk/pull/8969
RFR: 8288013: jpackage: test utility Windows registry enhancement
In jpackage test suite there is a test utility WindowsHelper.queryRegistryValue . It is used to verify changes to Windows registry that can be done by installer packages. It spawns `reg.exe query` process and parses its output. This patch adds support for quering empty values and additional REG_* datatypes. Correctness was verified manually with the following registry values: //(Default)REG_SZtest1 //string_valREG_SZtest2 //string_val_emptyREG_SZ //bin_valREG_BINARY4242 //bin_val_emptyREG_BINARY //dword_valREG_DWORD0x2a //qword_valREG_QWORD0x2a //multi_string_valREG_MULTI_SZtest3\0test4 //multi_string_val_emptyREG_MULTI_SZ //expand_string_valREG_EXPAND_SZtest5 //expand_string_val_emptyREG_EXPAND_SZ Corresponding utility output: [04:36:38.190] TRACE: Registry value [] at [...] path is [test1] [04:36:38.220] TRACE: Registry value [string_val] at [...] path is [test2] [04:36:38.251] TRACE: Registry value [string_val_empty] at [...] path is [] [04:36:38.282] TRACE: Registry value [bin_val] at [...] path is [4242] [04:36:38.315] TRACE: Registry value [bin_val_empty] at [...] path is [] [04:36:38.347] TRACE: Registry value [dword_val] at [...] path is [0x2a] [04:36:38.378] TRACE: Registry value [qword_val] at [...] path is [0x2a] [04:36:38.410] TRACE: Registry value [multi_string_val] at [...] path is [test3\\0test4] [04:36:38.441] TRACE: Registry value [multi_string_val_empty] at [...] path is [] [04:36:38.473] TRACE: Registry value [expand_string_val] at [...] path is [test5] [04:36:38.506] TRACE: Registry value [expand_string_val_empty] at [...] path is [] Testing: - [x] ran "jtreg:jdk/tools/jpackage/windows" before and after the patch - Commit messages: - 8288013: jpackage: test utility Windows registry enhancement Changes: https://git.openjdk.java.net/jdk/pull/9083/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=9083=00 Issue: https://bugs.openjdk.org/browse/JDK-8288013 Stats: 20 lines in 1 file changed: 17 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/9083.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/9083/head:pull/9083 PR: https://git.openjdk.java.net/jdk/pull/9083
Re: RFR: 8287908: Use non-cloning reflection methods where acceptable [v2]
> Instead of `Executable.getParameterTypes()` we could use > `Executable.getSharedParameterTypes()` in trusted code. Same is applicable > for `Executable.getExceptionTypes()`. Сергей Цыпанов 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 two additional commits since the last revision: - Merge branch 'master' into 8287908 - 8287908: Use non-cloning reflection methods where acceptable - Changes: - all: https://git.openjdk.java.net/jdk/pull/9064/files - new: https://git.openjdk.java.net/jdk/pull/9064/files/a6715257..635854b2 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=9064=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=9064=00-01 Stats: 1833 lines in 92 files changed: 1180 ins; 242 del; 411 mod Patch: https://git.openjdk.java.net/jdk/pull/9064.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/9064/head:pull/9064 PR: https://git.openjdk.java.net/jdk/pull/9064
RFR: 8288011: StringConcatFactory: Split application of stringifiers
To take optimal advantage of the pre-existing optimization for repeated filters we could split the application of different types of stringifiers. The resulting difference in order of evaluation is not observable by conventional means since all reference type share the same object stringifier, and the others are filtering primitives (floats and doubles) which have been passed by value already. This change neutral on many concatenation expression shapes, but for any complex expressions with interleaving float/double and reference parameters it brings a straightforward reduction in rebinds and underlying LFs generated. For example on the [MixedStringCombinations.java](https://gist.github.com/cl4es/08fb581dece3a73e89bfa52337bc4248) test there's a modest 2% reduction in total classes loaded with this change (from 16209 to 15872) - Commit messages: - StringConcatFactory: Split application of stringifiers Changes: https://git.openjdk.java.net/jdk/pull/9082/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=9082=00 Issue: https://bugs.openjdk.org/browse/JDK-8288011 Stats: 27 lines in 1 file changed: 16 ins; 3 del; 8 mod Patch: https://git.openjdk.java.net/jdk/pull/9082.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/9082/head:pull/9082 PR: https://git.openjdk.java.net/jdk/pull/9082
Re: RFR: JDK-8284858: Start of release updates for JDK 20 [v5]
On Thu, 2 Jun 2022 17:00:35 GMT, Joe Darcy wrote: >> Time to start getting ready for JDK 20... > > Joe Darcy 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 40 additional commits since > the last revision: > > - Update symbols for JDK 19 b25. > - Merge branch 'master' into JDK-8284858 > - Adjust deprecated options test. > - Merge branch 'master' into JDK-8284858 > - Update deprecated options test. > - Merge branch 'master' into JDK-8284858 > - Respond to review feedback. > - Update symbol information for JDK 19 b24. > - Merge branch 'master' into JDK-8284858 > - Merge branch 'master' into JDK-8284858 > - ... and 30 more: > https://git.openjdk.java.net/jdk/compare/6a77da51...e495a579 Build changes look good. Thanks for fixing the bug in `generate-symbol-data.sh`. - Marked as reviewed by ihse (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8236
Re: RFR: 8283726: x86_64 intrinsics for compareUnsigned method in Integer and Long
On Tue, 7 Jun 2022 17:14:18 GMT, Quan Anh Mai wrote: > Hi, > > This patch implements intrinsics for `Integer/Long::compareUnsigned` using > the same approach as the JVM does for long and floating-point comparisons. > This allows efficient and reliable usage of unsigned comparison in Java, > which is a basic operation and is important for range checks such as > discussed in #8620 . > > Thank you very much. I have added a benchmark for the intrinsic. The result is as follows, thanks a lot: Before After Benchmark (size) Mode Cnt Score Error Score Error Units Integers.compareUnsigned 500 avgt 15 0.527 ± 0.002 0.498 ± 0.011 us/op Longs.compareUnsigned500 avgt 15 0.677 ± 0.014 0.561 ± 0.006 us/op - PR: https://git.openjdk.java.net/jdk/pull/9068
Re: RFR: 8283726: x86_64 intrinsics for compareUnsigned method in Integer and Long [v2]
On Tue, 7 Jun 2022 17:41:13 GMT, Vladimir Kozlov wrote:
>> Quan Anh Mai has updated the pull request incrementally with two additional
>> commits since the last revision:
>>
>> - remove comments
>> - review comments
>
> src/hotspot/share/opto/subnode.hpp line 217:
>
>> 215:
>> //--CmpU3Node--
>> 216: // Compare 2 unsigned values, returning integer value (-1, 0 or 1).
>> 217: class CmpU3Node : public CmpUNode {
>
> Place it after `CmpUNode` class.
Done
-
PR: https://git.openjdk.java.net/jdk/pull/9068
Re: RFR: 8283726: x86_64 intrinsics for compareUnsigned method in Integer and Long [v2]
> Hi, > > This patch implements intrinsics for `Integer/Long::compareUnsigned` using > the same approach as the JVM does for long and floating-point comparisons. > This allows efficient and reliable usage of unsigned comparison in Java, > which is a basic operation and is important for range checks such as > discussed in #8620 . > > Thank you very much. Quan Anh Mai has updated the pull request incrementally with two additional commits since the last revision: - remove comments - review comments - Changes: - all: https://git.openjdk.java.net/jdk/pull/9068/files - new: https://git.openjdk.java.net/jdk/pull/9068/files/01c0a07c..b5627135 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=9068=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=9068=00-01 Stats: 44 lines in 3 files changed: 32 ins; 12 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/9068.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/9068/head:pull/9068 PR: https://git.openjdk.java.net/jdk/pull/9068
Re: RFR: 8287007: [cgroups] Consistently use stringStream throughout parsing code [v2]
On Wed, 8 Jun 2022 07:13:30 GMT, Ioi Lam wrote:
>> Severin Gehwolf 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 six additional
>> commits since the last revision:
>>
>> - Merge branch 'master' into jdk-8287007-string-stream
>> - Add cgroups v2 java test
>> - use stringStream in cgroups v2
>> - Add gtest for cgroups v2 code path
>>
>>Also fixes the bug when cgroup path is '/'.
>> - 8287007: [cgroups] Consistently use stringStream throughout parsing code
>> - 8287007: [cgroups] Consistently use stringStream throughout parsing code
>
> src/hotspot/os/linux/cgroupV1Subsystem_linux.cpp line 54:
>
>> 52: } else {
>> 53: char *p = strstr(cgroup_path, _root);
>> 54: if (p != NULL && p == cgroup_path) {
>
> I think this change should be done in a separate bug, because it will cause
> the `if` block to be executed. Previously the `if` block is never executed
> (unless `cgroup_path == _root` ??, but then it will just set `_path` to the
> same string as `_mount_point`) -- so we don't even know if this change of
> behavior might do something harmful.
OK. Then this will remain to be dead code and I'll remove the corresponding
test case in the new `gtests` too (as they'd otherwise fail in contrast to the
Java code).
-
PR: https://git.openjdk.java.net/jdk/pull/8969
Re: RFR: 8287903: Reduce runtime of java.math microbenchmarks
On Tue, 7 Jun 2022 12:34:25 GMT, Claes Redestad wrote: > - Reduce runtime by running fewer forks, fewer iterations, less warmup. All > micros tested in this group appear to stabilize very quickly. > - Refactor BigIntegers to avoid re-running some (most) micros over and over > with parameter values that don't affect them. > > Expected runtime down from 14 hours to 15 minutes. Marked as reviewed by aph (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/9062
Integrated: 8287442: Reduce list to array conversions in java.lang.invoke.MethodHandles
On Fri, 27 May 2022 14:18:19 GMT, Claes Redestad wrote: > In preparation of #8855 this PR refactors the conversions from `List` to > array and array to `List`, reducing the number of conversions when calling > `MethodHandles.dropArguments` in particular. This remove about ~5% of > allocations on the `StringConcatFactoryBootstraps` microbenchmark. This pull request has now been integrated. Changeset: ecf00785 Author:Claes Redestad URL: https://git.openjdk.java.net/jdk/commit/ecf00785f21125d88f5cc18311f586a7bb6ddc56 Stats: 49 lines in 3 files changed: 9 ins; 6 del; 34 mod 8287442: Reduce list to array conversions in java.lang.invoke.MethodHandles Reviewed-by: jvernee - PR: https://git.openjdk.java.net/jdk/pull/8923
Re: RFR: 8287007: [cgroups] Consistently use stringStream throughout parsing code [v2]
On Tue, 7 Jun 2022 12:42:26 GMT, Severin Gehwolf wrote:
>> Please review this cleanup change in the cgroup subsystem which used to use
>> hard-coded stack allocated
>> buffers for concatenating strings in memory. We can use `stringStream`
>> instead which doesn't have the issue
>> of hard-coding maximum lengths (and related checks) and makes the code,
>> thus, easier to read.
>>
>> While at it, I've written basic `gtests` verifying current behaviour (and
>> the same for the JDK code). From
>> a functionality point of view this is a no-op (modulo the one bug in the
>> substring case which seems to be
>> there since day one). I'd prefer if we could defer any change in
>> functionality to a separate bug as this is
>> meant to only use stringStream throughout the cgroups code.
>>
>> Testing:
>> - [x] Container tests on Linux x86_64 cgroups v1 and cgroups v2
>> - [x] Added tests, which I've verified also pass before the stringStream
>> change
>>
>> Thoughts?
>
> Severin Gehwolf 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 six additional
> commits since the last revision:
>
> - Merge branch 'master' into jdk-8287007-string-stream
> - Add cgroups v2 java test
> - use stringStream in cgroups v2
> - Add gtest for cgroups v2 code path
>
>Also fixes the bug when cgroup path is '/'.
> - 8287007: [cgroups] Consistently use stringStream throughout parsing code
> - 8287007: [cgroups] Consistently use stringStream throughout parsing code
Changes requested by iklam (Reviewer).
src/hotspot/os/linux/cgroupV1Subsystem_linux.cpp line 54:
> 52: } else {
> 53: char *p = strstr(cgroup_path, _root);
> 54: if (p != NULL && p == cgroup_path) {
I think this change should be done in a separate bug, because it will cause the
`if` block to be executed. Previously the `if` block is never executed (unless
`cgroup_path == _root` ??, but then it will just set `_path` to the same string
as `_mount_point`) -- so we don't even know if this change of behavior might do
something harmful.
-
PR: https://git.openjdk.java.net/jdk/pull/8969
Integrated: 8287522: StringConcatFactory: Add in prependers and mixers in batches
On Mon, 23 May 2022 20:03:05 GMT, Claes Redestad wrote: > When generating `MethodHandle`-based concatenation expressions in > `StringConcatFactory` we can reduce the number of classes generated at > runtime by creating small batches of prependers and mixers before binding > them into the root expression tree. > > Improvements on one-off tests are modest, while the improvement on > bootstrapping stress tests can be substantial > ([MixedStringCombinations.java](https://gist.github.com/cl4es/08fb581dece3a73e89bfa52337bc4248)): > > | Build |# classes| Runtime | > | --- | - | --- | > | Baseline | 31119 | 2.942s | > | Patch | 16208 | 1.958s | > > An earlier version of this patch saw a regression in the > `StringConcatFactoryBootstraps` microbenchmark. After some refactoring along > with the optimizations in #8881 and #8923 that is no longer the case, and > allocation pressure is down slightly compared to the baseline on such a > repeat-the-same-bootstrap stress test: > > Baseline: > > Benchmark Mode Cnt > Score Error Units > SCFB.makeConcatWithConstants avgt5 > 2170.039 ? 117.441 ns/op > SCFB.makeConcatWithConstants:?gc.alloc.rate.norm avgt5 > 3538.020 ?4.558B/op > > This patch: > > Benchmark Mode Cnt > Score Error Units > SCFB.makeConcatWithConstants avgt5 > 2144.807 ? 162.685 ns/op > SCFB.makeConcatWithConstants:?gc.alloc.rate.norm avgt5 > 3184.943 ?4.495B/op This pull request has now been integrated. Changeset: 5c39a366 Author:Claes Redestad URL: https://git.openjdk.java.net/jdk/commit/5c39a3664186b91512c6a6cfcd8aa0e9860614ea Stats: 468 lines in 4 files changed: 268 ins; 114 del; 86 mod 8287522: StringConcatFactory: Add in prependers and mixers in batches Reviewed-by: jlaskey, mchung - PR: https://git.openjdk.java.net/jdk/pull/8855
Re: [External] : Re: RFR: 8286850: [macos] Add support for signing user provided app image [v2]
> On Jun 7, 2022, at 9:21 PM, Alexander Matveev > wrote: > > Hi Michael, > > Yes, this is correct. It is a three step process as you outlined it below. > Alexander, Could you post an example of the three invocations, without needing to include any post-processing, to 1) create app-image 2) sign 3) add to DMG Or indicate any tests included, or that will be included, in the jdk source where something similar is done. There are not yet that I know of any documentation pages for the command? Thanks, Mike
Re: JDK-8186958 - Behaviour of large values for numMapping in WeakHashMap.newWeakHashMap API
On 07/06/22 9:39 am, Stuart Marks wrote: Hi Jai, The error java.lang.OutOfMemoryError: Java heap space indicates that the VM really has run out of memory. Presumably if you increased the heap size, it would actually be able to allocate that memory. You might have to add the /othervm test directive and add JVM options to require a larger heap. The table size must be a power of two, so the largest table size that will be allocated is 1 << 30 or 1073741824 as you noted. That will take about 8GB of heap (in the no-compressed-OOP case). That's not terribly large, but we might want to check to see if there are other tests that require that much memory. As you also noted, WeakHashMap eagerly allocates its table whereas LinkedHashMap and HashMap do not. I think this is an acceptable behavior variation. Note that we had to avoid this case in WhiteboxResizeTest: https://github.com/openjdk/jdk/blob/master/test/jdk/java/util/HashMap/WhiteBoxResizeTest.java#L167 We might have to make similar special cases here for WHM. I don't think we need to document this behavior difference. More precisely: this kind of implementation variation doesn't need to be specified. In the future we might change WHM to allocate lazily. The API should accommodate extremely large values of numMappings. Even if it's larger than 1 << 30 and the table size is allocated at 1 << 30, it's still possible to add numMappings mappings without resizing. (Memory permitting, of course.) Doing so will violate the load factor invariant, and it might result in more collisions than one would like, but it should still work. Thank you for those inputs, Stuart. I think we just need to decide whether we want to have a test that allocates this much memory, and if so, to apply the necessary settings to make sure the JVM has enough heap. As a start, I've removed the large value testing in these basic tests and moved the PR out of the draft state. -Jaikiran
RFR: 8285405: add test and check for negative argument to HashMap::newHashMap et al
Can I please get a review of this change which addresses https://bugs.openjdk.java.net/browse/JDK-8285405? I've added the test for `LinkedHashMap.newLinkedHashMap(int)` in the existing `test/jdk/java/util/LinkedHashMap/Basic.java` since that test has tests for various APIs of this class. For `WeakHashMap.newWeakHashMap` and `HashMap.newHashMap`, I have created new test classes under relevant locations, since these classes already have test classes (almost) per API/feature in those locations. - Commit messages: - 8285405: add test and check for negative argument to HashMap::newHashMap et al Changes: https://git.openjdk.java.net/jdk/pull/9036/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=9036=00 Issue: https://bugs.openjdk.org/browse/JDK-8285405 Stats: 189 lines in 6 files changed: 186 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/9036.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/9036/head:pull/9036 PR: https://git.openjdk.java.net/jdk/pull/9036
