Integrated: 8282060: RemoteRuntimeImageTest is not actually testing on JDK 8
On Thu, 5 May 2022 09:00:47 GMT, Athijegannathan Sundararajan wrote: > This test requires jdk8 to be available while running jdk tests. But > JDK8_HOME is defined to be BOOT_JDK and so version check fails in the test. > The test vacuously passes just printing a message. There are already tests > that exercise jrt-fs.jar on the same JDK being tested and so basic jrt-fs.jar > is covered for same target JDK case. This pull request has now been integrated. Changeset: ede06c3c Author:Athijegannathan Sundararajan URL: https://git.openjdk.java.net/jdk/commit/ede06c3c5f74c64dac3889d35b02541897ba4d94 Stats: 202 lines in 3 files changed: 0 ins; 202 del; 0 mod 8282060: RemoteRuntimeImageTest is not actually testing on JDK 8 Reviewed-by: alanb, erikj - PR: https://git.openjdk.java.net/jdk/pull/8547
RFR: 8282060: RemoteRuntimeImageTest is not actually testing on JDK 8
This test requires jdk8 to be available while running jdk tests. But JDK8_HOME is defined to be BOOT_JDK and so version check fails in the test. The test vacuously passes just printing a message. There are already tests that exercise jrt-fs.jar on the same JDK being tested and so basic jrt-fs.jar is covered for same target JDK case. - Commit messages: - 8282060: RemoteRuntimeImageTest is not actually testing on JDK 8 Changes: https://git.openjdk.java.net/jdk/pull/8547/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8547=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8282060 Stats: 202 lines in 3 files changed: 0 ins; 202 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/8547.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8547/head:pull/8547 PR: https://git.openjdk.java.net/jdk/pull/8547
Re: RFR: 8285485: Fix typos in corelibs
On Fri, 22 Apr 2022 15:08:51 GMT, Magnus Ihse Bursie wrote: > I ran `codespell` on modules owned by core-libs, and accepted those changes > where it indeed discovered real typos. > > I will update copyright years using a script before pushing (otherwise like > every second change would be a copyright update, making reviewing much > harder). > > The long term goal here is to make tooling support for running `codespell`. > The trouble with automating this is of course all false positives. But before > even trying to solve that issue, all true positives must be fixed. Hence this > PR. LGTM src/jdk.jlink/share/classes/jdk/tools/jlink/resources/plugins_zh_CN.properties line 188: > 186: main.plugin.module=\u63D2\u4EF6\u6A21\u5757 > 187: > 188: main.plugin.category=\u7C7B\u522B what's this? - Marked as reviewed by sundar (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8364
Re: RFR: 8283060: RawNativeLibraries should allow multiple clients to load/unload the same library
On Tue, 29 Mar 2022 20:05:50 GMT, Mandy Chung wrote: > A small improvement to `RawNativeLibraries`. `RawNativeLibraries::load` > returns the same `NativeLibrary` instance of a given path if it's called > multiple times. This means that multiple clients have to coordinate that the > last one using the loaded library is the one to close the library by calling > `RawNativeLibraries::unload`; otherwise, an exception may be thrown. > > This patch changes `RawNativeLibraries::load` to delegate to the underlying > platform-specific library loading mechanism e.g. dlopen/dlclose that keeps > the reference count. So each call to `RawNativeLibraries::load` and `unload` > is simply a call to dlopen and dlclose respectively. Each client of calling > `RawNativeLibraries::load` is responsible for calling > `RawNativeLibraries::unload`. This will be consistent with the current > behavior when you call `load` and `unload` with a different > `RawNativeLibraries` instance. LGTM - Marked as reviewed by sundar (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8022
Re: RFR: JDK-8283416: Update java.lang.invoke.MethodHandle to use sealed classes
On Mon, 21 Mar 2022 18:17:06 GMT, Mandy Chung wrote:
> > $ javac com/acme/*.java
> > is fine. Am I missing something?
>
> If you make P.Q private, it will fail the compilation.
>
Yes, I got that part. I did make a comment earlier ("Two of those subclasses
are nested "private" in another class and hence cannot be referred from
DelegatingMethodHandle's permits clause, right?"). My example was in response
to "I did try to name the nested type in the permits clause, and it was not
accepted by javac." comment by @jddarcy. I wondered if there is any other issue
with permits beyond private access of those 2 nested classes.
> To make `DelegatingMethodHandle` a sealed class, we could make
> `AsVarargsCollector` and `WrappedMember` package-private classes.
right.
-
PR: https://git.openjdk.java.net/jdk/pull/7881
Re: RFR: JDK-8283416: Update java.lang.invoke.MethodHandle to use sealed classes
On Sun, 20 Mar 2022 21:31:15 GMT, Joe Darcy wrote: > Small refactoring to use sealed classes in the MethodHandle implementation > hierarchy. > > DelegatingMethodHandle is non-sealed rather than sealed since it has two > subclasses, one defined as a nested class and only a separate type in the > same package. The permits clause does not allow listed those two kinds of > subclasses. > > Please also review the corresponding CSR > https://bugs.openjdk.java.net/browse/JDK-8283434 LGTM - Marked as reviewed by sundar (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7881
Re: RFR: JDK-8283416: Update java.lang.invoke.MethodHandle to use sealed classes
On Sun, 20 Mar 2022 21:31:15 GMT, Joe Darcy wrote:
> Small refactoring to use sealed classes in the MethodHandle implementation
> hierarchy.
>
> DelegatingMethodHandle is non-sealed rather than sealed since it has two
> subclasses, one defined as a nested class and only a separate type in the
> same package. The permits clause does not allow listed those two kinds of
> subclasses.
>
> Please also review the corresponding CSR
> https://bugs.openjdk.java.net/browse/JDK-8283434
Hmm.. I tried the following example:
$ cat com/acme/X.java
package com.acme;
// We can omit permits clause if all the subclasses are in the same compilation
unit.
// But that's applicable here as two subclasses "Z", "P.Q" are outside the
compilation unit.
// So we use explicit permits clause that lists all the subclasses.
public sealed class X
permits X.Y, W, Z, P.Q {
final class Y extends X {}
}
final class W extends X {}
$ cat com/acme/Z.java
package com.acme;
final class Z extends X {
}
$ cat com/acme/P.java
package com.acme;
class P {
final class Q extends X {}
}
$ javac com/acme/*.java
is fine. Am I missing something?
-
PR: https://git.openjdk.java.net/jdk/pull/7881
Re: RFR: JDK-8283416: Update java.lang.invoke.MethodHandle to use sealed classes
On Sun, 20 Mar 2022 21:31:15 GMT, Joe Darcy wrote:
> Small refactoring to use sealed classes in the MethodHandle implementation
> hierarchy.
>
> DelegatingMethodHandle is non-sealed rather than sealed since it has two
> subclasses, one defined as a nested class and only a separate type in the
> same package. The permits clause does not allow listed those two kinds of
> subclasses.
>
> Please also review the corresponding CSR
> https://bugs.openjdk.java.net/browse/JDK-8283434
"DelegatingMethodHandle is non-sealed rather than sealed since it has two
subclasses, one defined as a nested class and only a separate type in the same
package. The permits clause does not allow listed those two kinds of
subclasses."
We can have a permits clause that lists fully qualified class name for the
nested class and simple name for that separate type in the same package, right?
Also
$ cd $jdk_src/src/java.base/share/classes/java/lang/invoke
$ grep "extends DelegatingMethodHandle" *
MethodHandleImpl.java:private static final class AsVarargsCollector extends
DelegatingMethodHandle {
MethodHandleImpl.java:static class CountingWrapper extends
DelegatingMethodHandle {
MethodHandleImpl.java:private static final class WrappedMember extends
DelegatingMethodHandle {
MethodHandleImpl.java:static final class IntrinsicMethodHandle extends
DelegatingMethodHandle
All 4 are nested classes. Two of those subclasses are nested "private" in
another class and hence cannot be referred from DelegatingMethodHandle's
permits clause, right?
-
PR: https://git.openjdk.java.net/jdk/pull/7881
Re: RFR: 8282572: EnumSet should be a sealed class
On Tue, 8 Mar 2022 10:50:35 GMT, Jaikiran Pai wrote: > Can I please get a review of this change which proposes to implement the > enhancement requested in https://bugs.openjdk.java.net/browse/JDK-8282572? > > The (public) `EnumSet` class has 2 package private (JDK) internal sub-classes > - the `JumboEnumSet` and `RegularEnumSet`. These 2 classes don't have any > sub-classes of their own. > > In this commit, the `EnumSet` is marked as `sealed` and the `JumboEnumSet` > and `RegularEnumSet` are the two permitted sub classes. Both of these > sub-classes are now marked as `final` too. Usage of `non-sealed` modifier for > these 2 sub-classes was an option too. But I decided to start with the more > restrictive option since we don't have any other sub-classes and if and when > we do have their sub-classes, it's possible to change them to `non-sealed`. > > The `EnumSet` class implements the `java.io.Serializable` interface. As part > of this change, manual tests have been run to make sure that changing this > class to `sealed` and marking the sub-classes as `final` don't break any > serialization/deserialization semantics, across Java version and/or user > application versions. > > `tier1` testing across various platforms is currently in progress. LGTM - Marked as reviewed by sundar (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7741
Re: RFR: 8282572: EnumSet should be a sealed class
On Tue, 8 Mar 2022 10:50:35 GMT, Jaikiran Pai wrote: > Can I please get a review of this change which proposes to implement the > enhancement requested in https://bugs.openjdk.java.net/browse/JDK-8282572? > > The (public) `EnumSet` class has 2 package private (JDK) internal sub-classes > - the `JumboEnumSet` and `RegularEnumSet`. These 2 classes don't have any > sub-classes of their own. > > In this commit, the `EnumSet` is marked as `sealed` and the `JumboEnumSet` > and `RegularEnumSet` are the two permitted sub classes. Both of these > sub-classes are now marked as `final` too. Usage of `non-sealed` modifier for > these 2 sub-classes was an option too. But I decided to start with the more > restrictive option since we don't have any other sub-classes and if and when > we do have their sub-classes, it's possible to change them to `non-sealed`. > > The `EnumSet` class implements the `java.io.Serializable` interface. As part > of this change, manual tests have been run to make sure that changing this > class to `sealed` and marking the sub-classes as `final` don't break any > serialization/deserialization semantics, across Java version and/or user > application versions. > > `tier1` testing across various platforms is currently in progress. Do we need a CSR as a public class is changed from non-sealed to sealed? (Although the constructor has been package-private and hence it will not affect any non-dk code as noted by Jaikiran via private conversation. Only reflection will be able to see the difference of non-sealed to sealed change). - PR: https://git.openjdk.java.net/jdk/pull/7741
Re: RFR: 8268250: Class.arrayType() for a 255-d array throws undocumented IllegalArgumentException [v4]
On Thu, 17 Feb 2022 05:11:40 GMT, Joe Darcy wrote: >> Make explicit illegal argument cases of Class.arrayType. >> >> Please also review the corresponding CSR: >> https://bugs.openjdk.java.net/browse/JDK-8268300 > > Joe Darcy has updated the pull request incrementally with one additional > commit since the last revision: > > Respond to review feedback. Marked as reviewed by sundar (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4382
Re: RFR: 8268250: Class.arrayType() for a 255-d array throws undocumented IllegalArgumentException [v3]
On Thu, 17 Feb 2022 02:56:53 GMT, Joe Darcy wrote: >> Make explicit illegal argument cases of Class.arrayType. >> >> Please also review the corresponding CSR: >> https://bugs.openjdk.java.net/browse/JDK-8268300 > > Joe Darcy has updated the pull request with a new target base due to a merge > or a rebase. The incremental webrev excludes the unrelated changes brought in > by the merge/rebase. The pull request contains six additional commits since > the last revision: > > - Revisit and update fix to address review feedback. > - Merge branch 'master' into 8268250 > - Add missing ending newline. > - Add test file. > - Merge branch 'master' into 8268250 > - 8268250: Class.arrayType() for a 255-d array throws undocumented > IllegalArgumentException LGTM - Marked as reviewed by sundar (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/4382
Re: RFR: 8268250: Class.arrayType() for a 255-d array throws undocumented IllegalArgumentException [v3]
On Thu, 17 Feb 2022 02:56:53 GMT, Joe Darcy wrote: >> Make explicit illegal argument cases of Class.arrayType. >> >> Please also review the corresponding CSR: >> https://bugs.openjdk.java.net/browse/JDK-8268300 > > Joe Darcy has updated the pull request with a new target base due to a merge > or a rebase. The incremental webrev excludes the unrelated changes brought in > by the merge/rebase. The pull request contains six additional commits since > the last revision: > > - Revisit and update fix to address review feedback. > - Merge branch 'master' into 8268250 > - Add missing ending newline. > - Add test file. > - Merge branch 'master' into 8268250 > - 8268250: Class.arrayType() for a 255-d array throws undocumented > IllegalArgumentException test/jdk/java/lang/Class/ArrayType.java line 2: > 1: /* > 2: * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved. 2022? - PR: https://git.openjdk.java.net/jdk/pull/4382
Re: RFR: 8281335: Allow a library already loaded via System::loadLibrary to be loaded as a raw library [v2]
On Fri, 11 Feb 2022 03:49:45 GMT, Mandy Chung wrote: >> This patch removes the restriction in the raw library loading mechanism that >> does not allow mix-n-match of loading a library as a JNI library and as a >> raw library. >> >> The raw library loading mechanism is designed for panama to load native >> library essentially equivalent to dlopen/dlclose calls independent of JNI >> library loading. If a native library is loaded as a JNI library and a raw >> library, it will get different NativeLibrary instances. When a class loader >> is being unloaded, JNI_Unload will be invoked but the native library may not >> be unloaded until NativeLibrary::unload is explicitly called for the raw >> library. > > Mandy Chung has updated the pull request incrementally with one additional > commit since the last revision: > > review comment Marked as reviewed by sundar (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7435
Re: RFR: 8281335: Allow a library already loaded via System::loadLibrary to be loaded as a raw library
On Thu, 10 Feb 2022 23:27:49 GMT, Mandy Chung wrote:
> This patch removes the restriction in the raw library loading mechanism that
> does not allow mix-n-match of loading a library as a JNI library and as a raw
> library.
>
> The raw library loading mechanism is designed for panama to load native
> library essentially equivalent to dlopen/dlclose calls independent of JNI
> library loading. If a native library is loaded as a JNI library and a raw
> library, it will get different NativeLibrary instances. When a class loader
> is being unloaded, JNI_Unload will be invoked but the native library may not
> be unloaded until NativeLibrary::unload is explicitly called for the raw
> library.
LGTM
src/java.base/share/classes/jdk/internal/loader/NativeLibraries.java line 58:
> 56: * will fail.
> 57: */
> 58: public abstract class NativeLibraries {
could this be sealed with only two specific subtypes defined here?
src/java.base/share/classes/jdk/internal/loader/NativeLibraries.java line 251:
> 249: // do not search java.library.path
> 250: super(loader, loader != null ? null : NativeLibraries.class,
> 251: loader != null ? true : false);
The last argument expression of the super class could be just loader != null
-
Marked as reviewed by sundar (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/7435
Re: [jdk18] RFR: 8278574: update --help-extra message to include default value of --finalization option
On Thu, 16 Dec 2021 06:33:24 GMT, Stuart Marks wrote: > A small modification to the Launcher's help text. LGTM - Marked as reviewed by sundar (Reviewer). PR: https://git.openjdk.java.net/jdk18/pull/34
Re: [jdk18] RFR: 8278607: Misc issues in foreign API javadoc [v4]
On Tue, 14 Dec 2021 10:24:07 GMT, Maurizio Cimadamore wrote: >> This patch fixes a number of issues and typos in the foreign API javadoc >> following another internal round of reviews. > > Maurizio Cimadamore has updated the pull request incrementally with one > additional commit since the last revision: > > Fix typo as per review comment. Improve javadoc for VaList. LGTM - Marked as reviewed by sundar (Reviewer). PR: https://git.openjdk.java.net/jdk18/pull/17
Re: [jdk18] RFR: 8278607: Misc issues in foreign API javadoc [v3]
On Tue, 14 Dec 2021 02:07:00 GMT, Maurizio Cimadamore wrote: >> This patch fixes a number of issues and typos in the foreign API javadoc >> following another internal round of reviews. > > Maurizio Cimadamore has updated the pull request incrementally with one > additional commit since the last revision: > > Clarify sentence in package-info LGTM - Marked as reviewed by sundar (Reviewer). PR: https://git.openjdk.java.net/jdk18/pull/17
Integrated: 8278205: jlink plugins should dump .class file in debug mode
On Fri, 3 Dec 2021 10:03:30 GMT, Athijegannathan Sundararajan wrote: > jlink plugins dump .class files in debug mode. jpackage tests already set > jlink.debug property to true. This pull request has now been integrated. Changeset: ba2a8e5a Author:Athijegannathan Sundararajan URL: https://git.openjdk.java.net/jdk/commit/ba2a8e5a496799451095362279b9dd4b6df20b67 Stats: 41 lines in 5 files changed: 35 ins; 0 del; 6 mod 8278205: jlink plugins should dump .class file in debug mode Reviewed-by: jlaskey - PR: https://git.openjdk.java.net/jdk/pull/6696
RFR: 8278205: jlink plugins should dump .class file in debug mode
jlink plugins dump .class files in debug mode. jpackage tests already set jlink.debug property to true. - Commit messages: - 8278205: jlink plugins should dump .class file in debug mode Changes: https://git.openjdk.java.net/jdk/pull/6696/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6696=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8278205 Stats: 41 lines in 5 files changed: 35 ins; 0 del; 6 mod Patch: https://git.openjdk.java.net/jdk/pull/6696.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6696/head:pull/6696 PR: https://git.openjdk.java.net/jdk/pull/6696
Re: RFR: 8278145: Javadoc for MemorySegment::set/MemorySegment::setAtIndex is missing throws tag
On Thu, 2 Dec 2021 12:26:33 GMT, Maurizio Cimadamore wrote: > This patch adds missing `@throws` tags to the javadoc of `MemorySegment::set` > and `MemorySegment::setAtIndex`. > These methods can fail with `UnsupportedOperationException` if the segment is > read-only. LGTM - Marked as reviewed by sundar (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6668
Re: RFR: 8277647: [REDO] JDK-8277507 Add jlink.debug system property while launching jpackage tests to help diagonize recent intermittent failures
On Wed, 24 Nov 2021 08:54:08 GMT, Jaikiran Pai wrote: > Can I please get a review of this change which adds `jlink.debug=true` system > property while launching `jpackage` tests? > > The previous fix for this in https://github.com/openjdk/jdk/pull/6491 didn't > take into account the part where the `jpackage` tool gets launched as a > `ToolProvider` from some of these tests. This resulted in a large number of > tests failing (across different OS) in `tier2` with errors like: > > > Error: Invalid Option: [-J-Djlink.debug=true] > > > In this current PR, the changed code now takes into account the possibility > of `jpackage` being launched as a `ToolProvider` and in such cases doesn't > add this option. To achieve this, the adding of this argument is delayed till > when the actual execution is about to happen and thus it's now done in the > `adjustArgumentsBeforeExecution()` method of the jpackage test framework. > > With this change, I have now run the `jdk:tier2` locally on a macos instance > and the tests have all passed: > > > Test results: passed: 3,821; failed: 3 > Report written to > jdk/build/macosx-x86_64-server-release/test-results/jtreg_test_jdk_tier2/html/report.html > Results written to > jdk/build/macosx-x86_64-server-release/test-support/jtreg_test_jdk_tier2 > Error: Some tests failed or other problems occurred. > Finished running test 'jtreg:test/jdk:tier2' > Test report is stored in > build/macosx-x86_64-server-release/test-results/jtreg_test_jdk_tier2 > > == > Test summary > == >TEST TOTAL PASS FAIL ERROR > >>> jtreg:test/jdk:tier2 3824 3821 3 0 << > == > > The 3 failing tests are unrelated to this change and belong to the > `java/nio/channels/DatagramChannel/` test package. > Furthermore, I've looked into the generated logs of the following tests to > verify that the `-J-Djlink.debug=true` does get passed in relevant tests and > doesn't in those that failed previously in `tier2`: > > test/jdk/tools/jpackage/share/MultiLauncherTwoPhaseTest.java > test/jdk/tools/jpackage/macosx/NameWithSpaceTest.java > test/jdk/tools/jpackage/share/ArgumentsTest.java > > A sample from one of the logs where this system property is expected to be > passed along: > >> TRACE: exec: Execute >> [jdk/build/macosx-x86_64-server-release/images/jdk/bin/jpackage --input >> ./test/input --dest ./test/output --name "Name With Space" --type pkg >> --main-jar hello.jar --main-class Hello -J-Djlink.debug=true --verbose](15); >> inherit I/O... > > > I would still like to request someone with access to CI or other OSes (like > Windows and Linux) to help test `tier2` against this PR. LGTM - Marked as reviewed by sundar (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6534
Re: RFR: 8273682: Upgrade Jline to 3.20.0
On Thu, 23 Sep 2021 14:43:21 GMT, Jan Lahoda wrote: > I'd like to upgrade the internal JLine to 3.20.0, to support the rxvt > terminal (see JDK-8270943), and to generally use a newer version of the > library. This patch is basically a application of relevant parts of the diff > between JLine 3.14.0 and 3.20.0, with merge fixes as needed. > > Thanks! LGTM src/jdk.internal.le/share/classes/jdk/internal/org/jline/reader/CompletionMatcher.java line 32: > 30: * > 31: * @param candidates list of candidates > 32: * @return a map of candidates that completion matcher matches a list of ..? src/jdk.internal.le/share/classes/jdk/internal/org/jline/reader/EndOfFileException.java line 2: > 1: /* > 2: * Copyright (c) 2002-2020, the original author or authors. 2021 - Marked as reviewed by sundar (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5655
Re: [jdk17] RFR: JDK-8249646: Runtime.exec(String, String[], File) documentation contains literal {@link ...}
On Tue, 29 Jun 2021 04:39:28 GMT, Jonathan Gibbons wrote: > Please review a trivial `noreg-doc` fix for some javadoc tags for > `java.lang.Runtime` for JDK17 LGTM - Marked as reviewed by sundar (Reviewer). PR: https://git.openjdk.java.net/jdk17/pull/167
Integrated: 8240349: jlink should not leave partial image output directory on failure
On Mon, 7 Jun 2021 11:00:00 GMT, Athijegannathan Sundararajan wrote: > jlink should clean up output directory on any failure. should not leave > partially filled output. This pull request has now been integrated. Changeset: 4d1cf51b Author:Athijegannathan Sundararajan URL: https://git.openjdk.java.net/jdk/commit/4d1cf51b1d4a5e812c9f78b0104e40fbc4883a6c Stats: 71 lines in 5 files changed: 66 ins; 0 del; 5 mod 8240349: jlink should not leave partial image output directory on failure Reviewed-by: jlaskey, alanb - PR: https://git.openjdk.java.net/jdk/pull/4386
Re: RFR: 8240349: jlink should not leave partial image output directory on failure [v2]
> jlink should clean up output directory on any failure. should not leave > partially filled output. Athijegannathan Sundararajan has updated the pull request incrementally with one additional commit since the last revision: Fixed resouce closing issue for lib/moduls file. Pre-existing output directory should not be deleted when exiting. - Changes: - all: https://git.openjdk.java.net/jdk/pull/4386/files - new: https://git.openjdk.java.net/jdk/pull/4386/files/f12e6a29..df036a3a Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4386=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4386=00-01 Stats: 48 lines in 5 files changed: 22 ins; 12 del; 14 mod Patch: https://git.openjdk.java.net/jdk/pull/4386.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4386/head:pull/4386 PR: https://git.openjdk.java.net/jdk/pull/4386
Re: RFR: 8240349: jlink --vm with not present VM does not fail fast
On Tue, 8 Jun 2021 11:32:29 GMT, Alan Bateman wrote:
>> jlink should clean up output directory on any failure. should not leave
>> partially filled output.
>
> src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JlinkTask.java line 295:
>
>> 293: cleanupOutput(outputPath);
>> 294: return EXIT_ERROR;
>> 295: } catch (BadArgs e) {
>
> Can you confirm that this change is benign for the case that the output
> directory exists? I believe it's a BadArgs case so it won't attempt to delete
> a file or tree if it exists but I'd like confirmation.
Will check that.
> src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JlinkTask.java line 490:
>
>> 488:
>> 489: private static void deleteDirectory(Path dir) throws IOException {
>> 490: if (dir != null && Files.isDirectory(dir)) {
>
> I don't think this method should be checking dir, that should be up to the
> caller to ensure that it is not called when output is null.
okay. will move that check to caller.
-
PR: https://git.openjdk.java.net/jdk/pull/4386
Re: RFR: 8240349: jlink --vm with not present VM does not fail fast
On Tue, 8 Jun 2021 12:20:32 GMT, Jorn Vernee wrote:
> WRT the test failure on Windows discussed offline: when the directory is
> deleted as a result of a `PluginException` being thrown, there is still an
> open file handle on the `lib/modules` file in the image directory, which
> prevents the directory from being deleted.
>
> Bisecting this, it seems that the file handle is being created in
> `ImageFileCreater::writeImage` with a call to
> `plugins.getJImageFileOutputStream`
> (https://github.com/openjdk/jdk/blob/master/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ImageFileCreator.java#L162).
> This creates an output stream that is never closed.
>
> Following patch fixes:
>
> ```
> diff --git
> a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ImageFileCreator.java
> b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ImageFileCreator.java
> index 749025bea9d..8beddc5a037 100644
> ---
> a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ImageFileCreator.java
> +++
> b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ImageFileCreator.java
> @@ -158,8 +158,10 @@ public final class ImageFileCreator {
> BasicImageWriter writer = new BasicImageWriter(byteOrder);
> ResourcePoolManager allContent = createPoolManager(archives,
> entriesForModule, byteOrder, writer);
> -ResourcePool result = generateJImage(allContent,
> - writer, plugins, plugins.getJImageFileOutputStream());
> +ResourcePool result;
> +try (DataOutputStream out = plugins.getJImageFileOutputStream()) {
> +result = generateJImage(allContent, writer, plugins, out);
> +}
>
> //Handle files.
> try {
> ```
Thanks @JornVernee
-
PR: https://git.openjdk.java.net/jdk/pull/4386
Re: RFR: 8268327: Upstream: 8268169: The system lookup can not find stdio functions such as printf on Windows 10
On Mon, 7 Jun 2021 13:23:46 GMT, Jorn Vernee wrote: > Hi, > > The documentation of `CLinker::systemLookup` [1] says this: > > > * Obtains a system lookup which is suitable to find symbols in the standard C > libraries. > > > However, currently it is not possible to look up common stdio.h symbols, such > as `printf`, using the system lookup on Windows 10. This is because the > backing library that is loaded, namely `ucrtbase.dll`, does not contain these > symbols, as they are implemented in the standard library as inline functions. > > This patch changes the system lookup implementation on Windows 10 to make > these symbols findable as well, by using a fallback library that forces the > generation of the code for the inline functions, and exposes the missing > symbols as a table of function pointers. > > See also the prior review of this patch for the panama-foreign repo: > https://github.com/openjdk/panama-foreign/pull/547 > > Since the exact set of symbols findable by the system lookup is unspecified, > and since the API in question (`CLinker::systemLookup`) was added only last > week [2], I've elected to not create a CSR for this patch, but please let me > know if one is needed). > > Thanks, > Jorn > > [1] : > https://github.com/openjdk/jdk/blob/master/src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/CLinker.java#L134-L151 > [2] : > https://github.com/openjdk/jdk/commit/a223189b069a7cfe49511d49b5b09e7107cb3cab LGTM - Marked as reviewed by sundar (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/4390
RFR: 8240349: jlink --vm with not present VM does not fail fast
jlink should clean up output directory on any failure. should not leave partially filled output. - Commit messages: - 8240349: jlink --vm with not present VM does not fail fast Changes: https://git.openjdk.java.net/jdk/pull/4386/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4386=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8240349 Stats: 56 lines in 2 files changed: 56 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/4386.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4386/head:pull/4386 PR: https://git.openjdk.java.net/jdk/pull/4386
Integrated: 8240347: remove undocumented options from jlink --help message
On Thu, 27 May 2021 09:23:50 GMT, Athijegannathan Sundararajan wrote: > fixed constructor argument passing issue. This pull request has now been integrated. Changeset: ec65cf83 Author: Athijegannathan Sundararajan URL: https://git.openjdk.java.net/jdk/commit/ec65cf833294e21e9dc59dfe014148d3e1210b53 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod 8240347: remove undocumented options from jlink --help message Reviewed-by: alanb, redestad - PR: https://git.openjdk.java.net/jdk/pull/4221
RFR: 8240347: remove undocumented options from jlink --help message
fixed constructor argument passing issue. - Commit messages: - 8240347: remove undocumented options from jlink --help message Changes: https://git.openjdk.java.net/jdk/pull/4221/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4221=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8240347 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/4221.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4221/head:pull/4221 PR: https://git.openjdk.java.net/jdk/pull/4221
Integrated: 8267583: jmod fails on symlink to class file
On Wed, 26 May 2021 07:55:53 GMT, Athijegannathan Sundararajan wrote: > FileVisitOption.FOLLOW_LINKS used. Test extended for file symlinks case. This pull request has now been integrated. Changeset: bf8d4a8e Author: Athijegannathan Sundararajan URL: https://git.openjdk.java.net/jdk/commit/bf8d4a8ecab216e7d117ce045d4498d1fa1a6029 Stats: 41 lines in 2 files changed: 39 ins; 0 del; 2 mod 8267583: jmod fails on symlink to class file Reviewed-by: alanb - PR: https://git.openjdk.java.net/jdk/pull/4202
RFR: 8267583: jmod fails on symlink to class file
FileVisitOption.FOLLOW_LINKS used. Test extended for file symlinks case. - Commit messages: - 8267583: jmod fails on symlink to class file Changes: https://git.openjdk.java.net/jdk/pull/4202/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4202=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8267583 Stats: 41 lines in 2 files changed: 39 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/4202.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4202/head:pull/4202 PR: https://git.openjdk.java.net/jdk/pull/4202
Integrated: 8266291: (jrtfs) Calling Files.exists may break the JRT filesystem
On Fri, 14 May 2021 05:39:10 GMT, Athijegannathan Sundararajan wrote: > Problematic paths that start with /modules/modules prefix are handled properly This pull request has now been integrated. Changeset: af4cd04c Author: Athijegannathan Sundararajan URL: https://git.openjdk.java.net/jdk/commit/af4cd04c2e393f8d1ffef60f49e3269adee649b8 Stats: 35 lines in 2 files changed: 31 ins; 0 del; 4 mod 8266291: (jrtfs) Calling Files.exists may break the JRT filesystem Reviewed-by: redestad, alanb - PR: https://git.openjdk.java.net/jdk/pull/4022
Re: RFR: 8266291: (jrtfs) Calling Files.exists may break the JRT filesystem [v2]
> Problematic paths that start with /modules/modules prefix are handled properly Athijegannathan Sundararajan has updated the pull request incrementally with one additional commit since the last revision: edited comment as per code review. - Changes: - all: https://git.openjdk.java.net/jdk/pull/4022/files - new: https://git.openjdk.java.net/jdk/pull/4022/files/ba288d93..92611cbb Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4022=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4022=00-01 Stats: 1 line in 1 file changed: 0 ins; 1 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/4022.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4022/head:pull/4022 PR: https://git.openjdk.java.net/jdk/pull/4022
RFR: 8266291: (jrtfs) Calling Files.exists may break the JRT filesystem
Problematic paths that start with /modules/modules prefix are handled properly - Commit messages: - fixed typos in comments - 8266291: (jrtfs) Calling Files.exists may break the JRT filesystem Changes: https://git.openjdk.java.net/jdk/pull/4022/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4022=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8266291 Stats: 36 lines in 2 files changed: 32 ins; 0 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/4022.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4022/head:pull/4022 PR: https://git.openjdk.java.net/jdk/pull/4022
Integrated: 8260621: (jrtfs) ThreadLocal memory leak in ImageBufferCache when using jrtfs
On Tue, 4 May 2021 09:05:38 GMT, Athijegannathan Sundararajan wrote: > Instead of BufferReference class, Map.Entry is used as pair implementation. > This avoids the metaspace leak seen via thread local. This pull request has now been integrated. Changeset: c9873c41 Author:Athijegannathan Sundararajan URL: https://git.openjdk.java.net/jdk/commit/c9873c416d047ec97c12f77abad3ece407530063 Stats: 57 lines in 1 file changed: 30 ins; 7 del; 20 mod 8260621: (jrtfs) ThreadLocal memory leak in ImageBufferCache when using jrtfs Reviewed-by: jlaskey, vtewari - PR: https://git.openjdk.java.net/jdk/pull/3849
RFR: 8260621: (jrtfs) ThreadLocal memory leak in ImageBufferCache when using jrtfs
Instead of BufferReference class, Map.Entry is used as pair implementation. This avoids the metaspace leak seen via thread local. - Commit messages: - added comment. generics cleanup. - 8260621: (jrtfs) ThreadLocal memory leak in ImageBufferCache when using jrtfs Changes: https://git.openjdk.java.net/jdk/pull/3849/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3849=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8260621 Stats: 57 lines in 1 file changed: 30 ins; 7 del; 20 mod Patch: https://git.openjdk.java.net/jdk/pull/3849.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3849/head:pull/3849 PR: https://git.openjdk.java.net/jdk/pull/3849
Re: RFR: 8264326: Modernize javax.script.ScriptEngineManager and related classes' implementation [v2]
On Sun, 28 Mar 2021 13:38:51 GMT, Attila Szegedi wrote: >> I noticed that `javax.script.ScriptEngineManager` `getEngineByXxx` methods >> had a lot of code duplication among themselves, and even within each method. >> I refactored them into a modern unified implementation. While there I also >> took the opportunity to introduce `Objects.requireNonNull` in place of null >> checks followed by NPE throws, mark private fields final where possible, use >> lambdas for `doPrivileged` block, use `List.of` and `List.copyOf` where >> possible, and generally sanitize/deduplicate. > > Attila Szegedi 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. The pull request contains 13 new > commits since the last revision: > > - Tidy > - require non null in SimpleBindings > - Simplify SimpleScriptContext.scopes > - Mark fields final where possible > - Deduplicate registerEngineXxx methods > - Misc tidying > - Deduplicate exception reporting > - Lambdify > - Mark fields as final; eliminate now unnecessary init() method. > - Deduplicate engine creation and setup code > - ... and 3 more: > https://git.openjdk.java.net/jdk/compare/f378f350...56d89eb2 Marked as reviewed by sundar (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3229
Re: RFR: 8262503: Support records in Dynalink
On Sun, 28 Feb 2021 16:46:31 GMT, Attila Szegedi wrote: > 8262503: Support records in Dynalink Looks good - Marked as reviewed by sundar (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/2767
Re: RFR: 8260337: Optimize ImageReader lookup, used by Class.getResource [v2]
On Mon, 25 Jan 2021 16:09:01 GMT, Claes Redestad wrote:
>> This patch optimizes the code paths exercised by
>> `String.class.getResource("String.class")` by:
>>
>> - Adding an ASCII fast-path to methods verifying strings in the jimage,
>> which can then be done allocation-free
>> - Avoiding the allocation of the `long[8]` attributes when verifying only
>> for the purpose of verifying a path exists
>> - Using the `JNUA.create` fast-path in `SystemModuleReader` (which should be
>> OK since we just verified the given name is a JRT path)
>> - Remove a redundant check in `Class::resolveName` and fitting the
>> `StringBuilder` to size
>
> Claes Redestad has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Copyrights and rename containsLocation
LGTM
-
Marked as reviewed by sundar (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/2212
Integrated: 8259814: test/jdk/tools/jlink/plugins/CompressorPluginTest.java has compilation issues
On Fri, 15 Jan 2021 05:19:44 GMT, Athijegannathan Sundararajan wrote: > Fixed compilation issues with the test. Test compiles and runs fine locally. This pull request has now been integrated. Changeset: c0cde7dc Author: Athijegannathan Sundararajan URL: https://git.openjdk.java.net/jdk/commit/c0cde7dc Stats: 17 lines in 2 files changed: 5 ins; 1 del; 11 mod 8259814: test/jdk/tools/jlink/plugins/CompressorPluginTest.java has compilation issues Reviewed-by: alanb - PR: https://git.openjdk.java.net/jdk/pull/2091
Re: RFR: 8259814: test/jdk/tools/jlink/plugins/CompressorPluginTest.java has compilation issues [v2]
> Fixed compilation issues with the test. Test compiles and runs fine locally. Athijegannathan Sundararajan has updated the pull request incrementally with one additional commit since the last revision: Removed CompressorPluginTest.java from ProblemList.txt - Changes: - all: https://git.openjdk.java.net/jdk/pull/2091/files - new: https://git.openjdk.java.net/jdk/pull/2091/files/dfa440cd..6856cc68 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2091=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2091=00-01 Stats: 1 line in 1 file changed: 0 ins; 1 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/2091.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2091/head:pull/2091 PR: https://git.openjdk.java.net/jdk/pull/2091
RFR: 8259814: test/jdk/tools/jlink/plugins/CompressorPluginTest.java has compilation issues
Fixed compilation issues with the test. Test compiles and runs fine locally. - Commit messages: - 8259814: test/jdk/tools/jlink/plugins/CompressorPluginTest.java has compilation issues Changes: https://git.openjdk.java.net/jdk/pull/2091/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2091=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8259814 Stats: 16 lines in 1 file changed: 5 ins; 0 del; 11 mod Patch: https://git.openjdk.java.net/jdk/pull/2091.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2091/head:pull/2091 PR: https://git.openjdk.java.net/jdk/pull/2091
Re: [jdk16] RFR: 8259636: Check for buffer backed by shared segment kicks in in unexpected places
On Tue, 12 Jan 2021 15:48:18 GMT, Maurizio Cimadamore wrote: > When support for shared segment was added, we decided to disable support for > buffers derived from shared segment in certain async operations, as there's > currently no way to make sure that the memory won't be reclaimed while the IO > operation is still taking place. > > After looking at the code, it seemed like the best place to put the > restriction would be sun.nio.ch.DirectBuffer::address() method, since this > method is used in a lot of places just before jumping into some piece of JNI > code. > > While I still stand by that decision, the Netty team has discovered that this > decision also affected operations such as creating slices from byte buffers > derived from shared segment - this is caused by the fact that one direct > buffer constructor (the one for views and slices) is calling the dreaded > DirectBuffer::address method. > > The fix is simple: just avoid the method call - which is very easy to do in > the case of the buffer constructor: in fact this method just returns the > value of the `address` field inside the `Buffer` class, so we can always cast > to `Buffer` and then access `address` field from there. Marked as reviewed by sundar (Reviewer). - PR: https://git.openjdk.java.net/jdk16/pull/110
Integrated: 8242258: (jrtfs) Path::toUri throws AssertionError for malformed input
On Mon, 7 Dec 2020 16:35:52 GMT, Athijegannathan Sundararajan wrote: > Safe URI encode logic adopted from UnixUriUtils. This pull request has now been integrated. Changeset: d2b66196 Author: Athijegannathan Sundararajan URL: https://git.openjdk.java.net/jdk/commit/d2b66196 Stats: 218 lines in 2 files changed: 206 ins; 5 del; 7 mod 8242258: (jrtfs) Path::toUri throws AssertionError for malformed input Reviewed-by: alanb - PR: https://git.openjdk.java.net/jdk/pull/1669
Re: RFR: 8242258: (jrtfs) Path::toUri throws AssertionError for malformed input [v2]
On Mon, 7 Dec 2020 19:59:40 GMT, Alan Bateman wrote:
>> Athijegannathan Sundararajan has updated the pull request incrementally with
>> one additional commit since the last revision:
>>
>> renamed the test as per review comment.
>
> test/jdk/jdk/internal/jrtfs/Test8242258.java line 40:
>
>> 38: import static org.testng.Assert.assertEquals;
>> 39:
>> 40: public class Test8242258 {
>
> I think it would be better to create something like UriTests that we can add
> further tests for jrtfs URIs as they arise.
I'll renamed the test
> test/jdk/jdk/internal/jrtfs/Test8242258.java line 60:
>
>> 58: { "xyz ", "jrt:/xyz%20" },
>> 59: { "xy z", "jrt:/xy%20z" },
>> 60: };
>
> One other thing we test here is a malformed escape pair, e.g. "jrt:/%5" and
> check that getPath(URI) throws IAE.
URI.create(String) method itself checks for malformed escape pairs. Do we need
anything additional here?
-
PR: https://git.openjdk.java.net/jdk/pull/1669
Re: RFR: 8242258: (jrtfs) Path::toUri throws AssertionError for malformed input [v2]
> Safe URI encode logic adopted from UnixUriUtils. Athijegannathan Sundararajan has updated the pull request incrementally with one additional commit since the last revision: renamed the test as per review comment. - Changes: - all: https://git.openjdk.java.net/jdk/pull/1669/files - new: https://git.openjdk.java.net/jdk/pull/1669/files/b3d0a927..4d7417f8 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=1669=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1669=00-01 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/1669.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1669/head:pull/1669 PR: https://git.openjdk.java.net/jdk/pull/1669
RFR: 8242258: (jrtfs) Path::toUri throws AssertionError for malformed input
Safe URI encode logic adopted from UnixUriUtils. - Commit messages: - 8242258: (jrtfs) Path::toUri throws AssertionError for malformed input Changes: https://git.openjdk.java.net/jdk/pull/1669/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1669=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8242258 Stats: 218 lines in 2 files changed: 206 ins; 5 del; 7 mod Patch: https://git.openjdk.java.net/jdk/pull/1669.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1669/head:pull/1669 PR: https://git.openjdk.java.net/jdk/pull/1669
Re: RFR: 8255845: Memory leak in imageFile.cpp
On Thu, 3 Dec 2020 14:57:16 GMT, Evan Whelan wrote: > This is a straightforward fix to resolve a potential memory leak in > imageFile.cpp. > > If `find_location` returns true, the `path` char array is never released. > > This has been fixed Marked as reviewed by sundar (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/1599
Re: RFR: 8256862: Several java/foreign tests fail on x86_32 platforms [v2]
On Wed, 25 Nov 2020 22:00:13 GMT, Jorn Vernee wrote: >> This patch fixes several failures on x86_32 of java/foreign tests. >> >> This is mostly done by disabling the failing tests, but the impl of CLinker >> is also adjusted ton properly detect 32 bit platforms as unsupported. >> >> CLinker is specified to fail in the initializer on unsupported platforms, >> and a test is added to verify this as well. >> >> Note that this adds requires clauses to the failing tests that explicitly >> enumerate all available platforms, so this should fix the test failures on >> other platforms as well. >> >> https://bugs.openjdk.java.net/browse/JDK-8256757 is filed for the remaining >> failure in TestStringLatin1IndexOfChar. >> >> CSR link: https://bugs.openjdk.java.net/browse/JDK-8256863 > > Jorn Vernee has updated the pull request incrementally with one additional > commit since the last revision: > > Exclude TestNulls as well Marked as reviewed by sundar (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/1386
Re: RFR: 8256865: Foreign Memory Access and Linker API are missing NPE checks [v2]
On Mon, 23 Nov 2020 18:22:14 GMT, Maurizio Cimadamore wrote: >> Both the Foreign Memory Access and the Foreign Linker APIs leave something >> to be desired when it comes to handling NPEs - first, most of the API >> javadoc is oblivious to NPEs being thrown. Secondly, not all API method >> implementations add expicit NPE checks - with the result of NPE often being >> thrown very deep in the call chain - if at all. Third, test for API coverage >> of nulls is ad-hoc. >> >> This patch rectifies all these issues. To increase coverage for null >> injected into APIs, this patch introduces a new framework for testing an API >> in bulk, so that all methods are reflectively called with some values >> replaced with nulls, so that all combinations are tried. >> >> I've also added, as part of this patch, a test to cover the statics in >> MemoryAccess which were not covered throughly. > > Maurizio Cimadamore has updated the pull request incrementally with one > additional commit since the last revision: > > Fix cut/paste error in FunctionDescriptor Unused imports in TestLayouts.java? - Marked as reviewed by sundar (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/1388
Re: RFR: 8256477: Specialize heap memory segment implementations [v2]
On Wed, 18 Nov 2020 10:08:20 GMT, Maurizio Cimadamore wrote: >> The current memory segment implementation defines a hierarchy with 3 >> concrete classes: one for heap segments, one for native segments and one for >> mapped segments. >> >> Since there can be many kinds of heap segments (e.g. created from a byte[] >> or from a float[]) the current implementation is prone to type profile >> pollution problems: if enough heap segments are created (of different >> kinds), the JIT compiler will give up on speculating on the heap segment >> kind, which will then result in poor performances. >> >> This issue can be reproduced in one of the existing benchmark, by adding >> some initialization code which is enough to pollute the types profiles. When >> that happens, performance numbers look like the following: >> >> Benchmark (polluteProfile) Mode Cnt Score >> Error Units >> LoopOverNonConstantHeap.segment_loop false avgt 10 0.285 ± >> 0.003 ms/op >> LoopOverNonConstantHeap.segment_loop true avgt 10 5.540 ± >> 0.143 ms/op >> >> (Thanks to Vlad for coming up for the exact incantation which leads to >> profile pollution :-) ) >> >> The solution is to create a sharp subclass for each heap segment case. With >> this, C2 has always a sharp Unsafe *base* to work with, and performances are >> stable regardless of profile pollution attempts. >> >> This patch also tweaks the benchmark for heap segments so that it checks it >> with and without profile pollution. > > Maurizio Cimadamore has updated the pull request incrementally with one > additional commit since the last revision: > > Fix typo Marked as reviewed by sundar (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/1259
Re: RFR: 8252919: JDK built with --enable-cds=no fails with NoClassDefFoundError
On Wed, 9 Sep 2020 19:38:38 GMT, Mandy Chung wrote: > `jlink --generate-jli-classes` plugin should retain the original holder > classes if the default_jli_trace.txt > file does not exist. > > Before JDK-8252725, `JavaLangInvokeAccess::generateXXXHolderClassesBytes` > methods are invoked > unconditionally and therefore the holder classes are "regenerated" when > default_jli_trace.txt > does not exist. JDK-8252725 does not handle this case properly and results > in an image missing > the holder classes when the specified trace file does not exist. > > The fix is very simple. Retains the original holder classes when the file > does not exist. Looks good - Marked as reviewed by sundar (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/96
