Re: RFR: JDK-8267936: PreserveAllAnnotations option isn't tested
On 2/06/2021 3:47 pm, Jaroslav Tulach wrote: On Tue, 1 Jun 2021 09:30:40 GMT, Jaroslav Tulach wrote: There doesn't seem to be much support for the complete changes in #4245. To get at least something useful from that endeavor I have extracted the test for existing behavior of `-XX:+PreserveAllAnnotations` and I am offering it in this pull request without any changes to the JVM behavior. Right Peter, the `AnnotationTypeChangedToRuntimeTest` mimics closely the use-case: JVM as a late-binding runtime ... There are exceptions to the rule such as compile-time constants, ... and also annotation uses where the information from one source file (annotation retention) is baked into compilation artifacts of other source files (`RuntimeVisibleAnnotations` vs. `RuntimeInvisibleAnnotations`). `PreserveAllAnnotations` option helps to overcome the situation Great formulation of the problem. The late binding allows people to ignore time of compilation when thinking about the running system. Ignoring time makes the mental model of the overall system easier. But when certain information is _baked_ elsewhere, ignoring time is may no longer be possible as the sequence of actions becomes important - an up to date system may see relics of the past (old values of compile-time constants and annotation not being visible even their most recent retention is `RUNTIME`). This PR isn't going to modify behavior of the `-XX:+PreserveAllAnnotations` option in any way. It only provides a test. Having a test is better than having none. Can we consider this PR reviewed? Can somebody with enough merit mark this _change as properly reviewed_? Can somebody restart the _Windows aarch64 - Build_ - the error seems unrelated? Sorry Jaroslav but I don't really see this test as a basic functional test of the PreserveAllAnnotations flag. There is no need for any dynamic retention mode switch. All you need as I've said previously is a class with all the CLASS retention annotations of interest (8 IIRC) applied and a programs that reads them, and either expects to find them or not, based on the PreserveAllAnnotations flag. I get that you are just trying to not waste a test you already developed. The Windows build issue seems to be a software configuration issue on the Github machines and is outside of our control. I've no idea how to report issues to the Github folk. David - PR: https://git.openjdk.java.net/jdk/pull/4280
Re: RFR: JDK-8267936: PreserveAllAnnotations option isn't tested
On Tue, 1 Jun 2021 09:30:40 GMT, Jaroslav Tulach wrote: > There doesn't seem to be much support for the complete changes in #4245. To > get at least something useful from that endeavor I have extracted the test > for existing behavior of `-XX:+PreserveAllAnnotations` and I am offering it > in this pull request without any changes to the JVM behavior. Right Peter, the `AnnotationTypeChangedToRuntimeTest` mimics closely the use-case: > JVM as a late-binding runtime ... There are exceptions to the rule such as compile-time constants, ... and also annotation uses where the information from one source file (annotation retention) is baked into compilation artifacts of other source files (`RuntimeVisibleAnnotations` vs. `RuntimeInvisibleAnnotations`). `PreserveAllAnnotations` option helps to overcome the situation Great formulation of the problem. The late binding allows people to ignore time of compilation when thinking about the running system. Ignoring time makes the mental model of the overall system easier. But when certain information is _baked_ elsewhere, ignoring time is may no longer be possible as the sequence of actions becomes important - an up to date system may see relics of the past (old values of compile-time constants and annotation not being visible even their most recent retention is `RUNTIME`). This PR isn't going to modify behavior of the `-XX:+PreserveAllAnnotations` option in any way. It only provides a test. Having a test is better than having none. Can we consider this PR reviewed? Can somebody with enough merit mark this _change as properly reviewed_? Can somebody restart the _Windows aarch64 - Build_ - the error seems unrelated? - PR: https://git.openjdk.java.net/jdk/pull/4280
Re: RFR: JDK-8267936: PreserveAllAnnotations option isn't tested
On 01/06/2021 22:28, John Rose wrote: On Jun 1, 2021, at 7:02 AM, Brian Goetz mailto:brian.go...@oracle.com>> wrote: As Alan's archaeology discovered, this flag appears to be a leftover from the original implementation, and I could find no signs of its usage. Note to self and other reviewers of future versions of the JVMS: When you see proposed language like the “unless…” of JVMS 4.7.17, remember today's conversation and try to avoid putting dark corners like that into the JVMS. The RuntimeInvisibleAnnotations attribute is similar to the RuntimeVisibleAnnotations attribute (§4.7.16), except that the annotations represented by a RuntimeInvisibleAnnotations attribute must not be made available for return by reflective APIs, [[WE SHOULD HAVE STOPPED HERE]] unless the Java Virtual Machine has been instructed to retain these annotations via some implementation-specific mechanism such as a command line flag. In the absence of such instructions, the Java Virtual Machine ignores this attribute. https://docs.oracle.com/javase/specs/jvms/se16/html/jvms-4.html#jvms-4.7.17 Hi, I do think this option, in current form, is useful as an escape hatch for exactly the case that Jaroslav has. Java/JVM as a late-binding runtime does a very good job of keeping the information from source files local in direct compilation artefacts which facilitates separate compilation where changes to and recompilation of one source file have immediate effect on the whole app. There are exceptions to the rule such as compile-time constants, ... and also annotation uses where the information from one source file (annotation retention) is baked into compilation artefacts of other source files (RuntimeVisibleAnnotations vs. RuntimeInvisibleAnnotations). PreserveAllAnnotations option helps to overcome the situation where the annotation has been updated but classes where such annotation is used have not been recompiled (yet). I see the two class file attributes merely as a runtime optimization while both kinds of annotations could be kept in a single attribute. The JVM option just disables this optimization. So I would still keep the option. Regards, Peter
Integrated: 8266559: XPathEvaluationResult.XPathResultType.NODESET maps to incorrect type
On Sat, 29 May 2021 00:12:09 GMT, Joe Wang wrote:
> Makes a correction to XPathEvaluationResult.XPathResultType.NODESET mapping.
> Clarifies the supported types for the evaluateExpression methods.
>
> Other changes were javadoc tag usages, e.g. s/the code tag/{@code
This pull request has now been integrated.
Changeset: 7530c00b
Author:Joe Wang
URL:
https://git.openjdk.java.net/jdk/commit/7530c00b33aac8918841dbae4d928956b60c261f
Stats: 77 lines in 4 files changed: 28 ins; 0 del; 49 mod
8266559: XPathEvaluationResult.XPathResultType.NODESET maps to incorrect type
Reviewed-by: lancea, naoto
-
PR: https://git.openjdk.java.net/jdk/pull/4258
Re: RFR: 8265518: C1: Intrinsic support for Preconditions.checkIndex [v11]
> The JDK codebase re-created many variants of checkIndex(`grep -I -r > 'cehckIndex' jdk/`). A notable variant is java.nio.Buffer.checkIndex, which > annotated with @IntrinsicCandidate and it only has a corresponding C1 > intrinsic version. > > In fact, there is an utility method > `jdk.internal.util.Preconditions.checkIndex`(wrapped by > java.lang.Objects.checkIndex) that behaves the same as these variants of > checkIndex, we can replace these re-created variants of checkIndex by > Objects.checkIndex, it would significantly reduce duplicated code and enjoys > performance improvement because Preconditions.checkIndex is > @IntrinsicCandidate and it has a corresponding intrinsic method in HotSpot. > > But, the problem is currently HotSpot only implements the C2 version of > Preconditions.checkIndex. To reuse it global-widely in JDK code, I think we > can firstly implement its C1 counterpart. There are also a few kinds of stuff > we can do later: > > 1. Replace all variants of checkIndex by Objects.checkIndex in the whole JDK > codebase. > 2. Remove Buffer.checkIndex and obsolete/deprecate InlineNIOCheckIndex flag > > Testing: cds, compiler and jdk Yi Yang has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains eight commits: - x86_32 fails - build failed - cmp clobbers its left argument on x86_32 - better check1-4 - AssertionError when expected exception was not thrown - remove InlineNIOCheckIndex flag - remove java_nio_Buffer in javaClasses.hpp - consolidate - Changes: https://git.openjdk.java.net/jdk/pull/3615/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3615=10 Stats: 338 lines in 11 files changed: 242 ins; 78 del; 18 mod Patch: https://git.openjdk.java.net/jdk/pull/3615.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3615/head:pull/3615 PR: https://git.openjdk.java.net/jdk/pull/3615
Re: RFR: 8265518: C1: Intrinsic support for Preconditions.checkIndex [v10]
> The JDK codebase re-created many variants of checkIndex(`grep -I -r > 'cehckIndex' jdk/`). A notable variant is java.nio.Buffer.checkIndex, which > annotated with @IntrinsicCandidate and it only has a corresponding C1 > intrinsic version. > > In fact, there is an utility method > `jdk.internal.util.Preconditions.checkIndex`(wrapped by > java.lang.Objects.checkIndex) that behaves the same as these variants of > checkIndex, we can replace these re-created variants of checkIndex by > Objects.checkIndex, it would significantly reduce duplicated code and enjoys > performance improvement because Preconditions.checkIndex is > @IntrinsicCandidate and it has a corresponding intrinsic method in HotSpot. > > But, the problem is currently HotSpot only implements the C2 version of > Preconditions.checkIndex. To reuse it global-widely in JDK code, I think we > can firstly implement its C1 counterpart. There are also a few kinds of stuff > we can do later: > > 1. Replace all variants of checkIndex by Objects.checkIndex in the whole JDK > codebase. > 2. Remove Buffer.checkIndex and obsolete/deprecate InlineNIOCheckIndex flag > > Testing: cds, compiler and jdk Yi Yang has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains ten commits: - x86_32 fails - build failed - cmp clobbers its left argument on x86_32 - better check1-4 - AssertionError when expected exception was not thrown - remove extra newline - remove InlineNIOCheckIndex flag - remove java_nio_Buffer in javaClasses.hpp - consolidate - Changes: https://git.openjdk.java.net/jdk/pull/3615/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3615=09 Stats: 338 lines in 11 files changed: 242 ins; 78 del; 18 mod Patch: https://git.openjdk.java.net/jdk/pull/3615.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3615/head:pull/3615 PR: https://git.openjdk.java.net/jdk/pull/3615
Re: RFR: 8267893: Improve jtreg test failure handler do get native/mixed stack traces for cores and live processes
On Fri, 28 May 2021 00:54:21 GMT, Leonid Mesnik wrote: >> Hi Leonid, >> >> What is EFH? Please update the bug and PR to explain. >> >> Thanks, >> David > >> Hi Leonid, >> >> What is EFH? Please update the bug and PR to explain. >> >> Thanks, >> David > > Updated summary to "Improve jtreg test failure handler do get native/mixed > stack traces for cores and live processes". > @lmesnik , how has this been tested? I used it in the loom for several weeks. - PR: https://git.openjdk.java.net/jdk/pull/4234
Re: RFR: 8267969: Add vectorized implementation for VectorMask.eq() [v2]
> Currently `"VectorMask.eq()" `is not vectorized:
>
> public VectorMask eq(VectorMask m) {
> // FIXME: Generate good code here.
> return bOp(m, (i, a, b) -> a == b);
> }
>
> This can be implemented by calling `"xor(m.not())"` directly.
>
> The performance improved about 1.4x ~ 1.9x for the following benchmark with
> different basic types:
>
> @Benchmark
> public Object eq() {
> boolean[] ma = fm.apply(size);
> boolean[] mb = fmb.apply(size);
> boolean[] mt = fmt.apply(size);
> VectorMask m = VectorMask.fromArray(SPECIES, mt, 0);
>
> for (int ic = 0; ic < INVOC_COUNT; ic++) {
> for (int i = 0; i < ma.length; i += SPECIES.length()) {
> var av = SPECIES.loadMask(ma, i);
> var bv = SPECIES.loadMask(mb, i);
>
> // accumulate results, so JIT can't eliminate relevant
> computations
> m = m.and(av.eq(bv));
> }
> }
>
> return m;
> }
Xiaohong Gong 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 'jdk:master' into JDK-8267969
- 8267969: Add vectorized implementation for VectorMask.eq()
-
Changes:
- all: https://git.openjdk.java.net/jdk/pull/4272/files
- new: https://git.openjdk.java.net/jdk/pull/4272/files/0e8e0e84..f3a48026
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk=4272=01
- incr: https://webrevs.openjdk.java.net/?repo=jdk=4272=00-01
Stats: 22530 lines in 577 files changed: 2836 ins; 18744 del; 950 mod
Patch: https://git.openjdk.java.net/jdk/pull/4272.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/4272/head:pull/4272
PR: https://git.openjdk.java.net/jdk/pull/4272
Re: RFR: 8267969: Add vectorized implementation for VectorMask.eq()
On Tue, 1 Jun 2021 16:29:58 GMT, Paul Sandoz wrote: > Looks. Later we may want to consider pushing this down as an intrinsic, > perhaps reusing `VectorSupport.compare`. Thanks for your review @PaulSandoz ! Yes, reusing `VectorSupport.compare` is an alternative way to do the vectorization. - PR: https://git.openjdk.java.net/jdk/pull/4272
Re: RFR: 8267893: Improve jtreg test failure handler do get native/mixed stack traces for cores and live processes [v5]
On Fri, 28 May 2021 02:25:59 GMT, Igor Ignatyev wrote: >> Leonid Mesnik has updated the pull request incrementally with one additional >> commit since the last revision: >> >> spaces updated. > > @lmesnik , how has this been tested? @iignatev, thank you for your comments. I updated all of them. - PR: https://git.openjdk.java.net/jdk/pull/4234
RFR: 8199318: add idempotent copy operation for Map.Entry
I'm fixing this along with a couple intertwined issues. 1. Add Map.Entry::copyOf as an idempotent copy operation. 2. Clarify that AbstractMap.SimpleImmutableEntry is itself unmodifiable (not really immutable) but that subclasses can be modifiable. 3. Clarify some confusing, historical wording about Map.Entry instances being obtainable only via iteration of a Map's entry-set view. This was never really true, since anyone could implement the Map.Entry interface, and it certainly was not true since JDK 1.6 when SimpleEntry and SimpleImmutableEntry were added. - Commit messages: - More spec and test tweaks. - Fix up tests. - Spec wordsmithing. - Update specs. Add basic tests. - 8199318: add idempotent copy operation for Map.Entry Changes: https://git.openjdk.java.net/jdk/pull/4295/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4295=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8199318 Stats: 141 lines in 3 files changed: 120 ins; 2 del; 19 mod Patch: https://git.openjdk.java.net/jdk/pull/4295.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4295/head:pull/4295 PR: https://git.openjdk.java.net/jdk/pull/4295
Re: RFR: 8267893: Improve jtreg test failure handler do get native/mixed stack traces for cores and live processes [v5]
On Fri, 28 May 2021 02:20:04 GMT, Igor Ignatyev wrote: >> Leonid Mesnik has updated the pull request incrementally with one additional >> commit since the last revision: >> >> spaces updated. > > test/failure_handler/Makefile line 41: > >> 39: CONF_DIR = src/share/conf >> 40: >> 41: JAVA_RELEASE = 7 > > could you please update `DEPENDENCES` section in > `test/failure_handler/README`? done - PR: https://git.openjdk.java.net/jdk/pull/4234
Re: RFR: 8267893: Improve jtreg test failure handler do get native/mixed stack traces for cores and live processes [v5]
> EFH is improved to process cores and get mixed stack traces with jhsdb and > native stack traces with gdb/lldb. It might be useful because hs_err doesn't > contain info about all threads, sometimes it is even not generated. Leonid Mesnik has updated the pull request incrementally with one additional commit since the last revision: spaces updated. - Changes: - all: https://git.openjdk.java.net/jdk/pull/4234/files - new: https://git.openjdk.java.net/jdk/pull/4234/files/57d76163..e70518bc Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4234=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4234=03-04 Stats: 2 lines in 1 file changed: 0 ins; 1 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/4234.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4234/head:pull/4234 PR: https://git.openjdk.java.net/jdk/pull/4234
Re: RFR: 8267706: bin/idea.sh tries to use cygpath on WSL [v2]
On Wed, 2 Jun 2021 00:19:56 GMT, Jonathan Gibbons wrote: >> @jonathan-gibbons this can be fixed with `brew install coreutils`. We >> probably need to check `realpath` availability in idea.sh and suggest >> installing `coreutils` if it's not available > > @YaaZ I'm aware of the workaround, but the current situation is not > acceptable and needs to be fixed. A change to fix functionality on Windows > should not adversely affect users on other platforms. > > @erikj79 is there precedent for requiring the use of `brew` to install > packages? @jonathan-gibbons sure, but these changes also improve project setup process on all platforms, so realpath is required here because it's needed on MacOS as well, not only Windows Also, `brew` is already required to instal `autoconf`: https://openjdk.java.net/groups/build/doc/building.html#autoconf As `idea.sh` only works when project is configured, running it implies that `brew` is already installed, so asking user to install `coreutils` is not a big deal IMO - PR: https://git.openjdk.java.net/jdk/pull/4190
Integrated: 8265130: Make ConstantDesc class hierarchy sealed
On Thu, 20 May 2021 21:07:04 GMT, Gavin Bierman wrote: > Hi all, > > Please review this patch to make the ConstantDesc hierarchy `sealed`, as was > promised in its Javadoc, now that sealed classes are finalising in JDK 17. > > Thanks, > Gavin This pull request has now been integrated. Changeset: 379376f0 Author:Gavin Bierman Committer: Vicente Romero URL: https://git.openjdk.java.net/jdk/commit/379376f0783facba93e1d11db9b184ef8183a13b Stats: 54 lines in 6 files changed: 16 ins; 29 del; 9 mod 8265130: Make ConstantDesc class hierarchy sealed Reviewed-by: mchung, jvernee, vromero - PR: https://git.openjdk.java.net/jdk/pull/4135
Re: RFR: 8267893: Improve jtreg test failure handler do get native/mixed stack traces for cores and live processes [v4]
> EFH is improved to process cores and get mixed stack traces with jhsdb and > native stack traces with gdb/lldb. It might be useful because hs_err doesn't > contain info about all threads, sometimes it is even not generated. Leonid Mesnik has updated the pull request incrementally with one additional commit since the last revision: remove unused code. - Changes: - all: https://git.openjdk.java.net/jdk/pull/4234/files - new: https://git.openjdk.java.net/jdk/pull/4234/files/c48542b5..57d76163 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4234=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4234=02-03 Stats: 4 lines in 1 file changed: 0 ins; 3 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/4234.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4234/head:pull/4234 PR: https://git.openjdk.java.net/jdk/pull/4234
Re: RFR: 8267893: Improve jtreg test failure handler do get native/mixed stack traces for cores and live processes [v3]
> EFH is improved to process cores and get mixed stack traces with jhsdb and > native stack traces with gdb/lldb. It might be useful because hs_err doesn't > contain info about all threads, sometimes it is even not generated. Leonid Mesnik has updated the pull request incrementally with one additional commit since the last revision: README updated. - Changes: - all: https://git.openjdk.java.net/jdk/pull/4234/files - new: https://git.openjdk.java.net/jdk/pull/4234/files/cb1eb944..c48542b5 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4234=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4234=01-02 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/4234.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4234/head:pull/4234 PR: https://git.openjdk.java.net/jdk/pull/4234
Re: RFR: 8267893: Improve jtreg test failure handler do get native/mixed stack traces for cores and live processes [v2]
> EFH is improved to process cores and get mixed stack traces with jhsdb and > native stack traces with gdb/lldb. It might be useful because hs_err doesn't > contain info about all threads, sometimes it is even not generated. Leonid Mesnik has updated the pull request incrementally with two additional commits since the last revision: - Merge branch 'efh' of https://github.com/lmesnik/jdk into efh - updated after comments from Igor - Changes: - all: https://git.openjdk.java.net/jdk/pull/4234/files - new: https://git.openjdk.java.net/jdk/pull/4234/files/68fd69d9..cb1eb944 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4234=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4234=00-01 Stats: 47 lines in 7 files changed: 9 ins; 31 del; 7 mod Patch: https://git.openjdk.java.net/jdk/pull/4234.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4234/head:pull/4234 PR: https://git.openjdk.java.net/jdk/pull/4234
Re: RFR: 8267706: bin/idea.sh tries to use cygpath on WSL [v2]
On Tue, 1 Jun 2021 22:20:25 GMT, Nikita Gubarkov wrote: >> The fix fails on a Mac, where `realpath` is not available by default. > > @jonathan-gibbons this can be fixed with `brew install coreutils`. We > probably need to check `realpath` availability in idea.sh and suggest > installing `coreutils` if it's not available @YaaZ I'm aware of the workaround, but the current situation is not acceptable and needs to be fixed. @erikj79 is there precedent for requiring the use of `brew` to install packages? - PR: https://git.openjdk.java.net/jdk/pull/4190
Re: RFR: JDK-8267598: jpackage removes system libraries from java.library.path
On Wed, 26 May 2021 11:26:24 GMT, Andy Herrick wrote: > JDK-8267598: jpackage removes system libraries from java.library.path Marked as reviewed by almatvee (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4203
Re: RFR: 8267706: bin/idea.sh tries to use cygpath on WSL [v2]
On Tue, 1 Jun 2021 22:10:41 GMT, Jonathan Gibbons wrote: >> Nikita Gubarkov has updated the pull request incrementally with one >> additional commit since the last revision: >> >> 8267706: Break long line in make/ide/idea/jdk/idea.gmk > > The fix fails on a Mac, where `realpath` is not available by default. @jonathan-gibbons this can be fixed with `brew install coreutils`. We probably need to check `realpath` availability in idea.sh and suggest installing `coreutils` if it's not available - PR: https://git.openjdk.java.net/jdk/pull/4190
Re: RFR: 8267706: bin/idea.sh tries to use cygpath on WSL [v2]
On Thu, 27 May 2021 17:45:40 GMT, Nikita Gubarkov wrote: >> 8267706: bin/idea.sh tries to use cygpath on WSL > > Nikita Gubarkov has updated the pull request incrementally with one > additional commit since the last revision: > > 8267706: Break long line in make/ide/idea/jdk/idea.gmk The fix fails on a Mac, where `realpath` is not available by default. - PR: https://git.openjdk.java.net/jdk/pull/4190
Re: RFR: 8266559: XPathEvaluationResult.XPathResultType.NODESET maps to incorrect type [v2]
On Tue, 1 Jun 2021 20:29:30 GMT, Joe Wang wrote:
>> Makes a correction to XPathEvaluationResult.XPathResultType.NODESET mapping.
>> Clarifies the supported types for the evaluateExpression methods.
>>
>> Other changes were javadoc tag usages, e.g. s/the code tag/{@code
>
> Joe Wang has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Put Number in @code tag. Thanks Naoto.
Marked as reviewed by naoto (Reviewer).
-
PR: https://git.openjdk.java.net/jdk/pull/4258
Re: RFR: 8266559: XPathEvaluationResult.XPathResultType.NODESET maps to incorrect type [v2]
> Makes a correction to XPathEvaluationResult.XPathResultType.NODESET mapping.
> Clarifies the supported types for the evaluateExpression methods.
>
> Other changes were javadoc tag usages, e.g. s/the code tag/{@code
Joe Wang has updated the pull request incrementally with one additional commit
since the last revision:
Put Number in @code tag. Thanks Naoto.
-
Changes:
- all: https://git.openjdk.java.net/jdk/pull/4258/files
- new: https://git.openjdk.java.net/jdk/pull/4258/files/29dd4923..8ffd4ea3
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk=4258=01
- incr: https://webrevs.openjdk.java.net/?repo=jdk=4258=00-01
Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
Patch: https://git.openjdk.java.net/jdk/pull/4258.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/4258/head:pull/4258
PR: https://git.openjdk.java.net/jdk/pull/4258
Re: RFR: JDK-8267936: PreserveAllAnnotations option isn't tested
On Jun 1, 2021, at 7:02 AM, Brian Goetz mailto:brian.go...@oracle.com>> wrote: As Alan's archaeology discovered, this flag appears to be a leftover from the original implementation, and I could find no signs of its usage. Note to self and other reviewers of future versions of the JVMS: When you see proposed language like the “unless…” of JVMS 4.7.17, remember today's conversation and try to avoid putting dark corners like that into the JVMS. The RuntimeInvisibleAnnotations attribute is similar to the RuntimeVisibleAnnotations attribute (§4.7.16), except that the annotations represented by a RuntimeInvisibleAnnotations attribute must not be made available for return by reflective APIs, [[WE SHOULD HAVE STOPPED HERE]] unless the Java Virtual Machine has been instructed to retain these annotations via some implementation-specific mechanism such as a command line flag. In the absence of such instructions, the Java Virtual Machine ignores this attribute. https://docs.oracle.com/javase/specs/jvms/se16/html/jvms-4.html#jvms-4.7.17
Re: RFR: 8262891: Compiler implementation for Pattern Matching for switch (Preview) [v8]
On Tue, 1 Jun 2021 07:50:49 GMT, Jan Lahoda wrote: >> This is a preview of a patch implementing JEP 406: Pattern Matching for >> switch (Preview): >> https://bugs.openjdk.java.net/browse/JDK-8213076 >> >> The current draft of the specification is here: >> http://cr.openjdk.java.net/~gbierman/jep406/jep406-20210430/specs/patterns-switch-jls.html >> >> A summary of notable parts of the patch: >> -to support cases expressions and patterns in cases, there is a new common >> superinterface for expressions and patterns, `CaseLabelTree`, which >> expressions and patterns implement, and a list of case labels is returned >> from `CaseTree.getLabels()`. >> -to support `case default`, there is an implementation of `CaseLabelTree` >> that represents it (`DefaultCaseLabelTree`). It is used also to represent >> the conventional `default` internally and in the newly added methods. >> -in the parser, parenthesized patterns and expressions need to be >> disambiguated when parsing case labels. >> -Lower has been enhanced to handle `case null` for ordinary >> (boxed-primitive, String, enum) switches. This is a bit tricky for boxed >> primitives, as there is no value that is not part of the input domain so >> that could be used to represent `case null`. Requires a bit shuffling with >> values. >> -TransPatterns has been enhanced to handle the pattern matching switch. It >> produces code that delegates to a new bootstrap method, that will classify >> the input value to the switch and return the case number, to which the >> switch then jumps. To support guards, the switches (and the bootstrap >> method) are restartable. The bootstrap method as such is written very simply >> so far, but could be much more optimized later. >> -nullable type patterns are `case String s, null`/`case null, String >> s`/`case null: case String s:`/`case String s: case null:`, handling of >> these required a few tricks in `Attr`, `Flow` and `TransPatterns`. >> >> The specdiff for the change is here (to be updated): >> http://cr.openjdk.java.net/~jlahoda/8265981/specdiff.preview.01/overview-summary.html > > Jan Lahoda has updated the pull request incrementally with one additional > commit since the last revision: > > Fixing enum switch with patterns with guards. New changes looks good. I suggest to also add tests for strings in switch with guards and numeric with guards, to make sure every kind of switch works well. As discussed offline, we can probably simplify code generation logic for enum switches by leaning on the ConstantBootstraps BSM which allows to create enum constants given a class name and a constant name. - PR: https://git.openjdk.java.net/jdk/pull/3863
RFR: 8268077: java.util.List missing from Collections Framework Overview
Trivial changes to this non-normative document. - Commit messages: - 8268077: java.util.List missing from Collections Framework Overview Changes: https://git.openjdk.java.net/jdk/pull/4289/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4289=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8268077 Stats: 7 lines in 1 file changed: 2 ins; 0 del; 5 mod Patch: https://git.openjdk.java.net/jdk/pull/4289.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4289/head:pull/4289 PR: https://git.openjdk.java.net/jdk/pull/4289
Re: RFR: 8266559: XPathEvaluationResult.XPathResultType.NODESET maps to incorrect type
On Sat, 29 May 2021 00:12:09 GMT, Joe Wang wrote:
> Makes a correction to XPathEvaluationResult.XPathResultType.NODESET mapping.
> Clarifies the supported types for the evaluateExpression methods.
>
> Other changes were javadoc tag usages, e.g. s/the code tag/{@code
src/java.xml/share/classes/javax/xml/xpath/package-info.java line 276:
> 274: *
> 275: *
> 276: * Of the subtypes of Number, only {@code Double, Integer} and {@code
> Long} are supported.
Nit: `Number` may also be in @code emphasis.
-
PR: https://git.openjdk.java.net/jdk/pull/4258
Re: RFR: 8266559: XPathEvaluationResult.XPathResultType.NODESET maps to incorrect type
On Sat, 29 May 2021 00:12:09 GMT, Joe Wang wrote:
> Makes a correction to XPathEvaluationResult.XPathResultType.NODESET mapping.
> Clarifies the supported types for the evaluateExpression methods.
>
> Other changes were javadoc tag usages, e.g. s/the code tag/{@code
Marked as reviewed by lancea (Reviewer).
-
PR: https://git.openjdk.java.net/jdk/pull/4258
RFR: JDK-8267598: jpackage removes system libraries from java.library.path
JDK-8267598: jpackage removes system libraries from java.library.path - Commit messages: - JDK-8267598: jpackage removes system libraries from java.library.path - JDK-8267598: jpackage removes system libraries from java.library.path - JDK-8267598: jpackage removes system libraries from java.library.path - JDK-8267598: jpackage removes system libraries from java.library.path - JDK-8267598: jpackage removes system libraries from java.library.path - JDK-8267598: jpackage removes system libraries from java.library.path Changes: https://git.openjdk.java.net/jdk/pull/4203/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4203=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8267598 Stats: 49 lines in 5 files changed: 45 ins; 3 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/4203.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4203/head:pull/4203 PR: https://git.openjdk.java.net/jdk/pull/4203
Re: RFR: 8266054: VectorAPI rotate operation optimization [v7]
On Mon, 24 May 2021 05:50:44 GMT, Jatin Bhateja wrote: >> Current VectorAPI Java side implementation expresses rotateLeft and >> rotateRight operation using following operations:- >> >> vec1 = lanewise(VectorOperators.LSHL, n) >> vec2 = lanewise(VectorOperators.LSHR, n) >> res = lanewise(VectorOperations.OR, vec1 , vec2) >> >> This patch moves above handling from Java side to C2 compiler which >> facilitates dismantling the rotate operation if target ISA does not support >> a direct rotate instruction. >> >> AVX512 added vector rotate instructions vpro[rl][v][dq] which operate over >> long and integer type vectors. For other cases (i.e. sub-word type vectors >> or for targets which do not support direct rotate operations ) instruction >> sequence comprising of vector SHIFT (LEFT/RIGHT) and vector OR is emitted. >> >> Please find below the performance data for included JMH benchmark. >> Machine: Cascade Lake Server (Intel(R) Xeon(R) Platinum 8280 CPU @ 2.70GHz) >> >> >> Benchmark | (TESTSIZE) | Shift | Baseline AVX3 (ops/ms) | Withopt AVX3 >> (ops/ms) | Gain % | Baseline AVX2 (ops/ms) | Withopt AVX2 (ops/ms) | Gain % >> -- | -- | -- | -- | -- | -- | -- | -- | -- >> | | | | | | | | >> RotateBenchmark.testRotateLeftB | 128.00 | 7.00 | 17223.35 | 17094.69 | >> -0.75 | 17008.32 | 17488.06 | 2.82 >> RotateBenchmark.testRotateLeftB | 128.00 | 7.00 | 8944.98 | 8811.34 | -1.49 >> | 8878.17 | 9218.68 | 3.84 >> RotateBenchmark.testRotateLeftB | 128.00 | 15.00 | 17195.75 | 17137.32 | >> -0.34 | 16789.01 | 17780.34 | 5.90 >> RotateBenchmark.testRotateLeftB | 128.00 | 15.00 | 9052.67 | 8838.60 | -2.36 >> | 8814.62 | 9206.01 | 4.44 >> RotateBenchmark.testRotateLeftB | 128.00 | 31.00 | 17100.19 | 16950.64 | >> -0.87 | 16827.73 | 17720.37 | 5.30 >> RotateBenchmark.testRotateLeftB | 128.00 | 31.00 | 9079.95 | 8471.26 | -6.70 >> | .44 | 9167.68 | 3.14 >> RotateBenchmark.testRotateLeftB | 256.00 | 7.00 | 21231.33 | 21513.08 | 1.33 >> | 21824.51 | 21479.48 | -1.58 >> RotateBenchmark.testRotateLeftB | 256.00 | 7.00 | 11103.62 | 11180.16 | 0.69 >> | 11173.67 | 11529.22 | 3.18 >> RotateBenchmark.testRotateLeftB | 256.00 | 15.00 | 21119.14 | 21552.04 | >> 2.05 | 21693.05 | 21915.37 | 1.02 >> RotateBenchmark.testRotateLeftB | 256.00 | 15.00 | 11048.68 | 11094.20 | >> 0.41 | 11049.90 | 11439.07 | 3.52 >> RotateBenchmark.testRotateLeftB | 256.00 | 31.00 | 21506.31 | 21391.41 | >> -0.53 | 21263.18 | 21986.29 | 3.40 >> RotateBenchmark.testRotateLeftB | 256.00 | 31.00 | 11056.12 | 11232.78 | >> 1.60 | 10941.59 | 11397.09 | 4.16 >> RotateBenchmark.testRotateLeftB | 512.00 | 7.00 | 17976.56 | 18180.85 | 1.14 >> | 1212.26 | 2533.34 | 108.98 >> RotateBenchmark.testRotateLeftB | 512.00 | 15.00 | 17553.70 | 18219.07 | >> 3.79 | 1256.73 | 2537.41 | 101.91 >> RotateBenchmark.testRotateLeftB | 512.00 | 31.00 | 17618.03 | 17738.15 | >> 0.68 | 1214.69 | 2533.83 | 108.60 >> RotateBenchmark.testRotateLeftI | 128.00 | 7.00 | 7258.87 | 7468.88 | 2.89 | >> 7115.12 | 7117.26 | 0.03 >> RotateBenchmark.testRotateLeftI | 128.00 | 7.00 | 3586.65 | 3950.85 | 10.15 >> | 3532.17 | 3595.80 | 1.80 >> RotateBenchmark.testRotateLeftI | 128.00 | 7.00 | 1835.07 | 1999.68 | 8.97 | >> 1789.90 | 1819.93 | 1.68 >> RotateBenchmark.testRotateLeftI | 128.00 | 15.00 | 7273.36 | 7410.91 | 1.89 >> | 7198.60 | 6994.79 | -2.83 >> RotateBenchmark.testRotateLeftI | 128.00 | 15.00 | 3674.98 | 3926.27 | 6.84 >> | 3549.90 | 3755.09 | 5.78 >> RotateBenchmark.testRotateLeftI | 128.00 | 15.00 | 1840.94 | 1882.25 | 2.24 >> | 1801.56 | 1872.89 | 3.96 >> RotateBenchmark.testRotateLeftI | 128.00 | 31.00 | 7457.11 | 7361.48 | -1.28 >> | 6975.33 | 7385.94 | 5.89 >> RotateBenchmark.testRotateLeftI | 128.00 | 31.00 | 3570.74 | 3929.30 | 10.04 >> | 3635.37 | 3736.67 | 2.79 >> RotateBenchmark.testRotateLeftI | 128.00 | 31.00 | 1902.32 | 1960.46 | 3.06 >> | 1812.32 | 1813.88 | 0.09 >> RotateBenchmark.testRotateLeftI | 256.00 | 7.00 | 11174.24 | 12044.52 | 7.79 >> | 11509.87 | 11273.44 | -2.05 >> RotateBenchmark.testRotateLeftI | 256.00 | 7.00 | 5981.47 | 6073.70 | 1.54 | >> 5593.66 | 5661.93 | 1.22 >> RotateBenchmark.testRotateLeftI | 256.00 | 7.00 | 2932.49 | 3069.54 | 4.67 | >> 2950.86 | 2892.42 | -1.98 >> RotateBenchmark.testRotateLeftI | 256.00 | 15.00 | 11764.11 | 12098.63 | >> 2.84 | 11069.52 | 11476.93 | 3.68 >> RotateBenchmark.testRotateLeftI | 256.00 | 15.00 | 5855.20 | 6080.40 | 3.85 >> | 5919.11 | 5607.04 | -5.27 >> RotateBenchmark.testRotateLeftI | 256.00 | 15.00 | 2989.05 | 3048.56 | 1.99 >> | 2902.63 | 2821.83 | -2.78 >> RotateBenchmark.testRotateLeftI | 256.00 | 31.00 | 11652.84 | 11965.40 | >> 2.68 | 11525.62 | 11459.83 | -0.57 >> RotateBenchmark.testRotateLeftI | 256.00 | 31.00 | 5851.82 | 6164.94 | 5.35 >> | 5882.60 | 5842.30 | -0.69 >> RotateBenchmark.testRotateLeftI | 256.00 | 31.00 | 3015.99 | 3043.79 | 0.92 >> | 2963.71 | 2947.97 | -0.53 >> RotateBenchmark.testRotateLeftI | 512.00 | 7.00 |
Re: RFR: 8267569: java.io.File.equals contains misleading Javadoc [v3]
On Tue, 1 Jun 2021 17:43:42 GMT, Brian Burkhalter wrote: >> Modify the specification of `java.io.File.equals` to clarify that equality >> is based only on a comparison of abstract pathnames as opposed to the file >> system objects that the `File`s represent. > > Brian Burkhalter has updated the pull request incrementally with one > additional commit since the last revision: > > 8267569: Change @implNote to @apiNote Marked as reviewed by naoto (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4232
Re: RFR: 8195129: System.load() fails to load from unicode paths [v3]
On Thu, 27 May 2021 16:14:38 GMT, Maxim Kartashev
wrote:
>> Character strings within JVM are produced and consumed in several formats.
>> Strings come from/to Java in the UTF8 format and POSIX APIs (like fprintf()
>> or dlopen()) consume strings also in UTF8. On Windows, however, the
>> situation is far less simple: some new(er) APIs expect UTF16 (wide-character
>> strings), some older APIs can only work with strings in a "platform" format,
>> where not all UTF8 characters can be represented; which ones can depends on
>> the current "code page".
>>
>> This commit switches the Windows version of native library loading code to
>> using the new UTF16 API `LoadLibraryW()` and attempts to streamline the use
>> of various string formats in the surrounding code.
>>
>> Namely, exception messages are made to consume strings explicitly in the
>> UTF8 format, while logging functions (that end up using legacy Windows API)
>> are made to consume "platform" strings in most cases. One exception is
>> `JVM_LoadLibrary()` logging where the UTF8 name of the library is logged,
>> which can, of course, be fixed, but was considered not worth the additional
>> code (NB: this isn't a new bug).
>>
>> The test runs in a separate JVM in order to make NIO happy about non-ASCII
>> characters in the file name; tests are executed with LC_ALL=C and that
>> doesn't let NIO work with non-ASCII file names even on Linux or MacOS.
>>
>> Tested by running `test/hotspot/jtreg:tier1` on Linux and
>> `jtreg:test/hotspot/jtreg/runtime` on Windows 10. The new test (`
>> jtreg:test/hotspot/jtreg/runtime/jni/loadLibraryUnicode`) was explicitly ran
>> on those platforms as well.
>>
>> Results from Linux:
>>
>> Test summary
>> ==
>>TEST TOTAL PASS FAIL ERROR
>>
>>jtreg:test/hotspot/jtreg:tier1 1784 1784 0 0
>>
>> ==
>> TEST SUCCESS
>>
>>
>> Building target 'run-test-only' in configuration
>> 'linux-x86_64-server-release'
>> Test selection 'jtreg:test/hotspot/jtreg/runtime/jni/loadLibraryUnicode',
>> will run:
>> * jtreg:test/hotspot/jtreg/runtime/jni/loadLibraryUnicode
>>
>> Running test 'jtreg:test/hotspot/jtreg/runtime/jni/loadLibraryUnicode'
>> Passed: runtime/jni/loadLibraryUnicode/LoadLibraryUnicodeTest.java
>> Test results: passed: 1
>>
>>
>> Results from Windows 10:
>>
>> Test summary
>> ==
>>TEST TOTAL PASS FAIL ERROR
>>jtreg:test/hotspot/jtreg/runtime746 746 0 0
>> ==
>> TEST SUCCESS
>> Finished building target 'run-test-only' in configuration
>> 'windows-x86_64-server-fastdebug'
>>
>>
>> Building target 'run-test-only' in configuration
>> 'windows-x86_64-server-fastdebug'
>> Test selection 'test/hotspot/jtreg/runtime/jni/loadLibraryUnicode', will run:
>> * jtreg:test/hotspot/jtreg/runtime/jni/loadLibraryUnicode
>>
>> Running test 'jtreg:test/hotspot/jtreg/runtime/jni/loadLibraryUnicode'
>> Passed: runtime/jni/loadLibraryUnicode/LoadLibraryUnicodeTest.java
>> Test results: passed: 1
>
> Maxim Kartashev has updated the pull request incrementally with two
> additional commits since the last revision:
>
> - Coding style-related corrections.
> - Corrected the test to use Platform.sharedLibraryExt()
src/hotspot/os/windows/os_windows.cpp line 1491:
> 1489: static errno_t convert_UTF8_to_UTF16(char const* utf8_str, LPWSTR*
> utf16_str) {
> 1490: return convert_to_UTF16(utf8_str, CP_UTF8, utf16_str);
> 1491: }
IIUC, `utf8_str` is the "modified" UTF-8 string in JNI. Using it as the
standard UTF-8 (I believe Windows' encoding `CP_UTF8` is the one) may end up in
incorrect conversions in some corner cases, e.g., for supplementary characters.
test/hotspot/jtreg/runtime/jni/loadLibraryUnicode/LoadLibraryUnicodeTest.java
line 42:
> 40: String nativePathSetting = "-Dtest.nativepath=" +
> getSystemProperty("test.nativepath");
> 41: ProcessBuilder pb = ProcessTools.createTestJvm(nativePathSetting,
> LoadLibraryUnicode.class.getName());
> 42: pb.environment().put("LC_ALL", "en_US.UTF-8");
Some environments/user configs may not have `UTF-8` codeset on the platform.
May need to gracefully exit in such a case.
-
PR: https://git.openjdk.java.net/jdk/pull/4169
Re: RFR: JDK-8267936: PreserveAllAnnotations option doesn't expose the annotation to Java code
On 5/29/2021 12:20 AM, Jaroslav Tulach wrote: This PR exposes runtime invisible annotations via `Class.getAnnotation` when `-XX:+PreserveAllAnnotations` option is passed to the JVM. Existing `-XX:+PreserveAllAnnotations` option can be very useful for code that needs to access annotations with `RetentionPolicy.CLASS` without the need to parse the .class files manually. While the RuntimeInvisibleAnnotations are kept in the runtime, they are not visible via java.lang.reflect API. I assume that's just an omission. Not having CLASS retention annotations visible through the java.lang.reflect API is not an omission, it is a design choice from JSR 175. If CLASS retention annotations were visible through java.lang.reflec, they would just be RUNTIME retention annotations. -Joe
Re: java.util.List missing from "Collections Framework Overview" javadoc
I have created a JBS issue: https://bugs.openjdk.java.net/browse/JDK-8268077 > On 1 Jun 2021, at 18:56, Raffaello Giulietti > wrote: > > Hi, > > can anybody take a look at this? > I would do it myself but I don't have Author status to add an issue to the > JBS. > > Should be a trivial change. > > Thanks > Raffaello > > > On 2021-05-30 18:03, Raffaello Giulietti wrote: >> Hello, >> seems like java.util.List is missing from the list in the "Collection >> Interfaces" section in >> https://docs.oracle.com/en/java/javase/16/docs/api/java.base/java/util/doc-files/coll-overview.html >> Should be easy to fix and doesn't seem to require a CSR as the document is >> non-normative. >> Greetings >> Raffaello
Re: RFR: 8266317: Vector API enhancements [v4]
> This PR contains API and implementation changes for [JEP-414 Vector API > (Second Incubator)](https://openjdk.java.net/jeps/414), in preparation for > when targeted. > > Enhancements are made to the API for the support of operations on characters, > such as for UTF-8 character decoding. Specifically, methods for > loading/storing a `short` vector from/to a `char[]` array, and new vector > comparison operators for unsigned comparisons with integral vectors. The x64 > implementation is enhanced to supported unsigned comparisons. > > Enhancements are made to the API for loading/storing a `byte` vector from/to > a `boolean[]` array. > > The testing of loads/stores can be expanded for scatter/gather, but before > doing that i think some refactoring of the tests is required to reposition > tests in the right classes. I would like to do that work after integration of > the PR. Paul Sandoz has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains five commits: - Merge remote-tracking branch 'upstream/master' into JDK-8266317-vector-api-enhancements - JavaDoc refs for unsigned operators. - Rename method. - Minor clarifications to the specification. - 8266317: Vector API enhancements - Changes: https://git.openjdk.java.net/jdk/pull/3803/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3803=03 Stats: 10016 lines in 121 files changed: 9084 ins; 190 del; 742 mod Patch: https://git.openjdk.java.net/jdk/pull/3803.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3803/head:pull/3803 PR: https://git.openjdk.java.net/jdk/pull/3803
Re: RFR: 8267569: java.io.File.equals contains misleading Javadoc [v3]
On Tue, 1 Jun 2021 17:43:42 GMT, Brian Burkhalter wrote: >> Modify the specification of `java.io.File.equals` to clarify that equality >> is based only on a comparison of abstract pathnames as opposed to the file >> system objects that the `File`s represent. > > Brian Burkhalter has updated the pull request incrementally with one > additional commit since the last revision: > > 8267569: Change @implNote to @apiNote Marked as reviewed by bchristi (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4232
Re: RFR: 8267569: java.io.File.equals contains misleading Javadoc [v3]
On Tue, 1 Jun 2021 17:43:42 GMT, Brian Burkhalter wrote: >> Modify the specification of `java.io.File.equals` to clarify that equality >> is based only on a comparison of abstract pathnames as opposed to the file >> system objects that the `File`s represent. > > Brian Burkhalter has updated the pull request incrementally with one > additional commit since the last revision: > > 8267569: Change @implNote to @apiNote Marked as reviewed by dfuchs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4232
Re: java.util.List missing from "Collections Framework Overview" javadoc
Hi, can anybody take a look at this? I would do it myself but I don't have Author status to add an issue to the JBS. Should be a trivial change. Thanks Raffaello On 2021-05-30 18:03, Raffaello Giulietti wrote: Hello, seems like java.util.List is missing from the list in the "Collection Interfaces" section in https://docs.oracle.com/en/java/javase/16/docs/api/java.base/java/util/doc-files/coll-overview.html Should be easy to fix and doesn't seem to require a CSR as the document is non-normative. Greetings Raffaello
Re: RFR: 8267569: java.io.File.equals contains misleading Javadoc [v3]
On Tue, 1 Jun 2021 17:43:42 GMT, Brian Burkhalter wrote: >> Modify the specification of `java.io.File.equals` to clarify that equality >> is based only on a comparison of abstract pathnames as opposed to the file >> system objects that the `File`s represent. > > Brian Burkhalter has updated the pull request incrementally with one > additional commit since the last revision: > > 8267569: Change @implNote to @apiNote Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4232
Re: RFR: 8265518: C1: Intrinsic support for Preconditions.checkIndex [v6]
On Tue, 25 May 2021 02:44:41 GMT, Yi Yang wrote: >> Looks like now the test fails in the pre-submit tests? > > Thank you @veresov! > > I'm glad to have more comments from hotspot-compiler group. > > Later, I'd like to integrate it if there are no more comments/objections. > > Thanks! > Yang @kelthuzadx Sorry about the delay. Could you please rebase this to the current master and I'll push it. Thanks! - PR: https://git.openjdk.java.net/jdk/pull/3615
Re: RFR: 8267569: java.io.File.equals contains misleading Javadoc [v3]
> Modify the specification of `java.io.File.equals` to clarify that equality is > based only on a comparison of abstract pathnames as opposed to the file > system objects that the `File`s represent. Brian Burkhalter has updated the pull request incrementally with one additional commit since the last revision: 8267569: Change @implNote to @apiNote - Changes: - all: https://git.openjdk.java.net/jdk/pull/4232/files - new: https://git.openjdk.java.net/jdk/pull/4232/files/acd072ab..1e37eacd Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4232=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4232=01-02 Stats: 4 lines in 1 file changed: 0 ins; 1 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/4232.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4232/head:pull/4232 PR: https://git.openjdk.java.net/jdk/pull/4232
Re: RFR: 8191441: (Process) add Readers and Writer access to java.lang.Process streams [v5]
> Methods are added to java.lang.Process to read and write characters and lines > from and to a spawned Process. > The Charset used to encode and decode characters to bytes can be specified or > use the > operating system native encoding as is available from the "native.encoding" > system property. Roger Riggs has updated the pull request incrementally with one additional commit since the last revision: Clarified use of native.encoding property and api notes about useing both writers and streams - Changes: - all: https://git.openjdk.java.net/jdk/pull/4134/files - new: https://git.openjdk.java.net/jdk/pull/4134/files/a5ed7b73..37bfa009 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4134=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4134=03-04 Stats: 74 lines in 1 file changed: 12 ins; 28 del; 34 mod Patch: https://git.openjdk.java.net/jdk/pull/4134.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4134/head:pull/4134 PR: https://git.openjdk.java.net/jdk/pull/4134
Re: RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator) [v27]
> This PR contains the API and implementation changes for JEP-412 [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/412 Maurizio Cimadamore has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 38 commits: - Merge branch 'master' into JEP-412 - Merge branch 'master' into JEP-412 - * Add missing `final` in some static fields * Downgrade native methods in ProgrammableUpcallHandler to package-private - Add sealed modifiers in morally sealed API interfaces - Merge branch 'master' into JEP-412 - Fix VaList test Remove unused code in Utils - Merge pull request #11 from JornVernee/JEP-412-MXCSR Add MXCSR save and restore to upcall stubs for non-windows platforms - Add MXCSR save and restore to upcall stubs for non-windows platforms - Merge branch 'master' into JEP-412 - Fix issue with bounded arena allocator - ... and 28 more: https://git.openjdk.java.net/jdk/compare/36dc268a...10767bc0 - Changes: https://git.openjdk.java.net/jdk/pull/3699/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3699=26 Stats: 14501 lines in 219 files changed: 8847 ins; 3642 del; 2012 mod Patch: https://git.openjdk.java.net/jdk/pull/3699.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3699/head:pull/3699 PR: https://git.openjdk.java.net/jdk/pull/3699
Re: RFR: 8266846: Add java.time.InstantSource [v5]
On Sun, 30 May 2021 00:49:56 GMT, Stephen Colebourne wrote: >> 8266846: Add java.time.InstantSource > > Stephen Colebourne has updated the pull request incrementally with one > additional commit since the last revision: > > 8266846: Add java.time.InstantSource Marked as reviewed by rriggs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4016
Re: RFR: 8266846: Add java.time.InstantSource [v5]
On Sun, 30 May 2021 00:49:56 GMT, Stephen Colebourne wrote: >> 8266846: Add java.time.InstantSource > > Stephen Colebourne has updated the pull request incrementally with one > additional commit since the last revision: > > 8266846: Add java.time.InstantSource Looks good to me. - Marked as reviewed by naoto (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/4016
Re: RFR: 8267969: Add vectorized implementation for VectorMask.eq()
On Mon, 31 May 2021 10:25:26 GMT, Xiaohong Gong wrote:
> Currently `"VectorMask.eq()" `is not vectorized:
>
> public VectorMask eq(VectorMask m) {
> // FIXME: Generate good code here.
> return bOp(m, (i, a, b) -> a == b);
> }
>
> This can be implemented by calling `"xor(m.not())"` directly.
>
> The performance improved about 1.4x ~ 1.9x for the following benchmark with
> different basic types:
>
> @Benchmark
> public Object eq() {
> boolean[] ma = fm.apply(size);
> boolean[] mb = fmb.apply(size);
> boolean[] mt = fmt.apply(size);
> VectorMask m = VectorMask.fromArray(SPECIES, mt, 0);
>
> for (int ic = 0; ic < INVOC_COUNT; ic++) {
> for (int i = 0; i < ma.length; i += SPECIES.length()) {
> var av = SPECIES.loadMask(ma, i);
> var bv = SPECIES.loadMask(mb, i);
>
> // accumulate results, so JIT can't eliminate relevant
> computations
> m = m.and(av.eq(bv));
> }
> }
>
> return m;
> }
Looks. Later we may want to consider pushing this down as an intrinsic, perhaps
reusing `VectorSupport.compare`.
-
Marked as reviewed by psandoz (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/4272
Re: RFR: 8266310: deadlock while loading the JNI code [v6]
On Thu, 27 May 2021 14:31:59 GMT, Aleksei Voitylov
wrote:
>> Please review this PR which fixes the deadlock in ClassLoader between the
>> two lock objects - a lock object associated with the class being loaded, and
>> the ClassLoader.loadedLibraryNames hash map, locked during the native
>> library load operation.
>>
>> Problem being fixed:
>>
>> The initial reproducer demonstrated a deadlock between the JarFile/ZipFile
>> and the hash map. That deadlock exists even when the ZipFile/JarFile lock is
>> removed because there's another lock object in the class loader, associated
>> with the name of the class being loaded. Such objects are stored in
>> ClassLoader.parallelLockMap. The deadlock occurs when JNI_OnLoad() loads
>> exactly the same class, whose signature is being verified in another thread.
>>
>> Proposed fix:
>>
>> The proposed patch suggests to get rid of locking loadedLibraryNames hash
>> map and synchronize on each entry name, as it's done with class names in see
>> ClassLoader.getClassLoadingLock(name) method.
>>
>> The patch introduces nativeLibraryLockMap which holds the lock objects for
>> each library name, and the getNativeLibraryLock() private method is used to
>> lazily initialize the corresponding lock object. nativeLibraryContext was
>> changed to ThreadLocal, so that in any concurrent thread it would have a
>> NativeLibrary object on top of the stack, that's being currently
>> loaded/unloaded in that thread. nativeLibraryLockMap accumulates the names
>> of all native libraries loaded - in line with class loading code, it is not
>> explicitly cleared.
>>
>> Testing: jtreg and jck testing with no regressions. A new regression test
>> was developed.
>
> Aleksei Voitylov has updated the pull request incrementally with one
> additional commit since the last revision:
>
> address review comments
The change looks good in general with a minor suggestion to rename
`NativeLibraryContext::get` to `NativeLibraryContext::current` which would be
clearer as it returns the current native library context (of the current
thread).
src/java.base/share/classes/jdk/internal/loader/NativeLibraries.java line 207:
> 205: *
> 206: * We use a static stack to hold the list of libraries we are
> 207: * loading, so that each thread maintains its own stack.
line 206: no longer a static stack. line 206-207 can be replaced with:
* Each thread maintains its own stack to hold the list of
libraries it is loading.
src/java.base/share/classes/jdk/internal/loader/NativeLibraries.java line 213:
> 211: * a different class loader, we raise UnsatisfiedLinkError.
> 212: */
> 213: for (NativeLibraryImpl lib : NativeLibraryContext.get()) {
I suggest to rename the `get` method of `NativeLibraryContext` to `current`
that returns the current thread's context.
test/jdk/java/lang/ClassLoader/loadLibraryDeadlock/LoadLibraryDeadlock.java
line 49:
> 47: InstantiationException |
> 48: IllegalAccessException ignore) {
> 49: System.out.println("Class Class1 not found.");
In general a test should let the exception to propagate. In this case, the
threads (both t1 and t2) can warp the exception and throw `RuntimeException` as
it's an error.
test/jdk/java/lang/ClassLoader/loadLibraryDeadlock/LoadLibraryDeadlock.java
line 60:
> 58: System.out.println("Signed jar loaded.");
> 59: } catch (ClassNotFoundException ignore) {
> 60: System.out.println("Class Class2 not found.");
Same as above
test/jdk/java/lang/ClassLoader/loadLibraryDeadlock/TestLoadLibraryDeadlock.java
line 28:
> 26: * @test
> 27: * @bug 8266310
> 28: * @summary deadlock while loading the JNI code
please update `@summary` with a description of the test (rather than the
synposis of the bug).
test/jdk/java/lang/ClassLoader/loadLibraryDeadlock/TestLoadLibraryDeadlock.java
line 69:
> 67:
> 68: private static OutputAnalyzer genKey() throws Throwable {
> 69: runCommandInTestClassPath("rm", "-f", KEYSTORE);
Can you use `jdk.test.lib.util.FileUtils` instead of exec `rm -f`.
test/jdk/java/lang/ClassLoader/loadLibraryDeadlock/TestLoadLibraryDeadlock.java
line 84:
> 82:
> 83: private static OutputAnalyzer createJar(String outputJar, String...
> classes) throws Throwable {
> 84: String jar = JDKToolFinder.getJDKTool("jar");
You can consider using `jdk.test.lib.util.JarUtils` to reduce the test
execution time so that it can create the jar without exec'ing another process.
test/jdk/java/lang/ClassLoader/loadLibraryDeadlock/TestLoadLibraryDeadlock.java
line 141:
> 139: try {
> 140: return future.get(1000, TimeUnit.MILLISECONDS);
> 141: } catch (Exception ignoreAll) {
if this is an unexpected error case, it should throw an exception.
Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal [v8]
On Tue, 1 Jun 2021 15:21:33 GMT, Weijun Wang wrote: >> Please review this implementation of [JEP >> 411](https://openjdk.java.net/jeps/411). >> >> The code change is divided into 3 commits. Please review them one by one. >> >> 1. >> https://github.com/openjdk/jdk/commit/576161d15423f58281e384174d28c9f9be7941a1 >> The essential change for this JEP, including the `@Deprecate` annotations >> and spec change. It also update the default value of the >> `java.security.manager` system property to "disallow", and necessary test >> change following this update. >> 2. >> https://github.com/openjdk/jdk/commit/26a54a835e9f84aa528740a7c5c35d07355a8a66 >> Manual changes to several files so that the next commit can be generated >> programatically. >> 3. >> https://github.com/openjdk/jdk/commit/eb6c566ff9207974a03a53335e0e697cffcf0950 >> Automatic changes to other source files to avoid javac warnings on >> deprecation for removal >> >> The 1st and 2nd commits should be reviewed carefully. The 3rd one is >> generated programmatically, see the comment below for more details. If you >> are only interested in a portion of the 3rd commit and would like to review >> it as a separate file, please comment here and I'll generate an individual >> webrev. >> >> Due to the size of this PR, no attempt is made to update copyright years for >> any file to minimize unnecessary merge conflict. >> >> Furthermore, since the default value of `java.security.manager` system >> property is now "disallow", most of the tests calling >> `System.setSecurityManager()` need to launched with >> `-Djava.security.manager=allow`. This is covered in a different PR at >> https://github.com/openjdk/jdk/pull/4071. >> >> Update: the deprecation annotations and javadoc tags, build, compiler, >> core-libs, hotspot, i18n, jmx, net, nio, security, and serviceability are >> reviewed. Rest are 2d, awt, beans, sound, and swing. > > Weijun Wang has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 11 commits: > > - merge from master > - rename setSecurityManagerDirect to implSetSecurityManager > - default behavior reverted to allow > - move one annotation to new method > - merge from master, two files removed, one needs merge > - keep only one systemProperty tag > - fixing awt/datatransfer/DataFlavor/DataFlavorRemoteTest.java > - feedback from Sean, Phil and Alan > - add supresswarnings annotations automatically > - manual change before automatic annotating > - ... and 1 more: > https://git.openjdk.java.net/jdk/compare/74b70a56...ea2c4b48 Marked as reviewed by joehw (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4073
Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal [v8]
On Tue, 1 Jun 2021 15:21:33 GMT, Weijun Wang wrote: >> Please review this implementation of [JEP >> 411](https://openjdk.java.net/jeps/411). >> >> The code change is divided into 3 commits. Please review them one by one. >> >> 1. >> https://github.com/openjdk/jdk/commit/576161d15423f58281e384174d28c9f9be7941a1 >> The essential change for this JEP, including the `@Deprecate` annotations >> and spec change. It also update the default value of the >> `java.security.manager` system property to "disallow", and necessary test >> change following this update. >> 2. >> https://github.com/openjdk/jdk/commit/26a54a835e9f84aa528740a7c5c35d07355a8a66 >> Manual changes to several files so that the next commit can be generated >> programatically. >> 3. >> https://github.com/openjdk/jdk/commit/eb6c566ff9207974a03a53335e0e697cffcf0950 >> Automatic changes to other source files to avoid javac warnings on >> deprecation for removal >> >> The 1st and 2nd commits should be reviewed carefully. The 3rd one is >> generated programmatically, see the comment below for more details. If you >> are only interested in a portion of the 3rd commit and would like to review >> it as a separate file, please comment here and I'll generate an individual >> webrev. >> >> Due to the size of this PR, no attempt is made to update copyright years for >> any file to minimize unnecessary merge conflict. >> >> Furthermore, since the default value of `java.security.manager` system >> property is now "disallow", most of the tests calling >> `System.setSecurityManager()` need to launched with >> `-Djava.security.manager=allow`. This is covered in a different PR at >> https://github.com/openjdk/jdk/pull/4071. >> >> Update: the deprecation annotations and javadoc tags, build, compiler, >> core-libs, hotspot, i18n, jmx, net, nio, security, and serviceability are >> reviewed. Rest are 2d, awt, beans, sound, and swing. > > Weijun Wang has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 11 commits: > > - merge from master > - rename setSecurityManagerDirect to implSetSecurityManager > - default behavior reverted to allow > - move one annotation to new method > - merge from master, two files removed, one needs merge > - keep only one systemProperty tag > - fixing awt/datatransfer/DataFlavor/DataFlavorRemoteTest.java > - feedback from Sean, Phil and Alan > - add supresswarnings annotations automatically > - manual change before automatic annotating > - ... and 1 more: > https://git.openjdk.java.net/jdk/compare/74b70a56...ea2c4b48 Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4073
Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal [v8]
> Please review this implementation of [JEP > 411](https://openjdk.java.net/jeps/411). > > The code change is divided into 3 commits. Please review them one by one. > > 1. > https://github.com/openjdk/jdk/commit/576161d15423f58281e384174d28c9f9be7941a1 > The essential change for this JEP, including the `@Deprecate` annotations > and spec change. It also update the default value of the > `java.security.manager` system property to "disallow", and necessary test > change following this update. > 2. > https://github.com/openjdk/jdk/commit/26a54a835e9f84aa528740a7c5c35d07355a8a66 > Manual changes to several files so that the next commit can be generated > programatically. > 3. > https://github.com/openjdk/jdk/commit/eb6c566ff9207974a03a53335e0e697cffcf0950 > Automatic changes to other source files to avoid javac warnings on > deprecation for removal > > The 1st and 2nd commits should be reviewed carefully. The 3rd one is > generated programmatically, see the comment below for more details. If you > are only interested in a portion of the 3rd commit and would like to review > it as a separate file, please comment here and I'll generate an individual > webrev. > > Due to the size of this PR, no attempt is made to update copyright years for > any file to minimize unnecessary merge conflict. > > Furthermore, since the default value of `java.security.manager` system > property is now "disallow", most of the tests calling > `System.setSecurityManager()` need to launched with > `-Djava.security.manager=allow`. This is covered in a different PR at > https://github.com/openjdk/jdk/pull/4071. > > Update: the deprecation annotations and javadoc tags, build, compiler, > core-libs, hotspot, i18n, jmx, net, nio, security, and serviceability are > reviewed. Rest are 2d, awt, beans, sound, and swing. Weijun Wang has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 11 commits: - merge from master - rename setSecurityManagerDirect to implSetSecurityManager - default behavior reverted to allow - move one annotation to new method - merge from master, two files removed, one needs merge - keep only one systemProperty tag - fixing awt/datatransfer/DataFlavor/DataFlavorRemoteTest.java - feedback from Sean, Phil and Alan - add supresswarnings annotations automatically - manual change before automatic annotating - ... and 1 more: https://git.openjdk.java.net/jdk/compare/74b70a56...ea2c4b48 - Changes: https://git.openjdk.java.net/jdk/pull/4073/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4073=07 Stats: 2132 lines in 826 files changed: 1997 ins; 20 del; 115 mod Patch: https://git.openjdk.java.net/jdk/pull/4073.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4073/head:pull/4073 PR: https://git.openjdk.java.net/jdk/pull/4073
Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal [v7]
> Please review this implementation of [JEP > 411](https://openjdk.java.net/jeps/411). > > The code change is divided into 3 commits. Please review them one by one. > > 1. > https://github.com/openjdk/jdk/commit/576161d15423f58281e384174d28c9f9be7941a1 > The essential change for this JEP, including the `@Deprecate` annotations > and spec change. It also update the default value of the > `java.security.manager` system property to "disallow", and necessary test > change following this update. > 2. > https://github.com/openjdk/jdk/commit/26a54a835e9f84aa528740a7c5c35d07355a8a66 > Manual changes to several files so that the next commit can be generated > programatically. > 3. > https://github.com/openjdk/jdk/commit/eb6c566ff9207974a03a53335e0e697cffcf0950 > Automatic changes to other source files to avoid javac warnings on > deprecation for removal > > The 1st and 2nd commits should be reviewed carefully. The 3rd one is > generated programmatically, see the comment below for more details. If you > are only interested in a portion of the 3rd commit and would like to review > it as a separate file, please comment here and I'll generate an individual > webrev. > > Due to the size of this PR, no attempt is made to update copyright years for > any file to minimize unnecessary merge conflict. > > Furthermore, since the default value of `java.security.manager` system > property is now "disallow", most of the tests calling > `System.setSecurityManager()` need to launched with > `-Djava.security.manager=allow`. This is covered in a different PR at > https://github.com/openjdk/jdk/pull/4071. > > Update: the deprecation annotations and javadoc tags, build, compiler, > core-libs, hotspot, i18n, jmx, net, nio, security, and serviceability are > reviewed. Rest are 2d, awt, beans, sound, and swing. Weijun Wang has updated the pull request incrementally with one additional commit since the last revision: rename setSecurityManagerDirect to implSetSecurityManager - Changes: - all: https://git.openjdk.java.net/jdk/pull/4073/files - new: https://git.openjdk.java.net/jdk/pull/4073/files/8fd09c39..926e4b9a Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4073=06 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4073=05-06 Stats: 5 lines in 1 file changed: 0 ins; 1 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/4073.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4073/head:pull/4073 PR: https://git.openjdk.java.net/jdk/pull/4073
Re: RFR: 8195129: System.load() fails to load from unicode paths [v2]
On Fri, 28 May 2021 05:47:11 GMT, David Holmes wrote: > The core-libs folks have the experience/expertise with these character > encoding issues so I will defer to them. Naoto has agreed to look at this. - PR: https://git.openjdk.java.net/jdk/pull/4169
Re: RFR: JDK-8267936: PreserveAllAnnotations option isn't tested
Since the discussion happened over the holiday weekend, I didn't get a chance to respond until now, but I think that this came to a good outcome. As Alan's archaeology discovered, this flag appears to be a leftover from the original implementation, and I could find no signs of its usage. We might consider deprecating it (though, we might also leave sleeping dogs alone), but its good to have a test for the current behavior. Allow me to make a few meta-comments about how we got here, though, before we wrap up. they are not visible via java.lang.reflect API. I assume that's just an omission. While omissions do sometimes happen, it is usually not the best first theory in a situation like this. More often than not, there is a good, but often non-obvious, explanation. More importantly, though, the PR-first approach is not how we like to approach such a change, especially when it involves the behaviors of such fundamental APIs such as reflection, because it jumps over the most important steps: - What problem are we trying to solve? - Is it really a problem that needs to be solved? - Is it really a problem that needs to be solved in the JDK? - What solutions are there, besides the obvious one? - What are the pros, cons, and tradeoffs of the various solutions? - Of the proposed solution, what future downsides and risks could we imagine? Going straight to the code of a specific solution is likely to be unsatisfying in both the short term (because its the wrong conversation) or the long term (because it might not be the right solution to the right problem.) Instead, starting with a discussion of the problem, and of potential solutions and tradeoffs, is likely to yield a better result on both counts. (As an example of the last one, suppose we committed this patch. One could easily imagine some library later saying "must be run with -Xx:PreserveAllAnnotations" because that's what the author had to do to make it work. But a behavior change like non-runtime annotations showing up unexpectedly in reflection could confuse existing code, which might not work as expected with the new behavior. Which might then call for "can we please make the PreserveAllAnnotations flag more selective (e.g., per-class/module/package)" or calls for new flags or ... yuck. We try to avoid today's solutions becoming tomorrow's problems. This is not an exact science, of course, but this is the sort of stewardship we strive for.) On 6/1/2021 5:39 AM, Jaroslav Tulach wrote: There doesn't seem to be much support for the complete changes in #4245. To get at least something useful from that endeavor I have extracted the test for existing behavior of `-XX:+PreserveAllAnnotations` and I am offering it in this pull request without any changes to the JVM behavior. - Commit messages: - Test long time existing behavior of -XX:+PreserveAllAnnotations Changes: https://git.openjdk.java.net/jdk/pull/4280/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4280=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8267936 Stats: 172 lines in 1 file changed: 172 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/4280.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4280/head:pull/4280 PR: https://git.openjdk.java.net/jdk/pull/4280
Re: RFR: 8191441: (Process) add Readers and Writer access to java.lang.Process streams [v4]
On Tue, 25 May 2021 16:53:28 GMT, Roger Riggs wrote:
>> Methods are added to java.lang.Process to read and write characters and
>> lines from and to a spawned Process.
>> The Charset used to encode and decode characters to bytes can be specified
>> or use the
>> operating system native encoding as is available from the "native.encoding"
>> system property.
>
> Roger Riggs has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Described charset mapping of malformed chars in outputWriter
> Repeated calls to inputReader, errorReader, and outputWriter now return
> the same instance
> and check for inconsistent charset argument
> Added warnings about concurrently use of input/output streams and
> readers/writers.
src/java.base/share/classes/java/lang/Process.java line 105:
> 103: // Readers and Writers created for this process; so repeated calls
> return the same object
> 104: // All updates must be done while synchronized on this Process.
> 105: private BufferedWriter outputWriter = null;
No need to explicitly initialise all these fields to null.
src/java.base/share/classes/java/lang/Process.java line 131:
> 129: * Use {@link #getOutputStream} and {@link #outputWriter} with
> extreme care.
> 130: * Output to the {@code BufferedWriter} may be held in the buffer
> until
> 131: * {@linkplain BufferedWriter#flush flush} is called.
I think this will need a bit of wordsmithing to make it clear that its the
usage of both streams for the same Process requires great care (as it stands,
it reads like the usage of either method is dangerous).
src/java.base/share/classes/java/lang/Process.java line 207:
> 205: * {@link Charset} named by the {@code native.encoding}
> 206: * system property or the {@link Charset#defaultCharset()} if the
> 207: * {@code native.encoding} is not supported.
"if the native.encoding is not supported". I think this needs an adjustment to
make it clear that the value of the property is not a valid charset.
src/java.base/share/classes/java/lang/Process.java line 425:
> 423: } else {
> 424: if (!outputCharset.equals(charset))
> 425: throw new IllegalArgumentException("BufferedWriter
> was created with charset: " + outputCharset);
I'm not sure that IAE is the right exception here, I think it's closer to
IllegalStateException because the first usage of outputWriter has the side
effect of setting the charset for the Process's writer. Another option here is
to just put it into the "unpredictable" bucket that is using getOutputStream
and outputWriter at the same time. In that case, it could just return a new
BufferedWriter when it doesn't match the charset of the first usage.
-
PR: https://git.openjdk.java.net/jdk/pull/4134
Re: RFR: 8267569: java.io.File.equals contains misleading Javadoc [v2]
On Fri, 28 May 2021 23:42:39 GMT, Brian Burkhalter wrote:
>> Modify the specification of `java.io.File.equals` to clarify that equality
>> is based only on a comparison of abstract pathnames as opposed to the file
>> system objects that the `File`s represent.
>
> Brian Burkhalter has updated the pull request incrementally with one
> additional commit since the last revision:
>
> 8267569: Put clarifying verbiage in an @implNote
src/java.base/share/classes/java/io/File.java line 2228:
> 2226: * @implNote The abstract pathname equality test is equivalent to
> 2227: * {@code compareTo((File)obj) == 0}. This method does
> not
> 2228: * access the file system and the file is not required to
> exist.
I think it might be clear to replace this with an apiNote that makes it clear
to readers that this method just tests if the abstract paths are equal, it does
not access the file system.
src/java.base/share/classes/java/io/File.java line 2237:
> 2235: * @see #compareTo(File)
> 2236: * @see java.nio.file.Files#isSameFile(Path,Path)
> 2237: * @see java.nio.file.Path#equals(Object)
I think it could be confusing to link to Path.equals here, can this be dropped?
-
PR: https://git.openjdk.java.net/jdk/pull/4232
Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal [v6]
On Mon, 31 May 2021 15:02:57 GMT, Weijun Wang wrote: >> Please review this implementation of [JEP >> 411](https://openjdk.java.net/jeps/411). >> >> The code change is divided into 3 commits. Please review them one by one. >> >> 1. >> https://github.com/openjdk/jdk/commit/576161d15423f58281e384174d28c9f9be7941a1 >> The essential change for this JEP, including the `@Deprecate` annotations >> and spec change. It also update the default value of the >> `java.security.manager` system property to "disallow", and necessary test >> change following this update. >> 2. >> https://github.com/openjdk/jdk/commit/26a54a835e9f84aa528740a7c5c35d07355a8a66 >> Manual changes to several files so that the next commit can be generated >> programatically. >> 3. >> https://github.com/openjdk/jdk/commit/eb6c566ff9207974a03a53335e0e697cffcf0950 >> Automatic changes to other source files to avoid javac warnings on >> deprecation for removal >> >> The 1st and 2nd commits should be reviewed carefully. The 3rd one is >> generated programmatically, see the comment below for more details. If you >> are only interested in a portion of the 3rd commit and would like to review >> it as a separate file, please comment here and I'll generate an individual >> webrev. >> >> Due to the size of this PR, no attempt is made to update copyright years for >> any file to minimize unnecessary merge conflict. >> >> Furthermore, since the default value of `java.security.manager` system >> property is now "disallow", most of the tests calling >> `System.setSecurityManager()` need to launched with >> `-Djava.security.manager=allow`. This is covered in a different PR at >> https://github.com/openjdk/jdk/pull/4071. >> >> Update: the deprecation annotations and javadoc tags, build, compiler, >> core-libs, hotspot, i18n, jmx, net, nio, security, and serviceability are >> reviewed. Rest are 2d, awt, beans, sound, and swing. > > Weijun Wang has updated the pull request incrementally with one additional > commit since the last revision: > > default behavior reverted to allow System.setSecurityManagerDirect looks a bit ugly now. Can this be renamed to implSetSecurityManager and avoid the line break in the middle of the declaration? The usage of System.err usage in setSecurityManager also needs to be re-examined as this will run arbitrary code when System.err can be changed. To fix this will require capturing the stream at startup (as was done with the illegal access logger). It's okay to integrate with what you have for the first push and we can fix this issue with System.err when the warning message is changed to the intended message. - PR: https://git.openjdk.java.net/jdk/pull/4073
Integrated: 8267670: Update java.io, java.math, and java.text to use switch expressions
On Tue, 25 May 2021 09:37:58 GMT, Patrick Concannon wrote: > Hi, > > Could someone please review my code for updating the code in the `java.io`, > `java.math`, and `java.text` packages to make use of the switch expressions? > > Kind regards, > Patrick This pull request has now been integrated. Changeset: 4eb21682 Author:Patrick Concannon URL: https://git.openjdk.java.net/jdk/commit/4eb216824f39e3c3536972d76d778466c140df50 Stats: 354 lines in 11 files changed: 5 ins; 193 del; 156 mod 8267670: Update java.io, java.math, and java.text to use switch expressions Reviewed-by: darcy, chegar, naoto, iris, dfuchs, lancea, vtewari - PR: https://git.openjdk.java.net/jdk/pull/4182
RFR: JDK-8267936: PreserveAllAnnotations option isn't tested
There doesn't seem to be much support for the complete changes in #4245. To get at least something useful from that endeavor I have extracted the test for existing behavior of `-XX:+PreserveAllAnnotations` and I am offering it in this pull request without any changes to the JVM behavior. - Commit messages: - Test long time existing behavior of -XX:+PreserveAllAnnotations Changes: https://git.openjdk.java.net/jdk/pull/4280/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4280=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8267936 Stats: 172 lines in 1 file changed: 172 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/4280.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4280/head:pull/4280 PR: https://git.openjdk.java.net/jdk/pull/4280
Withdrawn: JDK-8267936: PreserveAllAnnotations option doesn't expose the annotation to Java code
On Fri, 28 May 2021 12:56:39 GMT, Jaroslav Tulach wrote: > This PR exposes runtime invisible annotations via `Class.getAnnotation` when > `-XX:+PreserveAllAnnotations` option is passed to the JVM. > > Existing `-XX:+PreserveAllAnnotations` option can be very useful for code > that needs to access annotations with `RetentionPolicy.CLASS` without the > need to parse the .class files manually. While the > RuntimeInvisibleAnnotations are kept in the runtime, they are not visible via > java.lang.reflect API. I assume that's just an omission. > > This PR provides a new test and a fix to make `Class.getAnnotation(...)` > useful when `-XX:+PreserveAllAnnotations` option is on. This pull request has been closed without being integrated. - PR: https://git.openjdk.java.net/jdk/pull/4245
Re: RFR: JDK-8267936: PreserveAllAnnotations option doesn't expose the annotation to Java code [v4]
On Mon, 31 May 2021 12:02:13 GMT, Peter Levart wrote: > A test could be constructed so that it would mimic the migration of an > annotation from CLASS to RUNTIME retention The test is ready for review in #4280 - I am closing this PR without integration as the change of core-libs proposed here hasn't gained enough support. - PR: https://git.openjdk.java.net/jdk/pull/4245
Re: RFR: 8267670: Update java.io, java.math, and java.text to use switch expressions [v10]
> Hi, > > Could someone please review my code for updating the code in the `java.io`, > `java.math`, and `java.text` packages to make use of the switch expressions? > > Kind regards, > Patrick Patrick Concannon 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 11 additional commits since the last revision: - Merge remote-tracking branch 'origin/master' into JDK-8267670 - Merge remote-tracking branch 'origin/master' into JDK-8267670 - Merge remote-tracking branch 'origin/master' into JDK-8267670 - Merge remote-tracking branch 'origin/master' into JDK-8267670 - 8267670: Added missing brace - Merge remote-tracking branch 'origin/master' into JDK-8267670 - 8267670: Removed trailing whitespace - 8267670: Remove redundant code from switch - 8267670: Updated code to use yield - Merge remote-tracking branch 'origin/master' into JDK-8267670 - ... and 1 more: https://git.openjdk.java.net/jdk/compare/4c495d3b...44f86889 - Changes: - all: https://git.openjdk.java.net/jdk/pull/4182/files - new: https://git.openjdk.java.net/jdk/pull/4182/files/44886603..44f86889 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4182=09 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4182=08-09 Stats: 10 lines in 3 files changed: 3 ins; 4 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/4182.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4182/head:pull/4182 PR: https://git.openjdk.java.net/jdk/pull/4182
Re: RFR: 8262891: Compiler implementation for Pattern Matching for switch (Preview) [v8]
> This is a preview of a patch implementing JEP 406: Pattern Matching for > switch (Preview): > https://bugs.openjdk.java.net/browse/JDK-8213076 > > The current draft of the specification is here: > http://cr.openjdk.java.net/~gbierman/jep406/jep406-20210430/specs/patterns-switch-jls.html > > A summary of notable parts of the patch: > -to support cases expressions and patterns in cases, there is a new common > superinterface for expressions and patterns, `CaseLabelTree`, which > expressions and patterns implement, and a list of case labels is returned > from `CaseTree.getLabels()`. > -to support `case default`, there is an implementation of `CaseLabelTree` > that represents it (`DefaultCaseLabelTree`). It is used also to represent the > conventional `default` internally and in the newly added methods. > -in the parser, parenthesized patterns and expressions need to be > disambiguated when parsing case labels. > -Lower has been enhanced to handle `case null` for ordinary (boxed-primitive, > String, enum) switches. This is a bit tricky for boxed primitives, as there > is no value that is not part of the input domain so that could be used to > represent `case null`. Requires a bit shuffling with values. > -TransPatterns has been enhanced to handle the pattern matching switch. It > produces code that delegates to a new bootstrap method, that will classify > the input value to the switch and return the case number, to which the switch > then jumps. To support guards, the switches (and the bootstrap method) are > restartable. The bootstrap method as such is written very simply so far, but > could be much more optimized later. > -nullable type patterns are `case String s, null`/`case null, String s`/`case > null: case String s:`/`case String s: case null:`, handling of these required > a few tricks in `Attr`, `Flow` and `TransPatterns`. > > The specdiff for the change is here (to be updated): > http://cr.openjdk.java.net/~jlahoda/8265981/specdiff.preview.01/overview-summary.html Jan Lahoda has updated the pull request incrementally with one additional commit since the last revision: Fixing enum switch with patterns with guards. - Changes: - all: https://git.openjdk.java.net/jdk/pull/3863/files - new: https://git.openjdk.java.net/jdk/pull/3863/files/a49b6109..80b1392b Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3863=07 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3863=06-07 Stats: 82 lines in 3 files changed: 80 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/3863.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3863/head:pull/3863 PR: https://git.openjdk.java.net/jdk/pull/3863
