Re: RFR: 8265518: C1: Intrinsic support for Preconditions.checkIndex [v2]
> 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: compiler and jdk Yi Yang has updated the pull request incrementally with one additional commit since the last revision: remove java_nio_Buffer in javaClasses.hpp - Changes: - all: https://git.openjdk.java.net/jdk/pull/3615/files - new: https://git.openjdk.java.net/jdk/pull/3615/files/a6affc54..7f30dc48 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3615=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3615=00-01 Stats: 27 lines in 2 files changed: 0 ins; 27 del; 0 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: 8260517: implement Sealed Classes as a standard feature in Java [v4]
> Please review this PR that intents to make sealed classes a final feature in > Java. This PR contains compiler and VM changes. In line with similar PRs, > which has made preview features final, this one is mostly removing preview > related comments from APIs plus options in test cases. Please also review the > related [CSR](https://bugs.openjdk.java.net/browse/JDK-8265090) > > Thanks > > note: this PR is related to > [PR-3528](https://github.com/openjdk/jdk/pull/3528) and must be integrated > after it. Vicente Romero 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 eight additional commits since the last revision: - Merge branch 'master' into JDK-8260517 - updating comment after review feedback - removing javax.lang.model changes - Merge branch 'master' into JDK-8260517 - Merge branch 'master' into JDK-8260517 - fixing failing regression tests - JVM related changes - 8260517: Compiler implementation for Sealed Classes - Changes: - all: https://git.openjdk.java.net/jdk/pull/3526/files - new: https://git.openjdk.java.net/jdk/pull/3526/files/8ebe56fd..43e9cb5f Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3526=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3526=02-03 Stats: 40790 lines in 1391 files changed: 8730 ins; 27464 del; 4596 mod Patch: https://git.openjdk.java.net/jdk/pull/3526.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3526/head:pull/3526 PR: https://git.openjdk.java.net/jdk/pull/3526
RFR: 8265248: Implementation Specific Properties: change prefix, plus add existing properties
Update module summary, add a few existing properties and features into the tables. - Commit messages: - 8265248: Implementation Specific Properties: change prefix, plus add existing properties Changes: https://git.openjdk.java.net/jdk/pull/3644/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3644=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8265248 Stats: 873 lines in 14 files changed: 640 ins; 66 del; 167 mod Patch: https://git.openjdk.java.net/jdk/pull/3644.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3644/head:pull/3644 PR: https://git.openjdk.java.net/jdk/pull/3644
RFR: 8265783: Create a separate library for x86 Intel SVML assembly intrinsics
Intel Short Vector Math Library (SVML) based intrinsics in native x86 assembly provide optimized implementation for Vector API transcendental and trigonometric methods. These methods are built into a separate library instead of being part of libjvm.so or jvm.dll. The following changes are made: The source for these methods is placed in the jdk.incubator.vector module under src/jdk.incubator.vector/linux/native/libsvml and src/jdk.incubator.vector/windows/native/libsvml. The assembly source files are named as “*.S” and include files are named as “*.S.inc”. The corresponding build script is placed at make/modules/jdk.incubator.vector/Lib.gmk. Changes are made to build system to support dependency tracking for assembly files with includes. The built native libraries (libsvml.so/svml.dll) are placed in bin directory of JDK on Windows and lib directory of JDK on Linux. The C2 JIT uses the dll_load and dll_lookup to get the addresses of optimized methods from this library. Build system changes and module library build scripts are contributed by Magnus (magnus.ihse.bur...@oracle.com). This work is part of second round of incubation of the Vector API. JEP: https://bugs.openjdk.java.net/browse/JDK-8261663 Please review. Performance: Micro benchmark Base Optimized Unit Gain(Optimized/Base) Double128Vector.ACOS 45.91 87.34 ops/ms 1.90 Double128Vector.ASIN 45.06 92.36 ops/ms 2.05 Double128Vector.ATAN 19.92 118.36 ops/ms 5.94 Double128Vector.ATAN2 15.24 88.17 ops/ms 5.79 Double128Vector.CBRT 45.77 208.36 ops/ms 4.55 Double128Vector.COS 49.94 245.89 ops/ms 4.92 Double128Vector.COSH 26.91 126.00 ops/ms 4.68 Double128Vector.EXP 71.64 379.65 ops/ms 5.30 Double128Vector.EXPM1 35.95 150.37 ops/ms 4.18 Double128Vector.HYPOT 50.67 174.10 ops/ms 3.44 Double128Vector.LOG 61.95 279.84 ops/ms 4.52 Double128Vector.LOG10 59.34 239.05 ops/ms 4.03 Double128Vector.LOG1P 18.56 200.32 ops/ms 10.79 Double128Vector.SIN 49.36 240.79 ops/ms 4.88 Double128Vector.SINH 26.59 103.75 ops/ms 3.90 Double128Vector.TAN 41.05 152.39 ops/ms 3.71 Double128Vector.TANH 45.29 169.53 ops/ms 3.74 Double256Vector.ACOS 54.21 106.39 ops/ms 1.96 Double256Vector.ASIN 53.60 107.99 ops/ms 2.01 Double256Vector.ATAN 21.53 189.11 ops/ms 8.78 Double256Vector.ATAN2 16.67 140.76 ops/ms 8.44 Double256Vector.CBRT 56.45 397.13 ops/ms 7.04 Double256Vector.COS 58.26 389.77 ops/ms 6.69 Double256Vector.COSH 29.44 151.11 ops/ms 5.13 Double256Vector.EXP 86.67 564.68 ops/ms 6.52 Double256Vector.EXPM1 41.96 201.28 ops/ms 4.80 Double256Vector.HYPOT 66.18 305.74 ops/ms 4.62 Double256Vector.LOG 71.52 394.90 ops/ms 5.52 Double256Vector.LOG10 65.43 362.32 ops/ms 5.54 Double256Vector.LOG1P 19.99 300.88 ops/ms 15.05 Double256Vector.SIN 57.06 380.98 ops/ms 6.68 Double256Vector.SINH 29.40 117.37 ops/ms 3.99 Double256Vector.TAN 44.90 279.90 ops/ms 6.23 Double256Vector.TANH 54.08 274.71 ops/ms 5.08 Double512Vector.ACOS 55.65 687.54 ops/ms 12.35 Double512Vector.ASIN 57.31 777.72 ops/ms 13.57 Double512Vector.ATAN 21.42 729.21 ops/ms 34.04 Double512Vector.ATAN2 16.37 414.33 ops/ms 25.32 Double512Vector.CBRT 56.78 834.38 ops/ms 14.69 Double512Vector.COS 59.88 837.04 ops/ms 13.98 Double512Vector.COSH 30.34 172.76 ops/ms 5.70 Double512Vector.EXP 99.66 1608.12 ops/ms 16.14 Double512Vector.EXPM1 43.39 318.61 ops/ms 7.34 Double512Vector.HYPOT 73.87 1502.72 ops/ms 20.34 Double512Vector.LOG 74.84 996.00 ops/ms 13.31 Double512Vector.LOG10 71.12 1046.52 ops/ms 14.72 Double512Vector.LOG1P 19.75 776.87 ops/ms 39.34 Double512Vector.POW 37.42 384.13 ops/ms 10.26 Double512Vector.SIN 59.74 728.45 ops/ms 12.19 Double512Vector.SINH 29.47 143.38 ops/ms 4.87 Double512Vector.TAN 46.20 587.21 ops/ms 12.71 Double512Vector.TANH 57.36 495.42 ops/ms 8.64 Double64Vector.ACOS 24.04 73.67 ops/ms 3.06 Double64Vector.ASIN 23.78 75.11 ops/ms 3.16 Double64Vector.ATAN 14.14 62.81 ops/ms 4.44 Double64Vector.ATAN2 10.38 44.43 ops/ms 4.28 Double64Vector.CBRT 16.47 107.50 ops/ms 6.53 Double64Vector.COS 23.42 152.01 ops/ms 6.49 Double64Vector.COSH 17.34 113.34 ops/ms 6.54 Double64Vector.EXP 27.08 203.53 ops/ms 7.52 Double64Vector.EXPM1 18.77 96.73 ops/ms 5.15 Double64Vector.HYPOT 18.54 103.62 ops/ms 5.59 Double64Vector.LOG 26.75 142.63 ops/ms 5.33 Double64Vector.LOG10 25.85 139.71 ops/ms 5.40 Double64Vector.LOG1P 13.26 97.94 ops/ms 7.38 Double64Vector.SIN 23.28 146.91 ops/ms 6.31 Double64Vector.SINH 17.62 88.59 ops/ms 5.03 Double64Vector.TAN 21.00 86.43 ops/ms 4.12 Double64Vector.TANH 23.75 111.35 ops/ms 4.69 Float128Vector.ACOS 57.52 110.65 ops/ms 1.92 Float128Vector.ASIN 57.15 117.95 ops/ms 2.06 Float128Vector.ATAN 22.52 318.74 ops/ms 14.15 Float128Vector.ATAN2 17.06 246.07 ops/ms 14.42 Float128Vector.CBRT 29.72 443.74 ops/ms 14.93 Float128Vector.COS 42.82 803.02 ops/ms 18.75 Float128Vector.COSH 31.44 118.34 ops/ms 3.76 Float128Vector.EXP 72.43 855.33 ops/ms 11.81 Float128Vector.EXPM1 37.82 127.85 ops/ms 3.38 Float128Vector.HYPOT 53.20 591.68 ops/ms 11.12 Float128Vector.LOG 52.95 877.94 ops/ms
Re: RFR: 8200559: Java agents doing instrumentation need a means to define auxiliary classes [v2]
On Fri, 16 Apr 2021 20:30:15 GMT, Rafael Winterhalter wrote: >> To allow agents the definition of auxiliary classes, an API is needed to >> allow this. Currently, this is often achieved by using `sun.misc.Unsafe` or >> `jdk.internal.misc.Unsafe` ever since the `defineClass` method was removed >> from `sun.misc.Unsafe`. > > Rafael Winterhalter has refreshed the contents of this pull request, and > previous commits have been removed. The incremental views will show > differences compared to the previous content of the PR. Setting '-javaagent' is mainly an operations problem. Many build tools do not allow to declare a test dependency this way as the life cycles are not laid out for it, the internal repository location might be machine dependent, for example, and it's difficult to point to a specific folder and file reliably. In this case, I'd say it would be easier to specify a parameter in the sense of '-Djdk.attach.allowAttachSelf=true' as it is a static value. This would however only work for build tools that initiate a new VM for running tests which is not overly attractive for simple builds due to the overhead. Of course, Maven, Gradle and similar tools could set this parameter by default for their own JVM, that part is likely overcomeable but it will certainly create confusion about how to run libraries that are omnipresent today and make the JVM ecosystem less approachable. What bothers me more is the tooling perspective for non-self-attached agents. For example, Aaeron offers a Java agent that adds plenty of debug logging to relevant lines of code. This affects method size and so forth, with Aaeron as a high-performance tool for banking and finance which is written very consciously with regards to the JIT, adding it directly was not feasible. Normally this logging is therefore thrown into a VM in retrospect, once an application starts failing and is already taken off the load balancer. For such a post-mortem, it would be rather annoying to realize that a JVM cannot be attached to with full capabilities if you forgot to set some flag. And often you did of course not consider the VM instance to fail, sometimes it takes months to get a JVM into this buggy state. This would be fairly inconvenient to face. Therefore, I really hope that the dynamic attach from 'outside' the VM will survive without imposing limits and that rather the self-attachment problem will be targeted as such. When I mention a 'jdk.test' module in the Mockito context, I am also rather hoping to improve performance compared to convenience. The problem with '-Djdk.attach.allowAttachSelf=true' is that you still need to start a new VM etc. Since Java 9, running single tests with Mockito has for example become much slower compared to Java 8. Despite the startup performance improvements in the JVM. If one could avoid the location-bound '-javaagent:...', but get the Instrumentation instance directly, I think this would render a real performance improvement in actual execution scenarios. Am Mi., 21. Apr. 2021 um 20:42 Uhr schrieb mlbridge[bot] < ***@***.***>: > *Mailing list message from Alan Bateman ***@***.***> on > core-libs-dev ***@***.***>:* > > On 20/04/2021 21:26, Rafael Winterhalter wrote: > > I have earlier proposed to offer a "jdk.test" module that > offers the Instrumentation instance via a simple API similar to Byte > Buddy's. The JVM would not load this module unless requested on the command > line. Build tools like Maven's surefire or Gradle's testrunner could then > standardize on loading this module as a convention to give access to this > test module by default such that libraries like Mockito could continue to > function out of the box without the libraries functioning on a standard VM > without extra configuration. As far as I know, mainly test libraries need > this API. This would also emphasise that Mockito and others are meant for > testing and fewer people would abuse it for production applications. People > would also have an explicit means of running a JVM for a production > application or for executing a test. > > Helping testing is good but a jdk.test module that hands out the > Instrumentation object could be problematic. Is there a reason why the > runner for Mockito and other mocking frameworks can't specify -javaagent > when launching? I would expect at least some mocking scenarios to > require load time transformations (to drop the final modifier from some > API elements for example) so important to have the transformer set > before classes are loaded. > > As for adding the API, my thought is that if the Instrumentation API were > to throw exceptions on some methods/arguments for dynamic agents in the > future, for example for retransformClasses(Object.class), this breaking > change would then simply extend to the proposed "defineClass" method. In > this sense, the Instrumentation API already assumes full power, I find it > not problematic to add the missing bit to this API even if it was >
Integrated: 8265793: Remove duplicate jtreg TEST.groups references for some client tests
On Thu, 22 Apr 2021 20:59:22 GMT, Phil Race wrote: > Delete some duplicates This pull request has now been integrated. Changeset: b84f6901 Author:Phil Race URL: https://git.openjdk.java.net/jdk/commit/b84f6901 Stats: 4 lines in 1 file changed: 0 ins; 4 del; 0 mod 8265793: Remove duplicate jtreg TEST.groups references for some client tests Reviewed-by: erikj - PR: https://git.openjdk.java.net/jdk/pull/3642
RFR: 8265793: Remove duplicate jtreg TEST.groups references for some client tests
Delete some duplicates - Commit messages: - 8265793: Remove duplicate jtreg TEST.groups references for some client tests Changes: https://git.openjdk.java.net/jdk/pull/3642/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3642=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8265793 Stats: 4 lines in 1 file changed: 0 ins; 4 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/3642.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3642/head:pull/3642 PR: https://git.openjdk.java.net/jdk/pull/3642
Re: RFR: 8265793: Remove duplicate jtreg TEST.groups references for some client tests
On Thu, 22 Apr 2021 20:59:22 GMT, Phil Race wrote: > Delete some duplicates Marked as reviewed by erikj (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3642
Re: RFR: 8263668: Update java.time to use instanceof pattern variable [v7]
On Thu, 22 Apr 2021 10:13:32 GMT, Patrick Concannon wrote: >> I was able to find (with IntelliJ IDEA help) few more places to improve >> >> java.time 27 warnings >> class Clock 2 warnings >> class FixedClock 1 warning >> method equals(Object) 1 warning >> WARNING Variable 'other' can be replaced with pattern >> variable >> class OffsetClock 1 warning >> method equals(Object) 1 warning >> WARNING Variable 'other' can be replaced with pattern >> variable >> class Instant 2 warnings >> method until(Temporal, TemporalUnit) 1 warning >> WARNING Variable 'f' can be replaced with pattern variable >> method with(TemporalField, long) 1 warning >> WARNING Variable 'f' can be replaced with pattern variable >> class LocalDate 5 warnings >> method minus(TemporalAmount) 1 warning >> WARNING Variable 'periodToSubtract' can be replaced with >> pattern variable >> method plus(long, TemporalUnit) 1 warning >> WARNING Variable 'f' can be replaced with pattern variable >> method plus(TemporalAmount) 1 warning >> WARNING Variable 'periodToAdd' can be replaced with pattern >> variable >> method range(TemporalField) 1 warning >> WARNING Variable 'f' can be replaced with pattern variable >> method with(TemporalField, long) 1 warning >> WARNING Variable 'f' can be replaced with pattern variable >> class LocalDateTime 8 warnings >> method get(TemporalField) 1 warning >> WARNING Variable 'f' can be replaced with pattern variable >> method getLong(TemporalField) 1 warning >> WARNING Variable 'f' can be replaced with pattern variable >> method isSupported(TemporalField) 1 warning >> WARNING Variable 'f' can be replaced with pattern variable >> method minus(TemporalAmount) 1 warning >> WARNING Variable 'periodToSubtract' can be replaced with >> pattern variable >> method plus(long, TemporalUnit) 1 warning >> WARNING Variable 'f' can be replaced with pattern variable >> method plus(TemporalAmount) 1 warning >> WARNING Variable 'periodToAdd' can be replaced with pattern >> variable >> method range(TemporalField) 1 warning >> WARNING Variable 'f' can be replaced with pattern variable >> method with(TemporalField, long) 1 warning >> WARNING Variable 'f' can be replaced with pattern variable >> class LocalTime 1 warning >> method with(TemporalField, long) 1 warning >> WARNING Variable 'f' can be replaced with pattern variable >> class OffsetDateTime 1 warning >> method with(TemporalField, long) 1 warning >> WARNING Variable 'f' can be replaced with pattern variable >> class Year 1 warning >> method with(TemporalField, long) 1 warning >> WARNING Variable 'f' can be replaced with pattern variable >> class YearMonth 1 warning >> method with(TemporalField, long) 1 warning >> WARNING Variable 'f' can be replaced with pattern variable >> class ZonedDateTime 6 warnings >> method equals(Object) 1 warning >> WARNING Variable 'other' can be replaced with pattern >> variable >> method minus(TemporalAmount) 1 warning >> WARNING Variable 'periodToSubtract' can be replaced with >> pattern variable >> method plus(TemporalAmount) 1 warning >> WARNING Variable 'periodToAdd' can be replaced with pattern >> variable >> method with(TemporalAdjuster) 2 warnings >> WARNING Variable 'odt' can be replaced with pattern variable >> WARNING Variable 'instant' can be replaced with pattern >> variable >> method with(TemporalField, long) 1 warning >> WARNING Variable 'f' can be replaced with pattern variable >> java.time.chrono 13 warnings >> class ChronoLocalDateImpl 1 warning >> method plus(long, TemporalUnit) 1 warning >> WARNING Variable 'f' can be replaced with pattern variable >> class ChronoLocalDateTimeImpl 6 warnings >> method get(TemporalField) 1 warning >> WARNING Variable 'f' can be replaced with pattern variable >> method getLong(TemporalField) 1 warning >> WARNING Variable 'f' can be replaced with pattern variable >> method
Re: RFR: 8264208: Console charset API [v12]
On Thu, 22 Apr 2021 17:38:43 GMT, Naoto Sato wrote: >> Please review the changes for the subject issue. This has been suggested in >> a recent discussion thread for the JEP 400 >> [[1](https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-March/075214.html)]. >> A CSR has also been drafted, and comments are welcome >> [[2](https://bugs.openjdk.java.net/browse/JDK-8264209)]. > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Revived charset() in JavaIOAccess interface. Marked as reviewed by rriggs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3419
Re: RFR: 8264208: Console charset API [v11]
On Thu, 22 Apr 2021 16:29:35 GMT, Roger Riggs wrote: >> Then `charset()` in the shared secret would return `null`. Would that >> suffice your case? > > I read lines 575-587 as initializing CHARSET regardless of whether the > Console was created. OK, revived the charset() method. - PR: https://git.openjdk.java.net/jdk/pull/3419
Re: RFR: 8264208: Console charset API [v12]
> Please review the changes for the subject issue. This has been suggested in > a recent discussion thread for the JEP 400 > [[1](https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-March/075214.html)]. > A CSR has also been drafted, and comments are welcome > [[2](https://bugs.openjdk.java.net/browse/JDK-8264209)]. Naoto Sato has updated the pull request incrementally with one additional commit since the last revision: Revived charset() in JavaIOAccess interface. - Changes: - all: https://git.openjdk.java.net/jdk/pull/3419/files - new: https://git.openjdk.java.net/jdk/pull/3419/files/e585d16f..2d90f8df Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3419=11 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3419=10-11 Stats: 5 lines in 2 files changed: 5 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/3419.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3419/head:pull/3419 PR: https://git.openjdk.java.net/jdk/pull/3419
Re: RFC: jdeps output - "jdk8internals" vs "JDK removed internal API" after JDK-8213909
Mandy, thanks for the confirmation we'll not have follow-up jdkXXinternals :) Let me know if you have a spare minute to fix it some time soon, otherwise I'll be happy to assist. -Aleksei On 22/04/2021 20:00, Mandy Chung wrote: > Hi Aleksei, > > Looks like is a bug. "JDK removed internal API" is the intended > description as it indicates that a non-existent API is referenced. > > I created https://bugs.openjdk.java.net/browse/JDK-8265773 to fix this. > > thanks > Mandy > > On 4/22/21 4:21 AM, Aleksei Voitylov wrote: >> Hi, >> >> JDK-8213909 [1] (since JDK 12, backported into 11.0.11), aside from >> bringing great additions to transitive module analysis, introduced a >> slight change in output of jdeps: >> >> jdk-11.0.11/bin/jdeps -s reactor-core-3.4.5.jar >> reactor-core-3.4.5.jar -> jdk8internals >> reactor-core-3.4.5.jar -> java.base >> reactor-core-3.4.5.jar -> java.logging >> reactor-core-3.4.5.jar -> not found >> >> jdk-11.0.10/bin/jdeps -s reactor-core-3.4.5.jar >> reactor-core-3.4.5.jar -> JDK removed internal API >> reactor-core-3.4.5.jar -> java.base >> reactor-core-3.4.5.jar -> java.logging >> reactor-core-3.4.5.jar -> not found >> >> Note the difference "jdk8internals" vs "JDK removed internal API", not >> discussed in the relevant CSR [2] or the review thread [3]. This change >> is likely to affect some automation systems built around jdeps. Since >> the change has already landed, I wanted to check if there is a reason >> behind this naming change, and if "jdk8internals" should be the naming >> convention the users are expected to standardize their automation on >> going forward? >> >> I would assume most now are used to parsing for "JDK removed internal >> API" to determine if the project uses any removed APIs (and should >> probably be using --print-module-deps to feed jlink input), but I also >> see merit in having a single convention for mainline and CPU releases. >> >> -Aleksei >> >> [1] https://bugs.openjdk.java.net/browse/JDK-8213909 >> [2] https://bugs.openjdk.java.net/browse/JDK-8213915 >> [3] >> http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-November/056660.html >> >> >> >
Re: RFC: jdeps output - "jdk8internals" vs "JDK removed internal API" after JDK-8213909
Hi Aleksei, Looks like is a bug. "JDK removed internal API" is the intended description as it indicates that a non-existent API is referenced. I created https://bugs.openjdk.java.net/browse/JDK-8265773 to fix this. thanks Mandy On 4/22/21 4:21 AM, Aleksei Voitylov wrote: Hi, JDK-8213909 [1] (since JDK 12, backported into 11.0.11), aside from bringing great additions to transitive module analysis, introduced a slight change in output of jdeps: jdk-11.0.11/bin/jdeps -s reactor-core-3.4.5.jar reactor-core-3.4.5.jar -> jdk8internals reactor-core-3.4.5.jar -> java.base reactor-core-3.4.5.jar -> java.logging reactor-core-3.4.5.jar -> not found jdk-11.0.10/bin/jdeps -s reactor-core-3.4.5.jar reactor-core-3.4.5.jar -> JDK removed internal API reactor-core-3.4.5.jar -> java.base reactor-core-3.4.5.jar -> java.logging reactor-core-3.4.5.jar -> not found Note the difference "jdk8internals" vs "JDK removed internal API", not discussed in the relevant CSR [2] or the review thread [3]. This change is likely to affect some automation systems built around jdeps. Since the change has already landed, I wanted to check if there is a reason behind this naming change, and if "jdk8internals" should be the naming convention the users are expected to standardize their automation on going forward? I would assume most now are used to parsing for "JDK removed internal API" to determine if the project uses any removed APIs (and should probably be using --print-module-deps to feed jlink input), but I also see merit in having a single convention for mainline and CPU releases. -Aleksei [1] https://bugs.openjdk.java.net/browse/JDK-8213909 [2] https://bugs.openjdk.java.net/browse/JDK-8213915 [3] http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-November/056660.html
Re: RFR: 8264208: Console charset API [v11]
On Thu, 22 Apr 2021 16:11:16 GMT, Naoto Sato wrote: >> Not always, for example, if stderr was redirected to a terminal but not >> stdin and stdout. >> The istty check is only true if both stdin and stdout are ttys. > > Then `charset()` in the shared secret would return `null`. Would that suffice > your case? I read lines 575-587 as initializing CHARSET regardless of whether the Console was created. - PR: https://git.openjdk.java.net/jdk/pull/3419
Integrated: 8265591: Remove vestiages of intermediate JSR 175 annotation format
On Tue, 20 Apr 2021 23:37:15 GMT, Joe Darcy wrote: > During the recent review of JDK-8228988, I noticed again the comments in the > annotation parser about support for the pre-GA annotation format used before > JDK 5.0 shipped. During the development of annotations, there was a late > change to correct a flaw in the annotation encoding, JDK-5020908. > > I don't think it is necessary to carry forward support for this transient > format any longer and this changeset removes support from both core > reflection and javac. > > Clean runs of relevant test; I gauge this fix as no-reg hard. This pull request has now been integrated. Changeset: 8758b554 Author:Joe Darcy URL: https://git.openjdk.java.net/jdk/commit/8758b554 Stats: 24 lines in 2 files changed: 0 ins; 19 del; 5 mod 8265591: Remove vestiages of intermediate JSR 175 annotation format Reviewed-by: jfranck - PR: https://git.openjdk.java.net/jdk/pull/3597
Re: RFR: 8265137: java.util.Random suddenly has new public methods nowhere documented [v6]
On Wed, 21 Apr 2021 13:13:16 GMT, Jim Laskey wrote:
>> Move makeXXXSpilterator from public (@hidden) to protected. No API ch
>
> Jim Laskey 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 eight additional commits since
> the last revision:
>
> - Merge branch 'master' into 8265137
> - Remove @hidden
> - Correct the hierarchy of Random
> - Remove extraneous references to makeXXXSpliterator
> - Move makeXXXSpliterator methods to RandomSupport
> - change static final from 'proxy' to 'PROXY'
> - Make makeXXXSpliterator final
> - Move makeXXXSpilterator from public (@hidden) to protected. No API change.
Overall looks good.
AbstractSpliteratorGenerator now is a bit weird because it's a utility class
that has static methods, _and_ it's an abstract superclass that has instance
methods that mostly just call the static methods. Seems to me that additional
cleanup is possible. It might not be worth it right now, since the main point
of this PR -- to remove "leakage" of these helper methods into the public API
-- has been achieved.
The RandomXXXSpliterator nested classes could also stand some scrutiny. The
constructors consume a RandomGenerator, which is stored in an instance
variable. Various comments still refer to this as an
AbstractSpliteratorGenerator. (See line 961 for example; can't comment directly
because it's not part of this commit.)
But it's not clear to me that the RandomXXXSpliterator classes really need a
full RandomGenerator. For example, as far as I can see, RandomIntSpliterator
_only_ needs `nextInt` to generate int values; therefore it could be
`IntSupplier`. Similar for long and double. (I'm applying the Interface
Segregation Principle here.) But again this is probably a cleanup for another
day.
src/java.base/share/classes/jdk/internal/util/random/RandomSupport.java line
1433:
> 1431: private AbstractSpliteratorGenerator() {
> 1432: }
> 1433:
This comment is wrong now since there are instances of this class (really,
subclasses). The constructor should probably simply be removed.
-
Marked as reviewed by smarks (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/3469
Re: RFR: 8264208: Console charset API [v11]
On Thu, 22 Apr 2021 16:03:12 GMT, Roger Riggs wrote: >> Would the singleton `Console.cons` be instantiated in your use case? It is >> created only when isatty() (or Windows' equivalent) in the native code >> returns true. > > Not always, for example, if stderr was redirected to a terminal but not stdin > and stdout. > The istty check is only true if both stdin and stdout are ttys. Then `charset()` in the shared secret would return `null`. Would that suffice your case? - PR: https://git.openjdk.java.net/jdk/pull/3419
Re: RFR: 8203359: Container level resources events [v9]
On Wed, 21 Apr 2021 22:38:32 GMT, Erik Gahlin wrote:
>> Jaroslav Bachorik has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Fix event metadata
>
> I wonder if something similar to below could be added to
> jdk.jfr.internal.Utils:
>
> private static Metrics[] metrics;
> public static Metrics getMetrics() {
> if (metrics == null) {
> metrics = new Metrics[] { Metrics.systemMetrics() };
> }
> return metrics[0];
> }
>
> public static boolean shouldSkipBytecode(String eventName, Class
> superClass) {
> if (superClass != AbstractJDKEvent.class) {
> return false;
> }
> if (!eventName.startsWith("jdk.Container")) {
> return false;
> }
> return getMetrics() == null;
> }
>
> Then we could add checks to
> jdk.jfr.internal.JVMUpcalls::bytesForEagerInstrumentation(...)
>
> eventName = ei.getEventName();
> if (Utils.shouldSkipBytecode(eventName, superClass))) {
> return oldBytes;
> }
>
> and jdk.jfr.internal.JVMUpcalls:onRetransform(...)
>
> if (jdk.internal.event.Event.class.isAssignableFrom(clazz) &&
> !Modifier.isAbstract(clazz.getModifiers())) {
> if (Utils.shouldSkipBytecode(clazz.getName(), clazz.getSuperclass()))
> {
> return oldBytes;
> }
>
> This way we would not pay for generating bytecode for events in a
> non-container environment.
>
> Not sure if it works, but could perhaps make startup faster? We would still
> pay for generating the event handlers during registration, but it's much
> trickier to avoid since we need to store the event type somewhere.
@egahlin Sounds good.
Any particular reason you are using `Metrics[]` array?
-
PR: https://git.openjdk.java.net/jdk/pull/3126
Re: RFR: 8264208: Console charset API [v11]
On Thu, 22 Apr 2021 15:42:02 GMT, Naoto Sato wrote: >> src/java.base/share/classes/java/io/Console.java line 597: >> >>> 595: return null; >>> 596: } >>> 597: }); >> >> Please keep the charset() method and return CHARSET. >> >> I'm looking at a use case that needs to know the platform charset regardless >> of whether the console exists. >> When a process is launched it may be redirected to /dev/tty or a pseudo tty >> and in that case >> a Reader from that stream should be able to use the encoding of the platform. >> Its still a work in progress, but it would save some refactoring or >> duplication later. > > Would the singleton `Console.cons` be instantiated in your use case? It is > created only when isatty() (or Windows' equivalent) in the native code > returns true. Not always, for example, if stderr was redirected to a terminal but not stdin and stdout. The istty check is only true if both stdin and stdout are ttys. - PR: https://git.openjdk.java.net/jdk/pull/3419
Re: RFR: 8200559: Java agents doing instrumentation need a means to define auxiliary classes [v2]
On Fri, 16 Apr 2021 20:30:15 GMT, Rafael Winterhalter wrote: >> To allow agents the definition of auxiliary classes, an API is needed to >> allow this. Currently, this is often achieved by using `sun.misc.Unsafe` or >> `jdk.internal.misc.Unsafe` ever since the `defineClass` method was removed >> from `sun.misc.Unsafe`. > > Rafael Winterhalter has refreshed the contents of this pull request, and > previous commits have been removed. The incremental views will show > differences compared to the previous content of the PR. Sorry to be late to this party. I've been wanting to read this thread for a while but have bene too busy up to now. I have just a few comments I too was party to the discussions about agent capabilities and recall well the decision to gradually impose restrictions, the first one being to control dynamic agent loading. I was happy to accept that general decision and the specific one to limit the opportunity for an agent self-hoisting into the current JVM. However, a key part of the plan to move forward on restrictions was to provide an override switch. I'd very much like to see that retained as an option. I know that in some uses self-hoisting is much preferable to having to install the agent on the command line and I'd expect he same to be true for any other capabilities for which restrictions were adopted. Although it is true -- as Ron said - -that configuring -javaagent on the command line is always /possible/ there are actually many scenarios for agent use where deployment of an agent after startup is pragmatically much more desirable. An obvious use is trouble shooting, where you only want an agent in place when something goes wrong. That turns out to be critical to solving some seriously difficult support cases. The interesting use cases also fall under testing where self-hoisting of a test agent by a test framework can result in an enormous simplification of test configuration. Usage of Byteman for testing went through the roof with this capability in place. Never underestimate the degree to which even the most minimal configuration complexity puts off Enterprise java developers when it is multiplied by the test suite size of a large project. So, likewise with other restrictions on behaviour, I'm very happy to see them put in place for dynamically hoisted agents so long as there is still a command line override along the lines of the agent attach property that allows a dynamic agent to do all that a command line agent can. One other thing I'd like to correct is a point made in the discussion about agent code residing in the system loader. It is true that the main agent class gets loaded by the system loader. However, it is perfectly possible to ensure that all the rest of the agent code is loaded by the bootstrap loader. A Main class can add the agent jar to the bootstrap path and then load and use reflection to invoke an effective main routine on a bootstrap loaded SubMain class. Byteman uses this trick on request in order to allow it to instrument bootstrap classes. Because all the Byteman classes except for the original Main shell class are loaded by the bootstrap loader Byteman can safely inject references to the Byteman rule engine and Byteman exception classes into bootstrap code. - PR: https://git.openjdk.java.net/jdk/pull/3546
Re: RFR: 8203359: Container level resources events [v10]
> With this change it becomes possible to surface various cgroup level metrics > (available via `jdk.internal.platform.Metrics`) as JFR events. > > Only a subset of the metrics exposed by `jdk.internal.platform.Metrics` is > turned into JFR events to start with. > * CPU related metrics > * Memory related metrics > * I/O related metrics > > For each of those subsystems a configuration data will be emitted as well. > The initial proposal is to emit the configuration data events at least once > per chunk and the metrics values at 30 seconds interval. > By using these values the emitted events seem to contain useful information > without increasing overhead (the metrics values are read from `/proc` > filesystem so that should not be done too frequently). Jaroslav Bachorik 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 13 additional commits since the last revision: - Prevent event container bytecode generation if no container present - Fix event metadata - Roll back conditional registration of container events - Remove container events flag - Remove trailing spaces - Doh - Report container type and register events conditionally - Remove unused test files - Initial test support for JFR container events - Update the JFR control files - ... and 3 more: https://git.openjdk.java.net/jdk/compare/7babcb00...04c3f092 - Changes: - all: https://git.openjdk.java.net/jdk/pull/3126/files - new: https://git.openjdk.java.net/jdk/pull/3126/files/f766faf0..04c3f092 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3126=09 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3126=08-09 Stats: 36024 lines in 1257 files changed: 5443 ins; 26264 del; 4317 mod Patch: https://git.openjdk.java.net/jdk/pull/3126.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3126/head:pull/3126 PR: https://git.openjdk.java.net/jdk/pull/3126
Re: RFR: 8264208: Console charset API [v11]
On Thu, 22 Apr 2021 15:18:11 GMT, Roger Riggs wrote: >> Naoto Sato 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 13 additional >> commits since the last revision: >> >> - Refined the test case. >> - Merge branch 'master' into JDK-8264208 >> - Changed shell based test into java based >> - Added link to Charset#defaultChaset() in InputStreamReader. >> - Modified javadocs per suggestions. >> - Added @see links. >> - Added Console::charset() relation with System.in >> - Added comment to System.out/err init. >> - Reflected further review comments. >> - Reverted PrintStream changes >> - ... and 3 more: >> https://git.openjdk.java.net/jdk/compare/51627555...e585d16f > > src/java.base/share/classes/java/io/Console.java line 597: > >> 595: return null; >> 596: } >> 597: }); > > Please keep the charset() method and return CHARSET. > > I'm looking at a use case that needs to know the platform charset regardless > of whether the console exists. > When a process is launched it may be redirected to /dev/tty or a pseudo tty > and in that case > a Reader from that stream should be able to use the encoding of the platform. > Its still a work in progress, but it would save some refactoring or > duplication later. Would the singleton `Console.cons` be instantiated in your use case? It is created only when isatty() (or Windows' equivalent) in the native code returns true. - PR: https://git.openjdk.java.net/jdk/pull/3419
Re: ReversibleCollection proposal
On Thu, 22 Apr 2021 at 13:58, Remi Forax wrote:
> I would like to preserve the invariant that, when calling a method on a
> Collection/iterator, an UnsupportedOperationException only occurs when trying
> to mutate that collection.
> If we are ok with that, this means that addFirst can not be a default method
> of Collection.
This implementation meets the invariant:
public interface Collection {
default void addFirst(E e) { add(e); }
default E getFirst() { return iterator().next(); }
default E removeFirst() {
var i = iterator(); i.next();
i.remove();
}
}
This is what I intended anyway, ie that its OK for "first" to work on
an unordered collection, just that addFirst() has very weak semantics
wrt first-ness.
"Ensures that this collection contains the specified element(optional
operation). This has the same basic behaviour as add(E), but
implementations may choose to add the element in first position if
they have some kind of guarantee of ordering. An exception should not
be thrown unless add(E) would also have thrown the exception."
Stephen
Re: RFR: 8264208: Console charset API [v11]
On Tue, 20 Apr 2021 22:35:00 GMT, Naoto Sato wrote: >> Please review the changes for the subject issue. This has been suggested in >> a recent discussion thread for the JEP 400 >> [[1](https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-March/075214.html)]. >> A CSR has also been drafted, and comments are welcome >> [[2](https://bugs.openjdk.java.net/browse/JDK-8264209)]. > > Naoto Sato 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 13 additional commits since > the last revision: > > - Refined the test case. > - Merge branch 'master' into JDK-8264208 > - Changed shell based test into java based > - Added link to Charset#defaultChaset() in InputStreamReader. > - Modified javadocs per suggestions. > - Added @see links. > - Added Console::charset() relation with System.in > - Added comment to System.out/err init. > - Reflected further review comments. > - Reverted PrintStream changes > - ... and 3 more: > https://git.openjdk.java.net/jdk/compare/72f17eb7...e585d16f src/java.base/share/classes/java/io/Console.java line 597: > 595: return null; > 596: } > 597: }); Please keep the charset() method and return CHARSET. I'm looking at a use case that needs to know the platform charset regardless of whether the console exists. When a process is launched it may be redirected to /dev/tty or a pseudo tty and in that case a Reader from that stream should be able to use the encoding of the platform. Its still a work in progress, but it would save some refactoring or duplication later. - PR: https://git.openjdk.java.net/jdk/pull/3419
Withdrawn: 8264161: BigDecimal#stripTrailingZeros can throw undocumented ArithmeticException
On Thu, 25 Mar 2021 08:19:24 GMT, Fabian Meumertzheim wrote: > Adds missing @throws declarations for ArithmeticException to the public > function > java.math.BigDecimal#stripTrailingZeros > as well as the private helper functions > java.math.BigDecimal#createAndStripZerosToMatchScale(long, int, long) > java.math.BigDecimal#createAndStripZerosToMatchScale(BigInteger, int, long) > > stripTrailingZeros calls one of the two helper functions, both of which > can repeatedly decrease the scale by 1 until it underflows. If it does, > the call to checkScale will result in an ArithmeticException (overflow). This pull request has been closed without being integrated. - PR: https://git.openjdk.java.net/jdk/pull/3189
Re: RFR: 8264161: BigDecimal#stripTrailingZeros can throw undocumented ArithmeticException
On Thu, 25 Mar 2021 08:19:24 GMT, Fabian Meumertzheim wrote: > Adds missing @throws declarations for ArithmeticException to the public > function > java.math.BigDecimal#stripTrailingZeros > as well as the private helper functions > java.math.BigDecimal#createAndStripZerosToMatchScale(long, int, long) > java.math.BigDecimal#createAndStripZerosToMatchScale(BigInteger, int, long) > > stripTrailingZeros calls one of the two helper functions, both of which > can repeatedly decrease the scale by 1 until it underflows. If it does, > the call to checkScale will result in an ArithmeticException (overflow). Thanks! - PR: https://git.openjdk.java.net/jdk/pull/3189
Re: Jpackage Mac entitlements
> On Apr 22, 2021, at 7:41 AM, Andy Herrick wrote: > > > On 4/21/2021 6:15 PM, Michael Hall wrote: >> Reverted to Jdk 16 and temporarily couldn’t figure out why AppleScript >> wasn’t working again. >> >> I needed to provide an entitlement. At jpackage 16 I found I needed to do >> this differently with a .entitlements file in the resources >> directory, as I noticed in the verbose command output. >> >> I had previously noticed that jpackage 17 now includes a parameter to >> provide this. >> >> Will both options continue to be supported, resource dir and parm, or will >> parm become the only supported way at or after 17? > > Yes, if --mac-entitlement options exists, it will use it, otherwise , the > previous behavior is maintained, the default resource entitlements.plist in > the source code will be used unless overridden by .entitlements in > the resource directory. > > /Andy > Ok so in order… —mac-entitlement .entitlements jpackage provided default If at some point after the —mac-entitlement parameter has been in place for a while and it is decided to eliminate the resource dir option a warning message might be an idea. I still think possibly including a verify error exception or warning might also be helpful if that has issues I figured out that when I was trying different things with signing I must of deleted my Developer ID Application: certificate from keychain. Which is what must of been being found by the jpackage /usr/bin/security find-certificate. Hard coding my fully qualified “3rd Party” cert was working fine. After putting my Developer ID Application cert back in the keychain signing again worked by just using my name - but once more the app doesn’t verify. This time if I codesign -v it seems ok. But for some reason the Taccy application shows… App signature check: ⛔️ spctl error 3 /Users/mjh/HalfPipe/HalfPipe_jpkg/outputdir/HalfPipe.app: rejected source=Unnotarized Developer ID origin=Developer ID Application: For what I am doing this is not a problem but could be a concern for someone trying to get an application into the App Store. So, if you do not currently have plans to do something or there is already a RFE in place that you know of. I will submit one to include a verify on signed applications.
Integrated: 8264514: HexFormat implementation tweaks
On Wed, 7 Apr 2021 18:49:36 GMT, Raffaello Giulietti wrote: > (Changed to new branch in personal fork) > > Please review these small tweaks. > For background information see > [1](https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-March/075822.html) > > Greetings > Raffaello This pull request has now been integrated. Changeset: fa82d475 Author:Raffaello Giulietti Committer: Roger Riggs URL: https://git.openjdk.java.net/jdk/commit/fa82d475 Stats: 67 lines in 1 file changed: 10 ins; 26 del; 31 mod 8264514: HexFormat implementation tweaks Reviewed-by: rriggs - PR: https://git.openjdk.java.net/jdk/pull/3381
Re: RFR: 8265137: java.util.Random suddenly has new public methods nowhere documented [v6]
On Wed, 21 Apr 2021 13:13:16 GMT, Jim Laskey wrote: >> Move makeXXXSpilterator from public (@hidden) to protected. No API ch > > Jim Laskey 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 eight additional commits since > the last revision: > > - Merge branch 'master' into 8265137 > - Remove @hidden > - Correct the hierarchy of Random > - Remove extraneous references to makeXXXSpliterator > - Move makeXXXSpliterator methods to RandomSupport > - change static final from 'proxy' to 'PROXY' > - Make makeXXXSpliterator final > - Move makeXXXSpilterator from public (@hidden) to protected. No API change. Looks good +1 - PR: https://git.openjdk.java.net/jdk/pull/3469
Re: ReversibleCollection proposal
- Mail original - > De: "Stuart Marks" > À: "Remi Forax" > Cc: "core-libs-dev" > Envoyé: Mercredi 21 Avril 2021 19:38:23 > Objet: Re: ReversibleCollection proposal > On 4/19/21 4:05 PM, fo...@univ-mlv.fr wrote: >>> * There's a useful clump of semantics here, combined with sensible >>> operations >>> that >>> rely on those semantics. There are a lot of places in the spec where there >>> is >>> hedging of the form "if the collection has an ordering, then... otherwise >>> the >>> results are undefined". This is unnecessary in the context of a new type. >>> Furthermore, the new operations fit well with the new types' semantics, >>> with no >>> hedging necessary. >> >> You can only say that a reversible collection has an ordering. But usually >> you >> want the opposite, all collections that have an ordering are a reversible >> collection. But while this can be true for the implementations inside the JDK >> that will never be true in Java because there all already collection >> implementations with an ordering which do not implement ReversibleCollection. >> >> Sadly, this ship has sailed. >> The spec will still have to say "if the collection has an ordering", >> otherwise >> the new semantics is not a backward compatible. > > The value being provided here is that the ReversibleCollection interface > provides a > context within which specs no longer need to hedge about "if the collection > has > an > ordering". APIs that produce or consume ReversibleCollection no longer need to > include this hedge, or have disclaimers about ordering. Potential new > ReversibleCollection implementations also need not worry about this issue in > the > same way they did when they were forced to implement Collection directly. An API can not use ReversibleCollection as parameter before all the implementations of Collection that have an ordering are updated to implement ReversibleCollection instead of Collection. By example, if I add a method public void foo(ReversibleCollection c) in my library, people that want to be able to use foo with an existing collection with an ordering have to wait that collection implementation to be updated. So i will not add a method like foo in my API until enough (whatever enough means) implementation of Collection with an ordering have migrated. This is a similar issue as the migration from Python 2 to Python 3, it takes at least 10 years, so in 10 years, I will be able to had a public method that takes a ReversibleCollection in my API, without people not be able to using it. > > Of course there will always be ordered collections that don't implement > ReversibleCollection. (Examples of this are the Queue-but-not-Deque > implementations > in the JDK and 3rd party ordered collections that implement Collection > directly.) > Existing APIs that produce or consume Collection but have stipulations about > ordering may need to remain, although some could be adjusted depending on the > context. > > You rightly point out that this problem can never be solved in general. > However, > it's not a goal for ReversibleCollection to represent *all* ordered > collections, > so > it's hardly a criticism that it doesn't solve a problem that cannot be solved. > >>> * These semantics appear across a broad range of existing collection >>> types and >>> implementations. It's quite valuable to have a type that unifies the common >>> pieces >>> of List, Deque, SortedSet, and LinkedHashSet into a single abstraction. >> >> yes in term of documentation, but in Java, it also means that you can use >> that >> interface as a type. >> >> For a List, a Deque or a Set, there are known algorithms that takes such type >> has parameter so it makes sense to have such type. >> For a ReversibleCollection, i do not see many codes that will benefit taking >> a >> ReversibleCollection as parameter instead of using Collection. > > It seems likely that many public APIs (both in the JDK and in external > libraries) > will continue to use Collection, for compatibility reasons. > > In certain cases (such as LinkedHashMap, see below) the APIs could be > adjusted, > if > the compatibility impact is small or can be mitigated. > > ReversibleCollection unifies a broad set of existing collections without > requiring > any retrofitting at all. Many applications and libraries don't have their own > collection implementations; they just use the ones in the JDK. They can > benefit > from > the new APIs in ReversibleCollection immediately, without the need for any > retrofitting at all. nope, many applications do not use *only* the collection from the JDK, but also collections that comes from other libraries than msut to be upgraded to use ReversibleCollection. > > ReversibleCollection also provide opportunities for new code and for existing > APIs > that don't have stringent compatibility constraints. Consider an application > that > has a method wants to consume an ordered collection; a
Re: RFR: 8265591: Remove vestiages of intermediate JSR 175 annotation format
On Tue, 20 Apr 2021 23:37:15 GMT, Joe Darcy wrote: > During the recent review of JDK-8228988, I noticed again the comments in the > annotation parser about support for the pre-GA annotation format used before > JDK 5.0 shipped. During the development of annotations, there was a late > change to correct a flaw in the annotation encoding, JDK-5020908. > > I don't think it is necessary to carry forward support for this transient > format any longer and this changeset removes support from both core > reflection and javac. > > Clean runs of relevant test; I gauge this fix as no-reg hard. Marked as reviewed by jfranck (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3597
Re: ReversibleCollection proposal
- Mail original -
> De: "Stephen Colebourne"
> À: "core-libs-dev"
> Envoyé: Jeudi 22 Avril 2021 00:14:10
> Objet: Re: ReversibleCollection proposal
> On Wed, 21 Apr 2021 at 18:39, Stuart Marks wrote:
>> The value being provided here is that the ReversibleCollection interface
>> provides a
>> context within which specs no longer need to hedge about "if the collection
>> has
>> an
>> ordering". APIs that produce or consume ReversibleCollection no longer need
>> to
>> include this hedge, or have disclaimers about ordering. Potential new
>> ReversibleCollection implementations also need not worry about this issue in
>> the
>> same way they did when they were forced to implement Collection directly.
>
> Having thought about this for a few days my "interesting thought" is
> that adding a `ReversibleCollection` interface and adding additional
> methods can be orthogonal choices.
>
> Specifically, I do rather like Peter's suggestion of adding more
> methods to Collection. Adding these methods would be pretty useful in
> general and give the proposed change much greater reach:
>
> public interface Collection {
> default void addFirst(E e) { throw new
> UnsupportedOperationException(); }
> default E getFirst() { return iterator().next(); }
> default E removeFirst() { var i = iterator(); i.next();
> i.remove(); }
> }
>
> My view being that every collection has some notion of "first", even
> if that notion is "undefined". If that was the only change made, I
> think that would be a good move forward.
>
> These would be the various options:
>
> - 7 new methods on ReversibleCollection (Stuart's suggestion)
> - 7 new methods on Collection, no ReversibleCollection (Peter's suggestion)
> - 3 new methods on Collection, no ReversibleCollection (my suggestion #1)
> - 3 new methods on Collection, 4 methods on ReversibleCollection (my
> suggestion #2)
> - 7 new methods on Collection, 0 methods on ReversibleCollection (my
> suggestion #3)
>
> FWIW, I think I prefer my suggestion #2
I would like to preserve the invariant that, when calling a method on a
Collection/iterator, an UnsupportedOperationException only occurs when trying
to mutate that collection.
If we are ok with that, this means that addFirst can not be a default method of
Collection.
getFirst() on Collection is a nice addition has I already said.
And i'm not a big fan of removeFirst() on Collection, mostly because i think
that the sequence this.iterator + next() + remove() is pretty rare on a
Collection or a Set
(i would also like to keep the invariant that Set and Collection have the same
set of methods).
So addFirst/addLast/removeFirst/removeLast should be added to
SortedSet/NavigableSet, Deque, List and LinkedHashMap (if there are not already
present).
For reverse/descending, I still prefer to have descendingSet() on Set and
descendingList() on List, the same way we have keySet() or subList() (the
method names have the resulting interface has suffix).
This also solve the issue with LinkedList implementing both List and Deque.
I believe that the cost of introducing ReversibleCollection is too high
- it's not source backward compatible change
- replacing Collection/Set by ReversibleCollection/ReversibleSet has return
type of an existing interface/class is not a source backward compatible change
- to be able to use ReversibleCollection parameter of a public method of a
library, we have to wait the whole Java ecosystem to have been updated to
implement ReversibleCollection (Python 2/3 like migration).
>
> Stephen
Rémi
Re: Jpackage Mac entitlements
On 4/21/2021 6:15 PM, Michael Hall wrote: Reverted to Jdk 16 and temporarily couldn’t figure out why AppleScript wasn’t working again. I needed to provide an entitlement. At jpackage 16 I found I needed to do this differently with a .entitlements file in the resources directory, as I noticed in the verbose command output. I had previously noticed that jpackage 17 now includes a parameter to provide this. Will both options continue to be supported, resource dir and parm, or will parm become the only supported way at or after 17? Yes, if --mac-entitlement options exists, it will use it, otherwise , the previous behavior is maintained, the default resource entitlements.plist in the source code will be used unless overridden by .entitlements in the resource directory. /Andy
Re: [External] : Re: ReversibleCollection proposal
- Mail original -
> De: "Stuart Marks"
> À: "Remi Forax"
> Cc: "core-libs-dev"
> Envoyé: Mercredi 21 Avril 2021 20:53:25
> Objet: Re: [External] : Re: ReversibleCollection proposal
> On 4/19/21 2:01 PM, Remi Forax wrote:
>> - Mail original -
>> Thinking a little bit about your proposal,
>> introducing an interface right in the middle of a hierarchy is not a backward
>> compatible change
>> (you have an issue when the compiler has to use the lowest upper bound).
>>
>> By example
>>void m(List> list) { ... }
>>
>>var list = List.of(new LinkedHashSet(), List.of("foo"));
>>m(list); // does not compile anymore
>>
>> currently the type of list is List> but with your
>> proposal,
>> the type will be List>
>
> Yes, interesting. Not too difficult to fix though. Either change the method
> declaration to
>
> void m(List> list)
>
> or change the 'var' to an explicit declaration of List>.
or specify the type argument in between '.' and 'of'
var list = List.>of(new LinkedHashSet(),
List.of("foo"));
anyway, the issue here is that adding ReversibleCollection is a source breaking
change,
which significantly raises the bar to add this interface.
>
> s'marks
Rémi
RFC: jdeps output - "jdk8internals" vs "JDK removed internal API" after JDK-8213909
Hi, JDK-8213909 [1] (since JDK 12, backported into 11.0.11), aside from bringing great additions to transitive module analysis, introduced a slight change in output of jdeps: jdk-11.0.11/bin/jdeps -s reactor-core-3.4.5.jar reactor-core-3.4.5.jar -> jdk8internals reactor-core-3.4.5.jar -> java.base reactor-core-3.4.5.jar -> java.logging reactor-core-3.4.5.jar -> not found jdk-11.0.10/bin/jdeps -s reactor-core-3.4.5.jar reactor-core-3.4.5.jar -> JDK removed internal API reactor-core-3.4.5.jar -> java.base reactor-core-3.4.5.jar -> java.logging reactor-core-3.4.5.jar -> not found Note the difference "jdk8internals" vs "JDK removed internal API", not discussed in the relevant CSR [2] or the review thread [3]. This change is likely to affect some automation systems built around jdeps. Since the change has already landed, I wanted to check if there is a reason behind this naming change, and if "jdk8internals" should be the naming convention the users are expected to standardize their automation on going forward? I would assume most now are used to parsing for "JDK removed internal API" to determine if the project uses any removed APIs (and should probably be using --print-module-deps to feed jlink input), but I also see merit in having a single convention for mainline and CPU releases. -Aleksei [1] https://bugs.openjdk.java.net/browse/JDK-8213909 [2] https://bugs.openjdk.java.net/browse/JDK-8213915 [3] http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-November/056660.html
Re: RFR: 8263668: Update java.time to use instanceof pattern variable [v7]
On Wed, 21 Apr 2021 13:02:06 GMT, Andrey Turbanov wrote: >> 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 10 additional >> commits since the last revision: >> >> - Updated single letter pattern variable name in java/time/Duration >> - Merge remote-tracking branch 'origin/master' into JDK-8263668 >> - Updated single letter pattern variable names >> - Merge remote-tracking branch 'origin/master' into JDK-8263668 >> - Merge remote-tracking branch 'origin/master' into JDK-8263668 >> - Merge remote-tracking branch 'origin/master' into JDK-8263668 >> - Merge remote-tracking branch 'origin/master' into JDK-8263668 >> - Merge remote-tracking branch 'origin/master' into JDK-8263668 >> - Merge remote-tracking branch 'origin/master' into JDK-8263668 >> - 8263668: Update java.time to use instanceof pattern variable > > I was able to find (with IntelliJ IDEA help) few more places to improve > > java.time 27 warnings > class Clock 2 warnings > class FixedClock 1 warning > method equals(Object) 1 warning > WARNING Variable 'other' can be replaced with pattern > variable > class OffsetClock 1 warning > method equals(Object) 1 warning > WARNING Variable 'other' can be replaced with pattern > variable > class Instant 2 warnings > method until(Temporal, TemporalUnit) 1 warning > WARNING Variable 'f' can be replaced with pattern variable > method with(TemporalField, long) 1 warning > WARNING Variable 'f' can be replaced with pattern variable > class LocalDate 5 warnings > method minus(TemporalAmount) 1 warning > WARNING Variable 'periodToSubtract' can be replaced with > pattern variable > method plus(long, TemporalUnit) 1 warning > WARNING Variable 'f' can be replaced with pattern variable > method plus(TemporalAmount) 1 warning > WARNING Variable 'periodToAdd' can be replaced with pattern > variable > method range(TemporalField) 1 warning > WARNING Variable 'f' can be replaced with pattern variable > method with(TemporalField, long) 1 warning > WARNING Variable 'f' can be replaced with pattern variable > class LocalDateTime 8 warnings > method get(TemporalField) 1 warning > WARNING Variable 'f' can be replaced with pattern variable > method getLong(TemporalField) 1 warning > WARNING Variable 'f' can be replaced with pattern variable > method isSupported(TemporalField) 1 warning > WARNING Variable 'f' can be replaced with pattern variable > method minus(TemporalAmount) 1 warning > WARNING Variable 'periodToSubtract' can be replaced with > pattern variable > method plus(long, TemporalUnit) 1 warning > WARNING Variable 'f' can be replaced with pattern variable > method plus(TemporalAmount) 1 warning > WARNING Variable 'periodToAdd' can be replaced with pattern > variable > method range(TemporalField) 1 warning > WARNING Variable 'f' can be replaced with pattern variable > method with(TemporalField, long) 1 warning > WARNING Variable 'f' can be replaced with pattern variable > class LocalTime 1 warning > method with(TemporalField, long) 1 warning > WARNING Variable 'f' can be replaced with pattern variable > class OffsetDateTime 1 warning > method with(TemporalField, long) 1 warning > WARNING Variable 'f' can be replaced with pattern variable > class Year 1 warning > method with(TemporalField, long) 1 warning > WARNING Variable 'f' can be replaced with pattern variable > class YearMonth 1 warning > method with(TemporalField, long) 1 warning > WARNING Variable 'f' can be replaced with pattern variable > class ZonedDateTime 6 warnings > method equals(Object) 1 warning > WARNING Variable 'other' can be replaced with pattern > variable > method minus(TemporalAmount) 1 warning > WARNING Variable 'periodToSubtract' can be replaced with > pattern variable > method plus(TemporalAmount) 1 warning > WARNING Variable 'periodToAdd' can be replaced with pattern > variable > method with(TemporalAdjuster) 2 warnings >
Re: RFR: 8263668: Update java.time to use instanceof pattern variable [v8]
> Hi, > > Could someone please review my code for updating the code in the `java.time` > package to make use of the `instanceof` pattern variable? > > 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 12 additional commits since the last revision: - Merge remote-tracking branch 'origin/master' into JDK-8263668 - Merge remote-tracking branch 'origin/master' into JDK-8263668 - Updated single letter pattern variable name in java/time/Duration - Merge remote-tracking branch 'origin/master' into JDK-8263668 - Updated single letter pattern variable names - Merge remote-tracking branch 'origin/master' into JDK-8263668 - Merge remote-tracking branch 'origin/master' into JDK-8263668 - Merge remote-tracking branch 'origin/master' into JDK-8263668 - Merge remote-tracking branch 'origin/master' into JDK-8263668 - Merge remote-tracking branch 'origin/master' into JDK-8263668 - ... and 2 more: https://git.openjdk.java.net/jdk/compare/88c33d40...db6e9bb7 - Changes: - all: https://git.openjdk.java.net/jdk/pull/3170/files - new: https://git.openjdk.java.net/jdk/pull/3170/files/3dc1e075..db6e9bb7 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3170=07 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3170=06-07 Stats: 522 lines in 75 files changed: 254 ins; 160 del; 108 mod Patch: https://git.openjdk.java.net/jdk/pull/3170.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3170/head:pull/3170 PR: https://git.openjdk.java.net/jdk/pull/3170
Integrated: 8263668: Update java.time to use instanceof pattern variable
On Wed, 24 Mar 2021 09:56:16 GMT, Patrick Concannon wrote: > Hi, > > Could someone please review my code for updating the code in the `java.time` > package to make use of the `instanceof` pattern variable? > > Kind regards, > Patrick This pull request has now been integrated. Changeset: 28af31db Author:Patrick Concannon URL: https://git.openjdk.java.net/jdk/commit/28af31db Stats: 176 lines in 34 files changed: 0 ins; 47 del; 129 mod 8263668: Update java.time to use instanceof pattern variable Reviewed-by: lancea, ryadav, naoto, rriggs, dfuchs, scolebourne, chegar - PR: https://git.openjdk.java.net/jdk/pull/3170
Re: RFR: 8263512: [macos_aarch64] issues with calling va_args functions from invoke_native
On Thu, 22 Apr 2021 08:19:53 GMT, Nick Gasson wrote: > macOS on Apple silicon uses slightly different ABI conventions to the > standard AArch64 ABI. The differences are outlined in [1]. In > particular in the standard (AAPCS) ABI, variadic arguments may be passed > in either registers or on the stack following the normal calling > convention. To handle this, va_list is a struct containing separate > pointers for arguments located in integer registers, floating point > registers, and on the stack. Apple's ABI simplifies this by passing all > variadic arguments on the stack and the va_list type becomes a simple > char* pointer. > > This patch adds a new MacOsAArch64 CABI type and MacOsAArch64Linker to > represent the new ABI variant on macOS. StackVaList is based on > WinVaList lightly modified to handle the different TypeClasses on > AArch64. The original AArch64Linker is renamed to AapcsLinker and is > currently used for all non-Mac platforms. I think we also need to add a > WinAArch64 CABI but I haven't yet been able to test on a Windows system > so will do that later. > > The macOS ABI also uses a different method of spilling arguments to the > stack (the standard ABI pads each argument to a multiple of 8 byte stack > slots, but the Mac ABI packs arguments according to their natural > alignment). None of the existing tests exercise this so I'll open a new > JBS issue and work on that separately. > > Tested jdk_foreign on macOS AArch64, Linux AArch64, and Linux X86_64. @mcimadamore @JornVernee could you take a look? Thanks! - PR: https://git.openjdk.java.net/jdk/pull/3617
RFR: 8263512: [macos_aarch64] issues with calling va_args functions from invoke_native
macOS on Apple silicon uses slightly different ABI conventions to the standard AArch64 ABI. The differences are outlined in [1]. In particular in the standard (AAPCS) ABI, variadic arguments may be passed in either registers or on the stack following the normal calling convention. To handle this, va_list is a struct containing separate pointers for arguments located in integer registers, floating point registers, and on the stack. Apple's ABI simplifies this by passing all variadic arguments on the stack and the va_list type becomes a simple char* pointer. This patch adds a new MacOsAArch64 CABI type and MacOsAArch64Linker to represent the new ABI variant on macOS. StackVaList is based on WinVaList lightly modified to handle the different TypeClasses on AArch64. The original AArch64Linker is renamed to AapcsLinker and is currently used for all non-Mac platforms. I think we also need to add a WinAArch64 CABI but I haven't yet been able to test on a Windows system so will do that later. The macOS ABI also uses a different method of spilling arguments to the stack (the standard ABI pads each argument to a multiple of 8 byte stack slots, but the Mac ABI packs arguments according to their natural alignment). None of the existing tests exercise this so I'll open a new JBS issue and work on that separately. Tested jdk_foreign on macOS AArch64, Linux AArch64, and Linux X86_64. - Commit messages: - 8263512: [macos_aarch64] issues with calling va_args functions from invoke_native Changes: https://git.openjdk.java.net/jdk/pull/3617/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3617=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8263512 Stats: 770 lines in 12 files changed: 568 ins; 112 del; 90 mod Patch: https://git.openjdk.java.net/jdk/pull/3617.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3617/head:pull/3617 PR: https://git.openjdk.java.net/jdk/pull/3617
