Re: RFR: 8263968: CDS: java/lang/ModuleLayer.EMPTY_LAYER should be singleton
On Mon, 22 Mar 2021 19:40:19 GMT, Aleksei Voitylov wrote: > With CDS and G1, test api/java_lang/ModuleLayer/Boot.html fails in JDK 16. > > After JDK-8253081 with CDS enabled two instances of empty layer appear in the > runtime. One comes from default initialization of > java/lang/ModuleLayer.EMPTY_LAYER, another one comes from CDS archive and is > returned by ModuleLayer.boot().parents().get(0). The fix makes > java/lang/ModuleLayer.EMPTY_LAYER a singleton with both CDS on and off, > similar to java/lang/module/Configuration.EMPTY_CONFIGURATION. > > Testing: JCK 16, jtreg (including newly added test), pre-submit GitHub > actions tests. Seems reasonable. Though I hadn't realized CDS intruded into the Java layer that way! - Marked as reviewed by dholmes (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/3131
Re: RFR: 8263903: Use Cleaner instead of finalize to auto stop Timer thread [v3]
On Tue, 23 Mar 2021 01:03:55 GMT, Kim Barrett wrote: >> Please review this change to java.util.Timer, replacing the use of >> deprecated finalization-based cleanup with use of java.lang.ref.Cleaner. >> >> In addition, Timer.cancel now cancels any later execution of the the no >> longer relevant cleanup. >> >> Testing: >> mach5 tier1 >> New AutoStop test verifies the specified cleanup behavior. >> (There are existing tests involving Timer.cancel.) > > Kim Barrett has updated the pull request incrementally with one additional > commit since the last revision: > > make ThreadReaper constructor non-public Marked as reviewed by dholmes (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3106
Re: RFR: 8263903: Use Cleaner instead of finalize to auto stop Timer thread [v3]
> Please review this change to java.util.Timer, replacing the use of deprecated > finalization-based cleanup with use of java.lang.ref.Cleaner. > > In addition, Timer.cancel now cancels any later execution of the the no > longer relevant cleanup. > > Testing: > mach5 tier1 > New AutoStop test verifies the specified cleanup behavior. > (There are existing tests involving Timer.cancel.) Kim Barrett has updated the pull request incrementally with one additional commit since the last revision: make ThreadReaper constructor non-public - Changes: - all: https://git.openjdk.java.net/jdk/pull/3106/files - new: https://git.openjdk.java.net/jdk/pull/3106/files/2153a4c3..57500464 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3106=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3106=01-02 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/3106.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3106/head:pull/3106 PR: https://git.openjdk.java.net/jdk/pull/3106
Re: RFR: JDK-8259926: Error in jpackage sample usage in the help text
On Mon, 22 Mar 2021 20:40:09 GMT, Andy Herrick wrote: > JDK-8259926: Error in jpackage sample usage in the help text Marked as reviewed by naoto (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3132
Re: RFR: JDK-8259926: Error in jpackage sample usage in the help text
On Mon, 22 Mar 2021 20:40:09 GMT, Andy Herrick wrote: > JDK-8259926: Error in jpackage sample usage in the help text Marked as reviewed by almatvee (Committer). - PR: https://git.openjdk.java.net/jdk/pull/3132
Re: RFR: 8263903: Use Cleaner instead of finalize to auto stop Timer thread [v2]
On Mon, 22 Mar 2021 07:19:28 GMT, Kim Barrett wrote: >> Please review this change to java.util.Timer, replacing the use of >> deprecated finalization-based cleanup with use of java.lang.ref.Cleaner. >> >> In addition, Timer.cancel now cancels any later execution of the the no >> longer relevant cleanup. >> >> Testing: >> mach5 tier1 >> New AutoStop test verifies the specified cleanup behavior. >> (There are existing tests involving Timer.cancel.) > > Kim Barrett has updated the pull request incrementally with one additional > commit since the last revision: > > dholmes review The change looks good to me (though making the ThreadReaper ctor non-public would be nice). - Marked as reviewed by bchristi (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/3106
Re: RFR: JDK-8259926: Error in jpackage sample usage in the help text
On Mon, 22 Mar 2021 20:40:09 GMT, Andy Herrick wrote: > JDK-8259926: Error in jpackage sample usage in the help text Marked as reviewed by asemenyuk (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3132
Re: RFR: JDK-8263887: Re-create default icons
On Mon, 22 Mar 2021 13:03:10 GMT, Andy Herrick wrote: > JDK-8263887: Re-create default icons Marked as reviewed by asemenyuk (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3118
Re: RFR: 4890732: GZIPOutputStream doesn't support, in fact thwarts, use of optional GZIP fields [v3]
On Fri, 19 Mar 2021 08:19:58 GMT, Lin Zang wrote:
>> 4890732: GZIPOutputStream doesn't support, in fact thwarts, use of optional
>> GZIP fields
>
> Lin Zang has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - update copyright
> - reuse arguments constructor for non-argument one.
Hi Lin,
Again, thank you for taking on this 19+ year feature request.
I have not done a deep dive on the CSR, but wanted to get a few comments back
to you on a quick scan of the last PR update.
Personally, I would like to see more testing for a change such as this given
the age of the code:
- Please do not modify the existing test, you can either create a new test(s)
or add tests to the existing test class
- We should capture Gzip files with these headers set from other tools and
store the Gzip in an array within the test which can then be written to disk so
the tests can validate interoperability. Please see some of the other Zip
tests for an example
- We should have tests that include some , but not all of the additional fields
as I believe they are all optional according to the RFC.
- Please include some negative tests
I have also include some additional comments within the code
src/java.base/share/classes/java/util/zip/GZIPOutputStream.java line 193:
> 191: * @param generateHeaderCRC
> 192: *if {@code true} the header will include the CRC16 of the
> header.
> 193: * @param extraFieldBytes the byte array of extra filed,
filed -> field
src/java.base/share/classes/java/util/zip/GZIPOutputStream.java line 195:
> 193: * @param extraFieldBytes the byte array of extra filed,
> 194: *the generated header would calculate the
> byte[] size
> 195: *and fill it before the byte[] in header.
This is not clear what you are trying to articulate here
src/java.base/share/classes/java/util/zip/GZIPOutputStream.java line 269:
> 267: * @param generateHeaderCRC
> 268: *if {@code true} the header will include the CRC16 of the
> header.
> 269: * @param extraFieldBytes the byte array of extra filed,
filed -> field
src/java.base/share/classes/java/util/zip/GZIPOutputStream.java line 320:
> 318: * +---+---+=+
> 319: */
> 320: int xlen = extraFieldBytes.length;
On a quick look at the RFC, I noticed the following:
2.3.1.1. Extra field
If the FLG.FEXTRA bit is set, an "extra field" is present in
the header, with total length XLEN bytes. It consists of a
series of subfields, each of the form:
+---+---+---+---+==+
|SI1|SI2| LEN |... LEN bytes of subfield data ...|
+---+---+---+---+==+
SI1 and SI2 provide a subfield ID, typically two ASCII letters
with some mnemonic value. Jean-Loup Gailly
is maintaining a registry of subfield
IDs; please send him any subfield ID you wish to use. Subfield
IDs with SI2 = 0 are reserved for future use. The following
IDs are currently defined:
---
This does not seem to be accounted for or is it?
src/java.base/share/classes/java/util/zip/GZIPOutputStream.java line 323:
> 321: if (xlen > 0x) {
> 322: throw new ZipException("extra field size out of range");
> 323: }
Where is the ZipException documented that is being thrown?
src/java.base/share/classes/java/util/zip/GZIPOutputStream.java line 342:
> 340: *+=+
> 341: */
> 342: out.write(filename);
The RFC indicates:
If FNAME is set, an original file name is present,
terminated by a zero byte. The name must consist of ISO
8859-1 (LATIN-1) characters; on operating systems using
EBCDIC or any other character set for file names, the name
must be translated to the ISO LATIN-1 character set. This
is the original name of the file being compressed, with any
directory components removed, and, if the file being
compressed is on a file system with case insensitive names,
forced to lower case. There is no original file name if the
data was compressed from a source other than a named file;
for example, if the source was stdin on a Unix system, there
is no file name.
It looks like there is no validation being done so I am not sure what the
expectation is.
src/java.base/share/classes/java/util/zip/GZIPOutputStream.java line 357:
> 355: */
> 356: out.write(fileComment);
> 357: out.write(0);
The RFC states:
If FCOMMENT is set, a zero-terminated file comment is
Re: RFR: 8263763: Synthetic constructor parameters of enum are not considered for annotation indices
On Mon, 22 Mar 2021 07:52:45 GMT, Tagir F. Valeev wrote: >> *ejc* issues the same constructor that prefixes a `String` and an `int` >> parameter. I agree that the solution is not perfect, but I would prefer it >> over the reflection API throwing an `IndexOutOfBoundsException` upon calling >> `getAnnotations()`, which nobody really expects upon a getter invocation. >> >> I added a check to see if the enum constructor in question starts with a >> `String` and `int` parameter after checking if there are two excess >> parameters. I believe that that's at least an improvement over today's state >> where it's very unlikely that an enum constructor yields an incorrect >> result in the reflection API whereas the result it is always incorrect >> today, given the implementation of both *ejc* and *javac*. >> >> Ideally, this would of course also be fixed by javac (and ejc) such that the >> annotations are placed on the correct indices, but I still argue that the >> fix is an improvement for being able to properly process enum constructors >> for older class files also. > > @raphw a duplicate? https://bugs.openjdk.java.net/browse/JDK-8246586 Yes, indeed a duplicate. I linked the issue up against the original. - PR: https://git.openjdk.java.net/jdk/pull/3082
Re: RFR: JDK-8263887: Re-create default icons
On Mon, 22 Mar 2021 13:03:10 GMT, Andy Herrick wrote: > JDK-8263887: Re-create default icons Marked as reviewed by almatvee (Committer). - PR: https://git.openjdk.java.net/jdk/pull/3118
RFR: JDK-8259926: Error in jpackage sample usage in the help text
JDK-8259926: Error in jpackage sample usage in the help text - Commit messages: - JDK-8259926: Error in jpackage sample usage in the help text Changes: https://git.openjdk.java.net/jdk/pull/3132/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3132=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8259926 Stats: 4 lines in 3 files changed: 1 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/3132.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3132/head:pull/3132 PR: https://git.openjdk.java.net/jdk/pull/3132
Re: RFR: 8263968: CDS: java/lang/ModuleLayer.EMPTY_LAYER should be singleton
On Mon, 22 Mar 2021 19:40:19 GMT, Aleksei Voitylov wrote: > With CDS and G1, test api/java_lang/ModuleLayer/Boot.html fails in JDK 16. > > After JDK-8253081 with CDS enabled two instances of empty layer appear in the > runtime. One comes from default initialization of > java/lang/ModuleLayer.EMPTY_LAYER, another one comes from CDS archive and is > returned by ModuleLayer.boot().parents().get(0). The fix makes > java/lang/ModuleLayer.EMPTY_LAYER a singleton with both CDS on and off, > similar to java/lang/module/Configuration.EMPTY_CONFIGURATION. > > Testing: JCK 16, jtreg (including newly added test), pre-submit GitHub > actions tests. LGTM. Thanks for fixing this! - Marked as reviewed by iklam (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/3131
RFR: 8263968: CDS: java/lang/ModuleLayer.EMPTY_LAYER should be singleton
With CDS and G1, test api/java_lang/ModuleLayer/Boot.html fails in JDK 16. After JDK-8253081 with CDS enabled two instances of empty layer appear in the runtime. One comes from default initialization of java/lang/ModuleLayer.EMPTY_LAYER, another one comes from CDS archive and is returned by ModuleLayer.boot().parents().get(0). The fix makes java/lang/ModuleLayer.EMPTY_LAYER a singleton with both CDS on and off, similar to java/lang/module/Configuration.EMPTY_CONFIGURATION. Testing: JCK 16, jtreg (including newly added test), pre-submit GitHub actions tests. - Commit messages: - fix jcheck requirements - JDK-8263968: CDS: java/lang/ModuleLayer.EMPTY_LAYER should be singleton Changes: https://git.openjdk.java.net/jdk/pull/3131/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3131=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8263968 Stats: 24 lines in 3 files changed: 21 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/3131.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3131/head:pull/3131 PR: https://git.openjdk.java.net/jdk/pull/3131
8214761: Bug in parallel Kahan summation implementation
I created a PR for 8214761: https://github.com/openjdk/jdk/pull/2988 - but have been stuck waiting on OCA signatory status to be confirmed. Did something get lost in the shuffle or do I just need to be more patient. Thanks, Chris
Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators [v34]
> This PR is to introduce a new random number API for the JDK. The primary API > is found in RandomGenerator and RandomGeneratorFactory. Further description > can be found in the JEP https://openjdk.java.net/jeps/356 . > > javadoc can be found at > http://cr.openjdk.java.net/~jlaskey/prng/doc/api/java.base/java/util/random/package-summary.html > > old PR: https://github.com/openjdk/jdk/pull/1273 Jim Laskey has updated the pull request incrementally with two additional commits since the last revision: - RandomGeneratorFactory::all(Class category) @implNote was out of date - Clarify all() - Changes: - all: https://git.openjdk.java.net/jdk/pull/1292/files - new: https://git.openjdk.java.net/jdk/pull/1292/files/5e98d915..4562dd13 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=1292=33 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1292=32-33 Stats: 16 lines in 2 files changed: 3 ins; 0 del; 13 mod Patch: https://git.openjdk.java.net/jdk/pull/1292.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1292/head:pull/1292 PR: https://git.openjdk.java.net/jdk/pull/1292
Re: RFR: 8263358: Update java.lang to use instanceof pattern variable [v8]
On Mon, 22 Mar 2021 17:47:10 GMT, Patrick Concannon wrote: >> Hi, >> >> Could someone please review my code for updating the code in the `java.lang` >> 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 ten additional > commits since the last revision: > > - 8263358: Moved check for declaringClass to after lineNumber > - Merge remote-tracking branch 'origin/master' into JDK-8263358 > - 8263358: Added wildcard to instanceof check; reordered primitive equality > check > - Merge remote-tracking branch 'origin/master' into JDK-8263358 > - 8263358: Refactored missed equals method > - Merge remote-tracking branch 'origin/master' into JDK-8263358 > - 8263358: Refactored equals methods > - Merge remote-tracking branch 'origin/master' into JDK-8263358 > - 8263358: Update java.lang to use instanceof pattern variable Marked as reviewed by mchung (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2913
Re: RFR: 8263358: Update java.lang to use instanceof pattern variable [v8]
On Mon, 22 Mar 2021 17:47:10 GMT, Patrick Concannon wrote: >> Hi, >> >> Could someone please review my code for updating the code in the `java.lang` >> 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 ten additional > commits since the last revision: > > - 8263358: Moved check for declaringClass to after lineNumber > - Merge remote-tracking branch 'origin/master' into JDK-8263358 > - 8263358: Added wildcard to instanceof check; reordered primitive equality > check > - Merge remote-tracking branch 'origin/master' into JDK-8263358 > - 8263358: Refactored missed equals method > - Merge remote-tracking branch 'origin/master' into JDK-8263358 > - 8263358: Refactored equals methods > - Merge remote-tracking branch 'origin/master' into JDK-8263358 > - 8263358: Update java.lang to use instanceof pattern variable The new changes look good to me. - Marked as reviewed by dfuchs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/2913
Re: RFR: 8263358: Update java.lang to use instanceof pattern variable [v8]
> Hi, > > Could someone please review my code for updating the code in the `java.lang` > 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 ten additional commits since the last revision: - 8263358: Moved check for declaringClass to after lineNumber - Merge remote-tracking branch 'origin/master' into JDK-8263358 - 8263358: Added wildcard to instanceof check; reordered primitive equality check - Merge remote-tracking branch 'origin/master' into JDK-8263358 - 8263358: Refactored missed equals method - Merge remote-tracking branch 'origin/master' into JDK-8263358 - 8263358: Refactored equals methods - Merge remote-tracking branch 'origin/master' into JDK-8263358 - 8263358: Update java.lang to use instanceof pattern variable - Changes: - all: https://git.openjdk.java.net/jdk/pull/2913/files - new: https://git.openjdk.java.net/jdk/pull/2913/files/b89027c4..a45233e8 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2913=07 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2913=06-07 Stats: 545 lines in 83 files changed: 236 ins; 22 del; 287 mod Patch: https://git.openjdk.java.net/jdk/pull/2913.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2913/head:pull/2913 PR: https://git.openjdk.java.net/jdk/pull/2913
Re: RFR: 8263358: Update java.lang to use instanceof pattern variable [v5]
On Mon, 15 Mar 2021 13:49:56 GMT, Pavel Rappo wrote: >> yes, >> javac should emit a warning in that case, that also the answer of Brian. >> >> https://mail.openjdk.java.net/pipermail/amber-spec-experts/2021-March/002925.html > > Then we should use an unbounded wildcard here and in similar places to avoid > "rawtype" warnings in the future. I've added the wildcard to the pattern variable check now. See b89027c - PR: https://git.openjdk.java.net/jdk/pull/2913
Re: RFR: 8263358: Update java.lang to use instanceof pattern variable [v5]
On Tue, 16 Mar 2021 19:26:35 GMT, Daniel Fuchs wrote: >> `declaringClass` is a string representing the class name (not the `Class` >> object). The variable name indeed causes confusion. > > Doh. My mistake. Was thinking about `StackFrame`. Thanks Mandy! I've reordered the checks as suggested and the updated code can be found in commit b89027c - PR: https://git.openjdk.java.net/jdk/pull/2913
Withdrawn: JDK-8263154: [macos] DMG builds have finder errors
On Sat, 13 Mar 2021 14:39:40 GMT, Andy Herrick wrote: > JDK-8263154: [macos] DMG builds have finder errors This pull request has been closed without being integrated. - PR: https://git.openjdk.java.net/jdk/pull/2987
RFR: 8203359: Container level resources events
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). - Commit messages: - Formatting spaces - 8203359: Container level resources events Changes: https://git.openjdk.java.net/jdk/pull/3126/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3126=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8203359 Stats: 389 lines in 8 files changed: 386 ins; 0 del; 3 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
Integrated: 8261785: Calling "main" method in anonymous nested class crashes the JVM
On Sun, 14 Mar 2021 23:34:55 GMT, Henry Jen wrote: > This patch ensure launcher won't crash JVM for the new static Methods from > local/anonymous class on MacOS. > > As @dholmes-ora pointed out in the analysis, this is a MacOS specific bug > when the launcher trying to grab class name to be displayed as the > Application name on the menu. > > The fix is to not setting name, test shows that GUI java application shows > 'bin' as the application name. It's possible for us to set the name to > something more friendly, for example, "Java", but I am not sure that should > be launcher's responsibility to choose such a default name. It seems to me > the consumer of the JAVA_MAIN_CLASS_%d environment variable should be > responsible to pick such name in case the environment variable is not set. This pull request has now been integrated. Changeset: b2df5137 Author:Henry Jen URL: https://git.openjdk.java.net/jdk/commit/b2df5137 Stats: 143 lines in 3 files changed: 142 ins; 0 del; 1 mod 8261785: Calling "main" method in anonymous nested class crashes the JVM Reviewed-by: serb - PR: https://git.openjdk.java.net/jdk/pull/2999
Integrated: 8263729: [test] divert spurious output away from stream under test in ProcessBuilder Basic test
On Wed, 17 Mar 2021 14:16:36 GMT, Roger Riggs wrote: > Intermittent failures on Windows in a test of destroying the child warrant > extending the time the parent waits after starting the child before > destroying the child. This pull request has now been integrated. Changeset: 0abbfb2f Author:Roger Riggs URL: https://git.openjdk.java.net/jdk/commit/0abbfb2f Stats: 28 lines in 1 file changed: 24 ins; 0 del; 4 mod 8263729: [test] divert spurious output away from stream under test in ProcessBuilder Basic test Reviewed-by: stuefe, iklam - PR: https://git.openjdk.java.net/jdk/pull/3049
Re: RFR: 8263729: [test] divert spurious output away from stream under test in ProcessBuilder Basic test [v4]
On Sat, 20 Mar 2021 05:55:08 GMT, Thomas Stuefe wrote: >> Roger Riggs has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Clarify modification of argument list after creating ProcessBuilder > > Looks good to me, if the intent was to fix potential write blockages in the > child process (in which case maybe reformulate the issue title). > > I'm sorry I derailed your original wait idea, as well as Ioi's ideas of > syncing with the child. Both may still be needed after this fix. > > Cheers, Thomas Thanks for the reviews and the suggestion to redirect the output to the inherited streams. - PR: https://git.openjdk.java.net/jdk/pull/3049
Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators [v32]
On Thu, 18 Mar 2021 21:43:16 GMT, Joe Darcy wrote:
>> Jim Laskey has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains 67 commits:
>>
>> - Merge branch 'master' into 8248862
>> - Review revisions
>> - Missing @since
>> - Review revisions
>> - Review requested changes
>> - Merge branch 'master' into 8248862
>> - Remove conflicts
>> - Use isAnnotationPresent
>> - Introduce isDeprecated
>> - Update javadoc
>> - ... and 57 more:
>> https://git.openjdk.java.net/jdk/compare/8c8d1b31...63094f9d
>
> src/java.base/share/classes/java/util/random/RandomGenerator.java line 1290:
>
>> 1288: * @return a new object that is a copy of this generator
>> 1289: */
>> 1290: LeapableGenerator copy();
>
> Suggest adding an Override annotation here and possibly to inheritDoc the
> text from Jumpable.copy.
Fails due to result variance.
> src/java.base/share/classes/java/util/random/RandomGenerator.java line 1455:
>
>> 1453: * period of this generator
>> 1454: */
>> 1455: void jump(double distance);
>
> Suggest Override and inheritDoc combo.
jump(double distance) does not occur in superinterface. The ability to jump
arbitrarily is specific to ArbitrarilyJumpableGenerator.
> src/java.base/share/classes/java/util/random/RandomGenerator.java line 1465:
>
>> 1463: * @implSpec The default implementation invokes
>> jump(jumpDistance()).
>> 1464: */
>> 1465: default void jump() { jump(jumpDistance()); }
>
> Should be able to avoid defining the jump method in this subinterface.
jump(double distance) does not occur in superinterface. The ability to jump
arbitrarily is specific to ArbitrarilyJumpableGenerator.
> src/java.base/share/classes/java/util/random/RandomGenerator.java line 1486:
>
>> 1484: * ({@link Long#MAX_VALUE Long.MAX_VALUE}).
>> 1485: */
>> 1486: default Stream jumps(double
>> distance) {
>
> Suggest adding an Override annotation.
jump(double distance) does not occur in superinterface. The ability to jump
arbitrarily is specific to ArbitrarilyJumpableGenerator.
> src/java.base/share/classes/java/util/random/RandomGenerator.java line 1521:
>
>> 1519: * {@link ArbitrarilyJumpableGenerator#leapDistance()
>> leapDistance}().
>> 1520: */
>> 1521: default void leap() { jump(leapDistance()); }
>
> Should need to define this method in the subinterface of Leapable.
jump(double distance) does not occur in superinterface. The ability to jump
arbitrarily is specific to ArbitrarilyJumpableGenerator.
> src/java.base/share/classes/java/util/random/RandomGenerator.java line 1538:
>
>> 1536: * returns the copy.
>> 1537: */
>> 1538: default ArbitrarilyJumpableGenerator copyAndJump(double
>> distance) {
>
> Suggest Override and inheritDoc.
jump(double distance) does not occur in superinterface. The ability to jump
arbitrarily is specific to ArbitrarilyJumpableGenerator.
-
PR: https://git.openjdk.java.net/jdk/pull/1292
Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators [v33]
> This PR is to introduce a new random number API for the JDK. The primary API > is found in RandomGenerator and RandomGeneratorFactory. Further description > can be found in the JEP https://openjdk.java.net/jeps/356 . > > javadoc can be found at > http://cr.openjdk.java.net/~jlaskey/prng/doc/api/java.base/java/util/random/package-summary.html > > old PR: https://github.com/openjdk/jdk/pull/1273 Jim Laskey has updated the pull request incrementally with one additional commit since the last revision: Cleaned up ints(), longs(), doubles() - Changes: - all: https://git.openjdk.java.net/jdk/pull/1292/files - new: https://git.openjdk.java.net/jdk/pull/1292/files/63094f9d..5e98d915 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=1292=32 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1292=31-32 Stats: 782 lines in 4 files changed: 310 ins; 416 del; 56 mod Patch: https://git.openjdk.java.net/jdk/pull/1292.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1292/head:pull/1292 PR: https://git.openjdk.java.net/jdk/pull/1292
Integrated: 8263855: Use the blessed modifier order in java.management/naming
On Thu, 18 Mar 2021 18:26:20 GMT, Alex Blewitt wrote: > As with #2993 changing the order of `final static` to `static final` for the > `java.management`, `java.management.rmi` and `java.naming` modules. This pull request has now been integrated. Changeset: 5262d95b Author:Alex Blewitt Committer: Claes Redestad URL: https://git.openjdk.java.net/jdk/commit/5262d95b Stats: 261 lines in 69 files changed: 0 ins; 0 del; 261 mod 8263855: Use the blessed modifier order in java.management/naming Reviewed-by: redestad, aefimov, dfuchs - PR: https://git.openjdk.java.net/jdk/pull/3078
RFR: JDK-8263887: Re-create default icons
JDK-8263887: Re-create default icons - Commit messages: - JDK-8263887: Re-create default icons Changes: https://git.openjdk.java.net/jdk/pull/3118/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3118=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8263887 Stats: 32 lines in 2 files changed: 7 ins; 21 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/3118.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3118/head:pull/3118 PR: https://git.openjdk.java.net/jdk/pull/3118
Re: RFR: 8263855: Use the blessed modifier order in java.management/naming [v2]
On Fri, 19 Mar 2021 17:13:58 GMT, Alex Blewitt wrote: >> As with #2993 changing the order of `final static` to `static final` for the >> `java.management`, `java.management.rmi` and `java.naming` modules. > > Alex Blewitt has updated the pull request incrementally with one additional > commit since the last revision: > > Added more replacements of 'final public' with 'public final' Marked as reviewed by dfuchs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3078
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v29]
> Please review the implementation of JEP 391: macOS/AArch64 Port. > > It's heavily based on existing ports to linux/aarch64, macos/x86_64, and > windows/aarch64. > > Major changes are in: > * src/hotspot/cpu/aarch64: support of the new calling convention (subtasks > JDK-8253817, JDK-8253818) > * src/hotspot/os_cpu/bsd_aarch64: copy of os_cpu/linux_aarch64 with necessary > adjustments (JDK-8253819) > * src/hotspot/share, test/hotspot/gtest: support of write-xor-execute (W^X), > required on macOS/AArch64 platform. It's implemented with > pthread_jit_write_protect_np provided by Apple. The W^X mode is local to a > thread, so W^X mode change relates to the java thread state change (for java > threads). In most cases, JVM executes in write-only mode, except when calling > a generated stub like SafeFetch, which requires a temporary switch to > execute-only mode. The same execute-only mode is enabled when a java thread > executes in java or native states. This approach of managing W^X mode turned > out to be simple and efficient enough. > * src/jdk.hotspot.agent: serviceability agent implementation (JDK-8254941) Anton Kozlov has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 115 commits: - Merge branch 'master' into jdk-macos - JDK-8262491: bsd_aarch64 part - JDK-8263002: bsd_aarch64 part - Merge remote-tracking branch 'upstream/jdk/master' into jdk-macos - Wider #ifdef block - Fix most of issues in java/foreign/ tests Failures related to va_args are tracked in JDK-8263512. - Add Azul copyright - Update Oracle copyright years - Use Thread::current_or_null_safe in SafeFetch - 8262903: [macos_aarch64] Thread::current() called on detached thread - ... and 105 more: https://git.openjdk.java.net/jdk/compare/a9d2267f...5add9269 - Changes: https://git.openjdk.java.net/jdk/pull/2200/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2200=28 Stats: 2947 lines in 75 files changed: 2838 ins; 27 del; 82 mod Patch: https://git.openjdk.java.net/jdk/pull/2200.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2200/head:pull/2200 PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8263855: Use the blessed modifier order in java.management/naming [v2]
On Fri, 19 Mar 2021 17:13:58 GMT, Alex Blewitt wrote: >> As with #2993 changing the order of `final static` to `static final` for the >> `java.management`, `java.management.rmi` and `java.naming` modules. > > Alex Blewitt has updated the pull request incrementally with one additional > commit since the last revision: > > Added more replacements of 'final public' with 'public final' `java.naming` module changes look good to me. - Marked as reviewed by aefimov (Committer). PR: https://git.openjdk.java.net/jdk/pull/3078
Re: RFR: 8263903: Use Cleaner instead of finalize to auto stop Timer thread [v2]
On Mon, 22 Mar 2021 07:19:28 GMT, Kim Barrett wrote: >> Please review this change to java.util.Timer, replacing the use of >> deprecated finalization-based cleanup with use of java.lang.ref.Cleaner. >> >> In addition, Timer.cancel now cancels any later execution of the the no >> longer relevant cleanup. >> >> Testing: >> mach5 tier1 >> New AutoStop test verifies the specified cleanup behavior. >> (There are existing tests involving Timer.cancel.) > > Kim Barrett has updated the pull request incrementally with one additional > commit since the last revision: > > dholmes review The update to java.util.Timer looks okay. A minor comment is that the ThreadReaper constructor does not need to be public. - Marked as reviewed by alanb (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/3106
Re: RFR: 8263358: Update java.lang to use instanceof pattern variable [v7]
> Hi, > > Could someone please review my code for updating the code in the `java.lang` > package to make use of the `instanceof` pattern variable? > > Kind regards, > Patrick Patrick Concannon has updated the pull request incrementally with one additional commit since the last revision: 8263358: Added wildcard to instanceof check; reordered primitive equality check - Changes: - all: https://git.openjdk.java.net/jdk/pull/2913/files - new: https://git.openjdk.java.net/jdk/pull/2913/files/e6746c9f..b89027c4 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2913=06 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2913=05-06 Stats: 3 lines in 2 files changed: 1 ins; 1 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/2913.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2913/head:pull/2913 PR: https://git.openjdk.java.net/jdk/pull/2913
RFR: JDK-8064681 : Jaxp unit test need to be improved
https://bugs.openjdk.java.net/browse/JDK-8064681 - Commit messages: - JDK-8064681 : Jaxp unit test need to be improved Changes: https://git.openjdk.java.net/jdk/pull/3115/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3115=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8064681 Stats: 541 lines in 6 files changed: 125 ins; 268 del; 148 mod Patch: https://git.openjdk.java.net/jdk/pull/3115.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3115/head:pull/3115 PR: https://git.openjdk.java.net/jdk/pull/3115
Re: java.io.File#toPath() on Windows doesn't understand "NUL:" (null device)?
On 22/03/21 2:36 pm, Alan Bateman wrote: On 20/03/2021 07:16, Jaikiran Pai wrote: : I received some inputs on that Ant bugzilla issue. Based on that, I was able to reproduce the exception and IMO it's a bug in Files.newOutputStream() API. I have opened https://bugs.openjdk.java.net/browse/JDK-8263898 with the relevant details. I considered this a bug and took the liberty of opening that JBS issue because as I note in that issue, this only happens specifically when TRUNCATE_EXISTING (default) option gets used against "nul" on Windows. OutputStream.nullOutputStream() may be a better and more portable alternative. In general, the DOS era reserved names (including NUL) are very under specified and many file operations lead to surprising behavior (this isn't solely a JDK issue, I think other runtimes and languages also get tripped up). In this case, the attempt to truncate the file to zero length is failing. Sure, it can be be worked around but workarounds like this tend to cause issues in other unusual cases so care is required. Understood. Thank you Alan for those inputs. Just yesterday, Stefan (one of the Ant developers) has implemented a way where we allow users to discard output without relying on null devices. It uses the approach that you note (although we use an internal NullOutputStream, since we want Ant to be usable in Java 8 where OutputStream.nullOutputStream() isn't present). We will be asking our users to move to this new way/attribute instead of relying on null devices. -Jaikiran
Re: RFR: 8263358: Update java.lang to use instanceof pattern variable [v6]
> Hi, > > Could someone please review my code for updating the code in the `java.lang` > 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 six additional commits since the last revision: - Merge remote-tracking branch 'origin/master' into JDK-8263358 - 8263358: Refactored missed equals method - Merge remote-tracking branch 'origin/master' into JDK-8263358 - 8263358: Refactored equals methods - Merge remote-tracking branch 'origin/master' into JDK-8263358 - 8263358: Update java.lang to use instanceof pattern variable - Changes: - all: https://git.openjdk.java.net/jdk/pull/2913/files - new: https://git.openjdk.java.net/jdk/pull/2913/files/f7924d27..e6746c9f Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2913=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2913=04-05 Stats: 16598 lines in 622 files changed: 7556 ins; 5892 del; 3150 mod Patch: https://git.openjdk.java.net/jdk/pull/2913.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2913/head:pull/2913 PR: https://git.openjdk.java.net/jdk/pull/2913
Re: java.io.File#toPath() on Windows doesn't understand "NUL:" (null device)?
On 20/03/2021 07:16, Jaikiran Pai wrote: : I received some inputs on that Ant bugzilla issue. Based on that, I was able to reproduce the exception and IMO it's a bug in Files.newOutputStream() API. I have opened https://bugs.openjdk.java.net/browse/JDK-8263898 with the relevant details. I considered this a bug and took the liberty of opening that JBS issue because as I note in that issue, this only happens specifically when TRUNCATE_EXISTING (default) option gets used against "nul" on Windows. OutputStream.nullOutputStream() may be a better and more portable alternative. In general, the DOS era reserved names (including NUL) are very under specified and many file operations lead to surprising behavior (this isn't solely a JDK issue, I think other runtimes and languages also get tripped up). In this case, the attempt to truncate the file to zero length is failing. Sure, it can be be worked around but workarounds like this tend to cause issues in other unusual cases so care is required. -Alan.
Re: RFR: 8263763: Synthetic constructor parameters of enum are not considered for annotation indices
On Fri, 19 Mar 2021 14:34:00 GMT, Rafael Winterhalter wrote: >> A general comment, the enum constructor situation is a bit tricky as >> >> 1) There is no 100% reliable way to determine which of a enum constructor's >> args are synthetic. >> >> 2) How a Java compiler generates enum constructors is a compiler-internal >> contract since all the enum constructors must be private. >> >> It is true that javac has consistently added two extra parameters as an >> implementation detail. Offhand, I don't know if ecj in particular has made >> the same choice. javac is not obliged to continue implementing enum >> constructors in this manner, so for better robustness, I think a fix in this >> space needs to be able to accommodate situations other than exactly N+2 >> parameters. > > *ejc* issues the same constructor that prefixes a `String` and an `int` > parameter. I agree that the solution is not perfect, but I would prefer it > over the reflection API throwing an `IndexOutOfBoundsException` upon calling > `getAnnotations()`, which nobody really expects upon a getter invocation. > > I added a check to see if the enum constructor in question starts with a > `String` and `int` parameter after checking if there are two excess > parameters. I believe that that's at least an improvement over today's state > where it's very unlikely that an enum constructor yields an incorrect result > in the reflection API whereas the result it is always incorrect today, given > the implementation of both *ejc* and *javac*. > > Ideally, this would of course also be fixed by javac (and ejc) such that the > annotations are placed on the correct indices, but I still argue that the fix > is an improvement for being able to properly process enum constructors for > older class files also. @raphw a duplicate? https://bugs.openjdk.java.net/browse/JDK-8246586 - PR: https://git.openjdk.java.net/jdk/pull/3082
Re: RFR: 8263903: Use Cleaner instead of finalize to auto stop Timer thread [v2]
On Mon, 22 Mar 2021 07:19:28 GMT, Kim Barrett wrote: >> Please review this change to java.util.Timer, replacing the use of >> deprecated finalization-based cleanup with use of java.lang.ref.Cleaner. >> >> In addition, Timer.cancel now cancels any later execution of the the no >> longer relevant cleanup. >> >> Testing: >> mach5 tier1 >> New AutoStop test verifies the specified cleanup behavior. >> (There are existing tests involving Timer.cancel.) > > Kim Barrett has updated the pull request incrementally with one additional > commit since the last revision: > > dholmes review This looks good to me, but of course core-libs folk need to review. Thanks, David - Marked as reviewed by dholmes (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/3106
Re: RFR: 8263903: Use Cleaner instead of finalize to auto stop Timer thread [v2]
On Mon, 22 Mar 2021 07:15:24 GMT, Kim Barrett wrote: >> Please review this change to java.util.Timer, replacing the use of >> deprecated finalization-based cleanup with use of java.lang.ref.Cleaner. >> >> In addition, Timer.cancel now cancels any later execution of the the no >> longer relevant cleanup. >> >> Testing: >> mach5 tier1 >> New AutoStop test verifies the specified cleanup behavior. >> (There are existing tests involving Timer.cancel.) > > Kim Barrett has updated the pull request incrementally with one additional > commit since the last revision: > > dholmes review test/jdk/java/util/Timer/AutoStop.java line 27: > 25: * @test > 26: * @bug 8263903 > 27: * @requires vm.gc != "Epsilon" I added the `@requires` late in dev/test and used the wrong test. - PR: https://git.openjdk.java.net/jdk/pull/3106
Re: RFR: 8263903: Use Cleaner instead of finalize to auto stop Timer thread [v2]
> Please review this change to java.util.Timer, replacing the use of deprecated > finalization-based cleanup with use of java.lang.ref.Cleaner. > > In addition, Timer.cancel now cancels any later execution of the the no > longer relevant cleanup. > > Testing: > mach5 tier1 > New AutoStop test verifies the specified cleanup behavior. > (There are existing tests involving Timer.cancel.) Kim Barrett has updated the pull request incrementally with one additional commit since the last revision: dholmes review - Changes: - all: https://git.openjdk.java.net/jdk/pull/3106/files - new: https://git.openjdk.java.net/jdk/pull/3106/files/0d120f56..2153a4c3 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3106=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3106=00-01 Stats: 5 lines in 1 file changed: 2 ins; 1 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/3106.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3106/head:pull/3106 PR: https://git.openjdk.java.net/jdk/pull/3106
Problem signing Windows JPackage installers
Hi, The installers that JPackage 16 generates seem to just extract an .msi which performs the actual installation. There are two problems with that: 1. The .msi is extracted to a temporary location and has a temporary name. So the windows prompt that shows up and asks the user whether they are sure they want to perform the installation does not show the name of the installer/application. 2. Furthermore, even if the .exe installer is signed with a code signing certificate, since the window prompts on the extracted .msi, the prompt is still colored yellow instead of blue (which is what it should be when the installer is signed) In my use case it is essential that the prompt appears blue, so I'm forced to use a hybrid approach of generating the app image with JPackage and then using a different installer framework (like NSIS) to generate the final distributable. Of course this is far from ideal. Just thought I'd let you know and best regards, Zlatin Balevsky
Re: RFR: 8263903: Use Cleaner instead of finalize to auto stop Timer thread
On Mon, 22 Mar 2021 05:40:10 GMT, Kim Barrett wrote:
>> test/jdk/java/util/Timer/AutoStop.java line 47:
>>
>>> 45: public void run() {
>>> 46: tdThread = Thread.currentThread();
>>> 47: synchronized(wakeup) {
>>
>> tdThread should be set inside the sync block, and then doesn't need to be
>> declared volatile
>
> Without volatile is the while loop still okay? And the read for the join
> call?
Yes the while loop is okay because the access is within a synchronized block.
When the loop exits tdThread is seen as non-null by the current thread, and
that field is never written to again, so when the current thread does the
join() it has to be using the non-null value of tdThread that it previously saw.
>> test/jdk/java/util/Timer/AutoStop.java line 67:
>>
>>> 65: t.schedule(new TimerTask() {
>>> 66: public void run() {
>>> 67: ++counter;
>>
>> This is not thread-safe. Operations on volatile variables are not atomic.
>
> Only one thread (the timer's thread) is writing, via a sequential series of
> task executions, so the simple increments are fine. I made `counter`
> volatile because it's being written by one thread and read by a different
> thread. Happy to drop the qualifier if that's not needed.
Apologies - I thought there was real concurrency going on there. :)
The volatile is correct for the reason you cited.
-
PR: https://git.openjdk.java.net/jdk/pull/3106
