Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> We will refresh this PR periodically to pick up changes and fixes from the >> loom repo. >> >> Most of the new mechanisms in the HotSpot VM are disabled by default and >> require running with `--enable-preview` to enable. >> >> The patch has support for x64 and aarch64 on the usual operating systems >> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for >> zero and some of the other ports. Additional ports can be contributed via >> PRs against the fibers branch in the loom repo. >> >> There are changes in many areas. To reduce notifications/mails, the labels >> have been trimmed down for now to hotspot, serviceability and core-libs. >> We'll add the complete set of labels when the PR is further along. >> >> The changes include a refresh of java.util.concurrent and ForkJoinPool from >> Doug Lea's CVS. These changes will probably be proposed and integrated in >> advance of this PR. >> >> The changes include some non-exposed and low-level infrastructure to support >> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to >> make life a bit easier and avoid having to separate VM changes and juggle >> branches at this time. > > Alan Bateman has updated the pull request incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e test/hotspot/jtreg/serviceability/jvmti/events/Breakpoint/breakpoint01/libbreakpoint01.cpp line 66: > 64: for (i = 0; i < METH_NUM; i++) > 65: bpEvents[i] = 0; > 66: } As I understand all new tests are C++ based. So, I'd suggest to always use C++ style of loops: for (int i = 0; i < METH_NUM; i++) - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> We will refresh this PR periodically to pick up changes and fixes from the >> loom repo. >> >> Most of the new mechanisms in the HotSpot VM are disabled by default and >> require running with `--enable-preview` to enable. >> >> The patch has support for x64 and aarch64 on the usual operating systems >> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for >> zero and some of the other ports. Additional ports can be contributed via >> PRs against the fibers branch in the loom repo. >> >> There are changes in many areas. To reduce notifications/mails, the labels >> have been trimmed down for now to hotspot, serviceability and core-libs. >> We'll add the complete set of labels when the PR is further along. >> >> The changes include a refresh of java.util.concurrent and ForkJoinPool from >> Doug Lea's CVS. These changes will probably be proposed and integrated in >> advance of this PR. >> >> The changes include some non-exposed and low-level infrastructure to support >> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to >> make life a bit easier and avoid having to separate VM changes and juggle >> branches at this time. > > Alan Bateman has updated the pull request incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e Marked as reviewed by will-mol...@github.com (no known OpenJDK username). - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284331: Add sanity check for signal handler modification warning.
On Tue, 5 Apr 2022 10:37:26 GMT, Kevin Walls wrote: > A sanity check using "jcmd VM.info" to catch the signal handler modification > warning: it should never trigger during this test. How does this relate the failure in JDK-8285647? Is this just meant to detect that failure, but a proper fix is still needed for it? - PR: https://git.openjdk.java.net/jdk/pull/8106
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> We will refresh this PR periodically to pick up changes and fixes from the >> loom repo. >> >> Most of the new mechanisms in the HotSpot VM are disabled by default and >> require running with `--enable-preview` to enable. >> >> The patch has support for x64 and aarch64 on the usual operating systems >> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for >> zero and some of the other ports. Additional ports can be contributed via >> PRs against the fibers branch in the loom repo. >> >> There are changes in many areas. To reduce notifications/mails, the labels >> have been trimmed down for now to hotspot, serviceability and core-libs. >> We'll add the complete set of labels when the PR is further along. >> >> The changes include a refresh of java.util.concurrent and ForkJoinPool from >> Doug Lea's CVS. These changes will probably be proposed and integrated in >> advance of this PR. >> >> The changes include some non-exposed and low-level infrastructure to support >> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to >> make life a bit easier and avoid having to separate VM changes and juggle >> branches at this time. > > Alan Bateman has updated the pull request incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e Remind: please use the command `/jep JEP-425` [1] to mark this PR. [1] https://wiki.openjdk.java.net/display/SKARA/Pull+Request+Commands#PullRequestCommands-/jep - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284331: Add sanity check for signal handler modification warning.
On Tue, 5 Apr 2022 10:37:26 GMT, Kevin Walls wrote: > A sanity check using "jcmd VM.info" to catch the signal handler modification > warning: it should never trigger during this test. Marked as reviewed by amenkov (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8106
Re: RFR: 8278123: serviceability/dcmd/vm/ClassLoaderStatsTest.java failing with java.lang.AssertionError: Should have a hidden class [v2]
> The test failed if GC happens somewhere between
> Class c = Class.forName("TestClass", true, dummyloader);
> and
> OutputAnalyzer output = executor.execute("VM.classloader_stats");
>
> The fix is to make hc static as Chris proposed.
>
> To verfiy fix I add System.gc() before
> executor.execute("VM.classloader_stats");
Leonid Mesnik 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 three additional
commits since the last revision:
- comment fixed
- Merge branch 'master' of https://github.com/openjdk/jdk into 8278123
- fix
-
Changes:
- all: https://git.openjdk.java.net/jdk/pull/8438/files
- new: https://git.openjdk.java.net/jdk/pull/8438/files/108363b1..1e9d05a8
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk=8438=01
- incr: https://webrevs.openjdk.java.net/?repo=jdk=8438=00-01
Stats: 22 lines in 2 files changed: 8 ins; 0 del; 14 mod
Patch: https://git.openjdk.java.net/jdk/pull/8438.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/8438/head:pull/8438
PR: https://git.openjdk.java.net/jdk/pull/8438
Re: RFR: 8285364: Remove REF_ enum for java.lang.ref.Reference [v4]
On Thu, 28 Apr 2022 11:05:45 GMT, Kim Barrett wrote: >> Albert Mingkun Yang has updated the pull request incrementally with two >> additional commits since the last revision: >> >> - comment >> - Rework reference type initialization >> >>Signed-off-by: Albert Yang > > src/hotspot/share/classfile/classFileParser.cpp line 6098: > >> 6096: >> 6097: return _super_klass->reference_type() != REF_NONE; >> 6098: } > > Stylistically, I'd prefer an if-ladder here. I might also swap the > reference-type check and the name check, so the for-bootstrapping name check > case being last (with a comment to that effect). These if-checks more or less mirror the type hierarchy, `Object -> Reference -> Soft|Weak... -> ...`. Then, walking down the type hierarchy using the early-return style here makes more sense to me. > src/hotspot/share/oops/instanceKlass.cpp line 497: > >> 495: _nest_host_index(0), >> 496: _init_state(allocated), >> 497: _reference_type(REF_NONE), > > This is initializing `_reference_type` to the wrong value for a > `InstanceRefKlass` object, which then needs to reset it in the derived > constructor. Why not get the reference type from the parser? The (currently > file-scoped static) determine_reference_type function in instanceRefKlass.cpp > doesn't have any dependency on the klass object being constructed, just the > parser. The current approach limits the knowledge of non-strong ref types to `instanceRefKlass` file. - PR: https://git.openjdk.java.net/jdk/pull/8332
Re: RFR: 8285364: Remove REF_ enum for java.lang.ref.Reference [v5]
> Simple rename and some comments update. > > Test: build Albert Mingkun Yang has updated the pull request incrementally with one additional commit since the last revision: review - Changes: - all: https://git.openjdk.java.net/jdk/pull/8332/files - new: https://git.openjdk.java.net/jdk/pull/8332/files/e4f6a66a..4b9081be Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8332=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8332=03-04 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/8332.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8332/head:pull/8332 PR: https://git.openjdk.java.net/jdk/pull/8332
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> We will refresh this PR periodically to pick up changes and fixes from the >> loom repo. >> >> Most of the new mechanisms in the HotSpot VM are disabled by default and >> require running with `--enable-preview` to enable. >> >> The patch has support for x64 and aarch64 on the usual operating systems >> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for >> zero and some of the other ports. Additional ports can be contributed via >> PRs against the fibers branch in the loom repo. >> >> There are changes in many areas. To reduce notifications/mails, the labels >> have been trimmed down for now to hotspot, serviceability and core-libs. >> We'll add the complete set of labels when the PR is further along. >> >> The changes include a refresh of java.util.concurrent and ForkJoinPool from >> Doug Lea's CVS. These changes will probably be proposed and integrated in >> advance of this PR. >> >> The changes include some non-exposed and low-level infrastructure to support >> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to >> make life a bit easier and avoid having to separate VM changes and juggle >> branches at this time. > > Alan Bateman has updated the pull request incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e Marked as reviewed by egahlin (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: JDK-8285676: Add missing @param tags for type parameters on classes and interfaces [v5]
On Thu, 28 Apr 2022 18:24:33 GMT, Andrey Turbanov wrote:
>> 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 seven additional commits
>> since the last revision:
>>
>> - Update copyright years.
>> - Merge branch 'master' into JDK-8285676
>> - Respond to more review feedback.
>> - Respond to more review feedback.
>> - Respond to review feedback.
>> - Merge branch 'master' into JDK-8285676
>> - JDK-8285676: Add missing @param tags for type parameters on classes and
>> interfaces
>
> src/java.base/share/classes/java/lang/ref/PhantomReference.java line 48:
>
>> 46: * The {@link #refersTo(Object) refersTo} method can be used to test
>> 47: * whether some object is the referent of a phantom reference.
>> 48: * @param the type of the referent
>
> Shouldn't there be a space after `@param` ?
Good catch. Sorry I missed it. This occurs in all `java/lang/ref` files.
-
PR: https://git.openjdk.java.net/jdk/pull/8410
Re: RFR: JDK-8285676: Add missing @param tags for type parameters on classes and interfaces [v5]
On Thu, 28 Apr 2022 18:05:39 GMT, Joe Darcy wrote:
>> To enable more complete doclint checking (courtesy @jonathan-gibbons),
>> please review this PR to add type-level @param tags where they are missing.
>>
>> To the maintainers of java.util.concurrent, those changes could be separated
>> out in another bug if that would ease maintenance of that code.
>>
>> Making these library fixes is a blocker for correcting and expanding the
>> doclint checks (JDK-8285496).
>>
>> I'll update copyright years before pushing.
>
> 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 seven additional commits since
> the last revision:
>
> - Update copyright years.
> - Merge branch 'master' into JDK-8285676
> - Respond to more review feedback.
> - Respond to more review feedback.
> - Respond to review feedback.
> - Merge branch 'master' into JDK-8285676
> - JDK-8285676: Add missing @param tags for type parameters on classes and
> interfaces
src/java.base/share/classes/java/lang/ref/PhantomReference.java line 48:
> 46: * The {@link #refersTo(Object) refersTo} method can be used to test
> 47: * whether some object is the referent of a phantom reference.
> 48: * @param the type of the referent
Shouldn't there be a space after `@param` ?
-
PR: https://git.openjdk.java.net/jdk/pull/8410
Re: RFR: 8285149: Using HashMap.newHashMap to replace new HashMap(int) [v5]
On Thu, 28 Apr 2022 17:36:41 GMT, Weijun Wang wrote: > Looks like 1 is enough. @wangweij done. - PR: https://git.openjdk.java.net/jdk/pull/8301
Re: RFR: 8285149: Using HashMap.newHashMap to replace new HashMap(int) [v6]
> These are the changes that too many to be reviewed in 8186958, thus split > some of them out. XenoAmess has updated the pull request incrementally with one additional commit since the last revision: change from 3 to 1 according to wangweij - Changes: - all: https://git.openjdk.java.net/jdk/pull/8301/files - new: https://git.openjdk.java.net/jdk/pull/8301/files/c9cfb50a..1b4bac0f Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8301=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8301=04-05 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/8301.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8301/head:pull/8301 PR: https://git.openjdk.java.net/jdk/pull/8301
Integrated: JDK-8285676: Add missing @param tags for type parameters on classes and interfaces
On Tue, 26 Apr 2022 22:24:26 GMT, Joe Darcy wrote: > To enable more complete doclint checking (courtesy @jonathan-gibbons), please > review this PR to add type-level @param tags where they are missing. > > To the maintainers of java.util.concurrent, those changes could be separated > out in another bug if that would ease maintenance of that code. > > Making these library fixes is a blocker for correcting and expanding the > doclint checks (JDK-8285496). > > I'll update copyright years before pushing. This pull request has now been integrated. Changeset: bba456a8 Author:Joe Darcy URL: https://git.openjdk.java.net/jdk/commit/bba456a8dbf9027e4b015567c17a79fc7441aa08 Stats: 102 lines in 40 files changed: 76 ins; 0 del; 26 mod 8285676: Add missing @param tags for type parameters on classes and interfaces Reviewed-by: wetmore, smarks, dfuchs, prr, alanb, mchung - PR: https://git.openjdk.java.net/jdk/pull/8410
Re: RFR: JDK-8285676: Add missing @param tags for type parameters on classes and interfaces [v5]
> To enable more complete doclint checking (courtesy @jonathan-gibbons), please > review this PR to add type-level @param tags where they are missing. > > To the maintainers of java.util.concurrent, those changes could be separated > out in another bug if that would ease maintenance of that code. > > Making these library fixes is a blocker for correcting and expanding the > doclint checks (JDK-8285496). > > I'll update copyright years before pushing. 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 seven additional commits since the last revision: - Update copyright years. - Merge branch 'master' into JDK-8285676 - Respond to more review feedback. - Respond to more review feedback. - Respond to review feedback. - Merge branch 'master' into JDK-8285676 - JDK-8285676: Add missing @param tags for type parameters on classes and interfaces - Changes: - all: https://git.openjdk.java.net/jdk/pull/8410/files - new: https://git.openjdk.java.net/jdk/pull/8410/files/aaa8f828..cb1fe1c2 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8410=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8410=03-04 Stats: 687 lines in 59 files changed: 610 ins; 8 del; 69 mod Patch: https://git.openjdk.java.net/jdk/pull/8410.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8410/head:pull/8410 PR: https://git.openjdk.java.net/jdk/pull/8410
Re: RFR: JDK-8285676: Add missing @param tags for type parameters on classes and interfaces [v4]
On Thu, 28 Apr 2022 16:58:40 GMT, Joe Darcy wrote: >> To enable more complete doclint checking (courtesy @jonathan-gibbons), >> please review this PR to add type-level @param tags where they are missing. >> >> To the maintainers of java.util.concurrent, those changes could be separated >> out in another bug if that would ease maintenance of that code. >> >> Making these library fixes is a blocker for correcting and expanding the >> doclint checks (JDK-8285496). >> >> I'll update copyright years before pushing. > > Joe Darcy has updated the pull request incrementally with one additional > commit since the last revision: > > Respond to more review feedback. Marked as reviewed by mchung (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8410
Re: RFR: 8285149: Using HashMap.newHashMap to replace new HashMap(int) [v5]
On Thu, 28 Apr 2022 16:56:38 GMT, XenoAmess wrote: >> These are the changes that too many to be reviewed in 8186958, thus split >> some of them out. > > XenoAmess has updated the pull request incrementally with one additional > commit since the last revision: > > revert changes to java.desktop as prrace said src/java.base/share/classes/sun/security/rsa/SunRsaSignEntries.java line 58: > 56: // start populating content using the specified provider > 57: // common attribute map > 58: HashMap attrs = HashMap.newHashMap(3); Looks like 1 is enough. - PR: https://git.openjdk.java.net/jdk/pull/8301
Re: RFR: JDK-8285676: Add missing @param tags for type parameters on classes and interfaces [v4]
On Thu, 28 Apr 2022 16:58:40 GMT, Joe Darcy wrote: >> To enable more complete doclint checking (courtesy @jonathan-gibbons), >> please review this PR to add type-level @param tags where they are missing. >> >> To the maintainers of java.util.concurrent, those changes could be separated >> out in another bug if that would ease maintenance of that code. >> >> Making these library fixes is a blocker for correcting and expanding the >> doclint checks (JDK-8285496). >> >> I'll update copyright years before pushing. > > Joe Darcy has updated the pull request incrementally with one additional > commit since the last revision: > > Respond to more review feedback. Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8410
Re: RFR: JDK-8285676: Add missing @param tags for type parameters on classes and interfaces [v4]
> To enable more complete doclint checking (courtesy @jonathan-gibbons), please > review this PR to add type-level @param tags where they are missing. > > To the maintainers of java.util.concurrent, those changes could be separated > out in another bug if that would ease maintenance of that code. > > Making these library fixes is a blocker for correcting and expanding the > doclint checks (JDK-8285496). > > I'll update copyright years before pushing. Joe Darcy has updated the pull request incrementally with one additional commit since the last revision: Respond to more review feedback. - Changes: - all: https://git.openjdk.java.net/jdk/pull/8410/files - new: https://git.openjdk.java.net/jdk/pull/8410/files/db4919a9..aaa8f828 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8410=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8410=02-03 Stats: 2 lines in 2 files changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/8410.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8410/head:pull/8410 PR: https://git.openjdk.java.net/jdk/pull/8410
Re: RFR: 8278123: serviceability/dcmd/vm/ClassLoaderStatsTest.java failing with java.lang.AssertionError: Should have a hidden class
On Thu, 28 Apr 2022 07:19:34 GMT, David Holmes wrote: >> Lookup.defineHiddenClass allows class options to be specified, one of which >> is "STRONG" to mean that the hidden class can't unloaded if its defining >> loader is reachable. >> >> A static reference or a reachability fence should work for this test. > > By default hidden classes are non-strong and can be unloaded when no longer > referenced, even if their classloader is still reachable. As you want the > hidden class to stick around so it can be reported in the stats then either > keep a reference (as you have) or else create it as a STRONG hidden class. I'm ok with the changes but would like to see the comment fixed. It looks like it should be: "Create a hidden non-strong class. Keep a reference in case a GC happens." - PR: https://git.openjdk.java.net/jdk/pull/8438
Re: RFR: JDK-8285676: Add missing @param tags for type parameters on classes and interfaces [v3]
On Thu, 28 Apr 2022 08:10:38 GMT, Alan Bateman wrote: >> Joe Darcy has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Respond to more review feedback. > > src/java.base/share/classes/java/nio/file/WatchEvent.java line 51: > >> 49: /** >> 50: * An event kind, for the purposes of identification. >> 51: * @param the type of the context value > > This is okay but the it differs slightly to the type parameters specified > further up. I think the issue here is that it was just wasn't copied down to > WatchEvent.Kind. Okay -- I'll for the T type parameter of the Kind interface I'll reuse the wording of the T type parameter of the enclosing WatchEvent interface. (The type variable on Kind could be renamed to show that the two type parameters are distinct.) - PR: https://git.openjdk.java.net/jdk/pull/8410
Re: RFR: 8285149: Using HashMap.newHashMap to replace new HashMap(int) [v5]
> These are the changes that too many to be reviewed in 8186958, thus split > some of them out. XenoAmess has updated the pull request incrementally with one additional commit since the last revision: revert changes to java.desktop as prrace said - Changes: - all: https://git.openjdk.java.net/jdk/pull/8301/files - new: https://git.openjdk.java.net/jdk/pull/8301/files/684d1528..c9cfb50a Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8301=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8301=03-04 Stats: 56 lines in 30 files changed: 8 ins; 0 del; 48 mod Patch: https://git.openjdk.java.net/jdk/pull/8301.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8301/head:pull/8301 PR: https://git.openjdk.java.net/jdk/pull/8301
Re: RFR: JDK-8285676: Add missing @param tags for type parameters on classes and interfaces [v3]
On Thu, 28 Apr 2022 08:08:37 GMT, Alan Bateman wrote: >> Joe Darcy has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Respond to more review feedback. > > src/java.base/share/classes/java/nio/file/SecureDirectoryStream.java line 55: > >> 53: * against the original path of the directory (irrespective of if >> the >> 54: * directory is moved since it was opened). >> 55: * @param the type of path > > It may not be a path. The type parameter is specified in the super > interfaces, can you copy that down instead? Will change in the next push. - PR: https://git.openjdk.java.net/jdk/pull/8410
Re: RFR: 8285149: Using HashMap.newHashMap to replace new HashMap(int) [v4]
On Wed, 27 Apr 2022 05:11:54 GMT, XenoAmess wrote: > Also do not change J2DBench. We deliberately avoid using new API so that we > can take it and run it on very old JDK versions to help track regressions. For J2DBench, I don't see any changes that not complicated with older jdk version for now. please recheck. thanks. - PR: https://git.openjdk.java.net/jdk/pull/8301
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Thu, 28 Apr 2022 15:10:18 GMT, Markus Grönlund wrote:
>> src/jdk.jfr/share/classes/jdk/jfr/events/VirtualThreadSubmitFailedEvent.java
>> line 35:
>>
>>> 33:
>>> 34: @Category({"Java Runtime"})
>>> 35: @Label("Virtual thread submit task failed")
>>
>> The label is a bit a long, would "Virtual Thread Submit Failed" work?
>
> It works. Thanks.
Yes, this is okay.
-
PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Thu, 28 Apr 2022 14:44:05 GMT, Erik Gahlin wrote:
>> Alan Bateman has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e
>
> src/hotspot/share/jfr/leakprofiler/sampling/objectSampler.cpp line 168:
>
>> 166: assert(!tl->is_excluded(), "invariant");
>> 167: if (virtual_thread) {
>> 168: // TODO: blob cache for virtual threads
>
> Fix this now or after integration?
Well spotted. It is an optimization and can happen after integration. I will
create an enhancement for tracking; thank you.
> src/hotspot/share/jfr/metadata/metadata.xml line 121:
>
>> 119: thread="true" stackTrace="true">
>> 120: > description="Thread enlisted as a carrier" />
>> 121: > description="Class of the continuation" />
>
> "Continuation class" = >" Continuation Class"
> numFrames = frames
> numRefs = references
> "Number of interpreted frames" => "Interpreted Frames"
> "Number of references" => "References"
> "Stack size in bytes" => "Stack Size" contentType"bytes"
Will fix.
> src/hotspot/share/jfr/metadata/metadata.xml line 130:
>
>> 128: thread="true" stackTrace="true">
>> 129: > description="Thread enlisted as a carrier" />
>> 130: > description="Class of the continuation" />
>
> contClass => continuationClass
> "Continuation class" => "Continuation Class"
> "Class of the continuation" Remove (not needed)
> "Number of interpreted frames" => "Interpreted Frames"
> numFrames => frames
> "Number of references" => "References"
> numRefs => references
> "Stack size in bytes" => "Stack Size" contentType="bytes"
Will fix.
> src/hotspot/share/jfr/metadata/metadata.xml line 138:
>
>> 136: > category="Java Virtual Machine" label="Continuation Freeze Young"
>> thread="true" stackTrace="false" startTime="false">
>> 137:
>> 138:
>
> "Allocated new" => "Allocated New"
Will fix, thanks.
> src/jdk.jfr/share/classes/jdk/jfr/events/VirtualThreadEndEvent.java line 35:
>
>> 33:
>> 34: @Category({"Java Runtime"})
>> 35: @Label("Virtual thread end")
>
> "Virtual thread end" => "Virtual Thread End"
> Remove description.
Will fix.
> src/jdk.jfr/share/classes/jdk/jfr/events/VirtualThreadPinnedEvent.java line
> 35:
>
>> 33:
>> 34: @Category({"Java Runtime"})
>> 35: @Label("Virtual thread pinned")
>
> "Virtual thread pinned" => "Virtual Thread Pinned"
> Remove description
Will fix.
> src/jdk.jfr/share/classes/jdk/jfr/events/VirtualThreadStartEvent.java line 35:
>
>> 33:
>> 34: @Category({"Java Runtime"})
>> 35: @Label("Virtual thread start")
>
> "virtual thread start" => "Virtual Thread Start"
> Remove description
Will fix, thanks.
> src/jdk.jfr/share/classes/jdk/jfr/events/VirtualThreadSubmitFailedEvent.java
> line 35:
>
>> 33:
>> 34: @Category({"Java Runtime"})
>> 35: @Label("Virtual thread submit task failed")
>
> The label is a bit a long, would "Virtual Thread Submit Failed" work?
It works. Thanks.
-
PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Thu, 28 Apr 2022 14:41:04 GMT, Erik Gahlin wrote: >> Alan Bateman has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e > > src/hotspot/share/jfr/recorder/checkpoint/jfrCheckpointManager.cpp line 94: > >> 92: static const size_t global_buffer_size = 512 * K; >> 93: >> 94: static const size_t thread_local_buffer_prealloc_count = 32; > > Why is more memory needed? Moore's law or something specific to virtual > threads? The carrier thread will write the metadata for the mounted virtual threads lazily with virtual threads as part of the event write. They write this thread locally, and the memory increase accommodates fewer roundtrips to fetch new buffers. The previous size was small because it would only hold at most one entry. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Thu, 28 Apr 2022 14:37:22 GMT, Erik Gahlin wrote: >> Alan Bateman has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e > > src/hotspot/share/jfr/recorder/checkpoint/jfrCheckpointWriter.cpp line 81: > >> 79: be_writer.write(size); >> 80: be_writer.write(time); >> 81: be_writer.write(JfrTicks::now().value() - time); > > Why is this changed? It was a small optimization attempted before the current system of writing virtual thread checkpoints in bulk. Will restore it, thank you. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote:
>> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which
>> JDK version to target.
>>
>> We will refresh this PR periodically to pick up changes and fixes from the
>> loom repo.
>>
>> Most of the new mechanisms in the HotSpot VM are disabled by default and
>> require running with `--enable-preview` to enable.
>>
>> The patch has support for x64 and aarch64 on the usual operating systems
>> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for
>> zero and some of the other ports. Additional ports can be contributed via
>> PRs against the fibers branch in the loom repo.
>>
>> There are changes in many areas. To reduce notifications/mails, the labels
>> have been trimmed down for now to hotspot, serviceability and core-libs.
>> We'll add the complete set of labels when the PR is further along.
>>
>> The changes include a refresh of java.util.concurrent and ForkJoinPool from
>> Doug Lea's CVS. These changes will probably be proposed and integrated in
>> advance of this PR.
>>
>> The changes include some non-exposed and low-level infrastructure to support
>> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to
>> make life a bit easier and avoid having to separate VM changes and juggle
>> branches at this time.
>
> Alan Bateman has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e
src/hotspot/share/jfr/leakprofiler/sampling/objectSampler.cpp line 168:
> 166: assert(!tl->is_excluded(), "invariant");
> 167: if (virtual_thread) {
> 168: // TODO: blob cache for virtual threads
Fix this now or after integration?
src/hotspot/share/jfr/metadata/metadata.xml line 121:
> 119: thread="true" stackTrace="true">
> 120: description="Thread enlisted as a carrier" />
> 121: description="Class of the continuation" />
"Continuation class" = >" Continuation Class"
numFrames = frames
numRefs = references
"Number of interpreted frames" => "Interpreted Frames"
"Number of references" => "References"
"Stack size in bytes" => "Stack Size" contentType"bytes"
src/hotspot/share/jfr/metadata/metadata.xml line 130:
> 128: thread="true" stackTrace="true">
> 129: description="Thread enlisted as a carrier" />
> 130: description="Class of the continuation" />
contClass => continuationClass
"Continuation class" => "Continuation Class"
"Class of the continuation" Remove (not needed)
"Number of interpreted frames" => "Interpreted Frames"
numFrames => frames
"Number of references" => "References"
numRefs => references
"Stack size in bytes" => "Stack Size" contentType="bytes"
src/hotspot/share/jfr/metadata/metadata.xml line 138:
> 136:category="Java Virtual Machine" label="Continuation Freeze Young"
> thread="true" stackTrace="false" startTime="false">
> 137:
> 138:
"Allocated new" => "Allocated New"
src/hotspot/share/jfr/recorder/checkpoint/jfrCheckpointManager.cpp line 94:
> 92: static const size_t global_buffer_size = 512 * K;
> 93:
> 94: static const size_t thread_local_buffer_prealloc_count = 32;
Why is more memory needed? Moore's law or something specific to virtual threads?
src/hotspot/share/jfr/recorder/checkpoint/jfrCheckpointWriter.cpp line 81:
> 79: be_writer.write(size);
> 80: be_writer.write(time);
> 81: be_writer.write(JfrTicks::now().value() - time);
Why is this changed?
src/jdk.jfr/share/classes/jdk/jfr/events/VirtualThreadEndEvent.java line 35:
> 33:
> 34: @Category({"Java Runtime"})
> 35: @Label("Virtual thread end")
"Virtual thread end" => "Virtual Thread End"
Remove description.
src/jdk.jfr/share/classes/jdk/jfr/events/VirtualThreadPinnedEvent.java line 35:
> 33:
> 34: @Category({"Java Runtime"})
> 35: @Label("Virtual thread pinned")
"Virtual thread pinned" => "Virtual Thread Pinned"
Remove description
src/jdk.jfr/share/classes/jdk/jfr/events/VirtualThreadStartEvent.java line 35:
> 33:
> 34: @Category({"Java Runtime"})
> 35: @Label("Virtual thread start")
"virtual thread start" => "Virtual Thread Start"
Remove description
src/jdk.jfr/share/classes/jdk/jfr/events/VirtualThreadSubmitFailedEvent.java
line 35:
> 33:
> 34: @Category({"Java Runtime"})
> 35: @Label("Virtual thread submit task failed")
The label is a bit a long, would "Virtual Thread Submit Failed" work?
-
PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8285794: AsyncGetCallTrace might acquire a lock via JavaThread::thread_from_jni_environment [v2]
On Thu, 28 Apr 2022 11:38:32 GMT, Johannes Bechberger
wrote:
>> Calling JavaThread::thread_from_jni_environment for a terminated thread in
>> AsyncGetCallTrace might cause the acquisition of a lock, making
>> AsyncGetCallTrace non-signal-safe.
>>
>> AsyncGetCallTrace can only be called for the current threads (there are
>> asserts for that), therefore using JavaThread::current directly and checking
>> the termination status is semantically equivalent.
>
> Johannes Bechberger has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Use Thread::current_or_null_safe() and add comments
Changes requested by dholmes (Reviewer).
src/hotspot/share/prims/forte.cpp line 35:
> 33: #include "runtime/frame.inline.hpp"
> 34: #include "runtime/javaCalls.hpp"
> 35: #include "runtime/thread.hpp"
You don't need this as you already include the thread.inline.hpp which has to
include thread.hpp.
src/hotspot/share/prims/forte.cpp line 571:
> 569: Thread* raw_thread = Thread::current_or_null_safe();
> 570:
> 571: if (trace->env_id == NULL || raw_thread == NULL ||
> !raw_thread->is_Java_thread() || ((JavaThread*)raw_thread)->is_exiting()) {
use `rawThread->as_JavaThread()` not a plain cast.
src/hotspot/share/prims/forte.cpp line 577:
> 575: }
> 576:
> 577: JavaThread* thread = (JavaThread*)raw_thread;
use `rawThread->as_JavaThread()` not a plain cast. You could also incorporate
this into the earlier code so you don't do it twice.
-
PR: https://git.openjdk.java.net/jdk/pull/8446
Re: RFR: 8285794: AsyncGetCallTrace might acquire a lock via JavaThread::thread_from_jni_environment [v2]
> Calling JavaThread::thread_from_jni_environment for a terminated thread in > AsyncGetCallTrace might cause the acquisition of a lock, making > AsyncGetCallTrace non-signal-safe. > > AsyncGetCallTrace can only be called for the current threads (there are > asserts for that), therefore using JavaThread::current directly and checking > the termination status is semantically equivalent. Johannes Bechberger has updated the pull request incrementally with one additional commit since the last revision: Use Thread::current_or_null_safe() and add comments - Changes: - all: https://git.openjdk.java.net/jdk/pull/8446/files - new: https://git.openjdk.java.net/jdk/pull/8446/files/28180bc9..1e2885e6 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8446=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8446=00-01 Stats: 8 lines in 1 file changed: 6 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/8446.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8446/head:pull/8446 PR: https://git.openjdk.java.net/jdk/pull/8446
Re: RFR: 8285794: AsyncGetCallTrace might acquire a lock via JavaThread::thread_from_jni_environment
On Thu, 28 Apr 2022 11:15:34 GMT, David Holmes wrote:
>> Calling JavaThread::thread_from_jni_environment for a terminated thread in
>> AsyncGetCallTrace might cause the acquisition of a lock, making
>> AsyncGetCallTrace non-signal-safe.
>>
>> AsyncGetCallTrace can only be called for the current threads (there are
>> asserts for that), therefore using JavaThread::current directly and checking
>> the termination status is semantically equivalent.
>
> src/hotspot/share/prims/forte.cpp line 566:
>
>> 564: void AsyncGetCallTrace(ASGCT_CallTrace *trace, jint depth, void*
>> ucontext) {
>> 565:
>> 566: JavaThread* thread = JavaThread::current_or_null();
>
> As this can be in a signal handling context it needs to be
> Thread::current_or_null_safe().
Also please add a comment before this:
// Can't use thread_from_jni_environment as it may also perform a VM exit check
that is unsafe to
// do from this context.
-
PR: https://git.openjdk.java.net/jdk/pull/8446
Re: RFR: 8285794: AsyncGetCallTrace might acquire a lock via JavaThread::thread_from_jni_environment
On Thu, 28 Apr 2022 09:38:30 GMT, Johannes Bechberger
wrote:
> Calling JavaThread::thread_from_jni_environment for a terminated thread in
> AsyncGetCallTrace might cause the acquisition of a lock, making
> AsyncGetCallTrace non-signal-safe.
>
> AsyncGetCallTrace can only be called for the current threads (there are
> asserts for that), therefore using JavaThread::current directly and checking
> the termination status is semantically equivalent.
Changes requested by dholmes (Reviewer).
src/hotspot/share/prims/forte.cpp line 566:
> 564: void AsyncGetCallTrace(ASGCT_CallTrace *trace, jint depth, void*
> ucontext) {
> 565:
> 566: JavaThread* thread = JavaThread::current_or_null();
As this can be in a signal handling context it needs to be
Thread::current_or_null_safe().
src/hotspot/share/prims/forte.cpp line 568:
> 566: JavaThread* thread = JavaThread::current_or_null();
> 567:
> 568: if (trace->env_id == NULL || thread == NULL || thread->is_terminated()
> || thread->is_exiting()) {
`is_exiting()` also covers `is_terminated()`.
src/hotspot/share/prims/forte.cpp line 580:
> 578: }
> 579:
> 580: assert(thread ==
> JavaThread::thread_from_jni_environment(trace->env_id),
Please add a comment before the assert:
// This is safe now as the thread has not terminated and so no VM exit check
occurs.
-
PR: https://git.openjdk.java.net/jdk/pull/8446
Re: RFR: 8285364: Remove REF_ enum for java.lang.ref.Reference [v4]
On Thu, 28 Apr 2022 09:15:33 GMT, Albert Mingkun Yang wrote:
>> Simple rename and some comments update.
>>
>> Test: build
>
> Albert Mingkun Yang has updated the pull request incrementally with two
> additional commits since the last revision:
>
> - comment
> - Rework reference type initialization
>
>Signed-off-by: Albert Yang
There are further cleanups possible if we didn't have REF_NONE and
InstanceKlass didn't have a reference type. But there are a number of uses of
InstanceKlass::reference_type. Doing anything along those lines (assuming it's
deemed worthwhile to do so) can be done in a followup RFE.
src/hotspot/share/classfile/classFileParser.cpp line 6098:
> 6096:
> 6097: return _super_klass->reference_type() != REF_NONE;
> 6098: }
Stylistically, I'd prefer an if-ladder here. I might also swap the
reference-type check and the name check, so the for-bootstrapping name check
case being last (with a comment to that effect).
src/hotspot/share/oops/instanceKlass.cpp line 497:
> 495: _nest_host_index(0),
> 496: _init_state(allocated),
> 497: _reference_type(REF_NONE),
This is initializing `_reference_type` to the wrong value for a
`InstanceRefKlass` object, which then needs to reset it in the derived
constructor. Why not get the reference type from the parser? The (currently
file-scoped static) determine_reference_type function in instanceRefKlass.cpp
doesn't have any dependency on the klass object being constructed, just the
parser.
src/hotspot/share/oops/instanceKlass.hpp line 580:
> 578: protected:
> 579: // Only used by the InstanceRefKlass constructor
> 580: void set_reference_type(ReferenceType t) {
This function wouldn't be needed at all if the reference type was properly
initialized.
src/hotspot/share/oops/instanceRefKlass.cpp line 55:
> 53: }
> 54:
> 55: // Bootstrapping: this is either of the four direct subclasses of
> java.lang.ref.Reference
s/either of the four/one of the/. In particular remove "four" else, someday,
finalization will finally be GC'd and this comment will need to be noticed and
updated.
-
Changes requested by kbarrett (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/8332
RFR: 8285794: AsyncGetCallTrace might acquire a lock via JavaThread::thread_from_jni_environment
Calling JavaThread::thread_from_jni_environment for a terminated thread in AsyncGetCallTrace might cause the acquisition of a lock, making AsyncGetCallTrace non-signal-safe. AsyncGetCallTrace can only be called for the current threads (there are asserts for that), therefore using JavaThread::current directly and checking the termination status is semantically equivalent. - Commit messages: - Use JavaThread::current_or_null - Do not call JavaThread::thread_from_jni_environment Changes: https://git.openjdk.java.net/jdk/pull/8446/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8446=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8285794 Stats: 4 lines in 1 file changed: 0 ins; 1 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/8446.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8446/head:pull/8446 PR: https://git.openjdk.java.net/jdk/pull/8446
Re: RFR: 8285364: Remove REF_ enum for java.lang.ref.Reference [v4]
> Simple rename and some comments update. > > Test: build Albert Mingkun Yang has updated the pull request incrementally with two additional commits since the last revision: - comment - Rework reference type initialization Signed-off-by: Albert Yang - Changes: - all: https://git.openjdk.java.net/jdk/pull/8332/files - new: https://git.openjdk.java.net/jdk/pull/8332/files/73f80ea4..e4f6a66a Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8332=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8332=02-03 Stats: 57 lines in 7 files changed: 50 ins; 4 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/8332.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8332/head:pull/8332 PR: https://git.openjdk.java.net/jdk/pull/8332
Re: RFR: 8285364: Remove REF_ enum for java.lang.ref.Reference [v3]
On Fri, 22 Apr 2022 09:43:45 GMT, Albert Mingkun Yang wrote: >> Simple rename and some comments update. >> >> Test: build > > Albert Mingkun Yang has updated the pull request incrementally with one > additional commit since the last revision: > > Remove REF_ enum for java.lang.ref.Reference > > Signed-off-by: Albert Yang I have taken "Rework reference type initialization" from Stefan and added comments on a potential surprise that `j.l.r.Reference` is `instanceKlass` not `instanceRefKlass`. - PR: https://git.openjdk.java.net/jdk/pull/8332
Re: RFR: JDK-8285676: Add missing @param tags for type parameters on classes and interfaces [v3]
On Thu, 28 Apr 2022 01:34:19 GMT, Joe Darcy wrote: >> To enable more complete doclint checking (courtesy @jonathan-gibbons), >> please review this PR to add type-level @param tags where they are missing. >> >> To the maintainers of java.util.concurrent, those changes could be separated >> out in another bug if that would ease maintenance of that code. >> >> Making these library fixes is a blocker for correcting and expanding the >> doclint checks (JDK-8285496). >> >> I'll update copyright years before pushing. > > Joe Darcy has updated the pull request incrementally with one additional > commit since the last revision: > > Respond to more review feedback. src/java.base/share/classes/java/nio/file/SecureDirectoryStream.java line 55: > 53: * against the original path of the directory (irrespective of if > the > 54: * directory is moved since it was opened). > 55: * @param the type of path It may not be a path. The type parameter is specified in the super interfaces, can you copy that down instead? src/java.base/share/classes/java/nio/file/WatchEvent.java line 51: > 49: /** > 50: * An event kind, for the purposes of identification. > 51: * @param the type of the context value This is okay but the it differs slightly to the type parameters specified further up. I think the issue here is that it was just wasn't copied down to WatchEvent.Kind. - PR: https://git.openjdk.java.net/jdk/pull/8410
Re: RFR: JDK-8285676: Add missing @param tags for type parameters on classes and interfaces [v3]
On Thu, 28 Apr 2022 01:34:19 GMT, Joe Darcy wrote: >> To enable more complete doclint checking (courtesy @jonathan-gibbons), >> please review this PR to add type-level @param tags where they are missing. >> >> To the maintainers of java.util.concurrent, those changes could be separated >> out in another bug if that would ease maintenance of that code. >> >> Making these library fixes is a blocker for correcting and expanding the >> doclint checks (JDK-8285496). >> >> I'll update copyright years before pushing. > > Joe Darcy has updated the pull request incrementally with one additional > commit since the last revision: > > Respond to more review feedback. Thanks for the updates Joe. Your new wording looks good to me. - Marked as reviewed by dfuchs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8410
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 19:43:22 GMT, Paul Sandoz wrote:
>> Alan Bateman has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e
>
> src/java.base/share/classes/java/util/concurrent/ThreadPerTaskExecutor.java
> line 45:
>
>> 43: * threads is unbounded.
>> 44: */
>> 45: class ThreadPerTaskExecutor implements ExecutorService {
>
> This class manages the set of per-task threads which arguably should be the
> job of the thread container, and it awkwardly overrides the container's set
> of threads by setting a field on `SharedThreadContainer.threadsSupplier`.
>
> `SharedThreadContainer` supports a number of different shared container
> policies that could be made clearer.
>
> Architecturally i think this could be better layered but it can be iterated
> on later.
Indeed, the layering is a bit awkward and I plan to replace it. It came about
as a short term solution to avoid double bookkeeping of virtual threads.
-
PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 19:21:58 GMT, Alan Bateman wrote: >> src/java.base/share/classes/jdk/internal/misc/UnsafeConstants.java line 122: >> >>> 120: */ >>> 121: >>> 122: public static final int SCOPED_CACHE_SHIFT; >> >> I can't find this constant being used. If added for future, maybe keep >> `UnsafeConstants` class and this static field package-private for now. > > It originally configured the number of lines in extent local cache but the > implementation has changed. @theRealAph would be best to comment on this, it > may be possible to delete it. Yes, it's dead. Thanks. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8278123: serviceability/dcmd/vm/ClassLoaderStatsTest.java failing with java.lang.AssertionError: Should have a hidden class
On Thu, 28 Apr 2022 07:16:05 GMT, Alan Bateman wrote: >> Yes, that seems to be the case, and you are right that it is not something >> that is safe to assume. > > Lookup.defineHiddenClass allows class options to be specified, one of which > is "STRONG" to mean that the hidden class can't unloaded if its defining > loader is reachable. > > A static reference or a reachability fence should work for this test. By default hidden classes are non-strong and can be unloaded when no longer referenced, even if their classloader is still reachable. As you want the hidden class to stick around so it can be reported in the stats then either keep a reference (as you have) or else create it as a STRONG hidden class. - PR: https://git.openjdk.java.net/jdk/pull/8438
Re: RFR: 8278123: serviceability/dcmd/vm/ClassLoaderStatsTest.java failing with java.lang.AssertionError: Should have a hidden class
On Thu, 28 Apr 2022 04:46:35 GMT, Chris Plummer wrote: >> This line added by 8238358: Implementation of JEP 371: Hidden Classes which >> has many co-authors. Hope someone could provide an explanation during this >> review. >> >> It might be possible that the goal was to verify that VM.classloader_stats >> provide might provide info for non-reachable clasees. However it makes test >> to fragile, since can't block class unloading now. > > Yes, that seems to be the case, and you are right that it is not something > that is safe to assume. Lookup.defineHiddenClass allows class options to be specified, one of which is "STRONG" to mean that the hidden class can't unloaded if its defining loader is reachable. A static reference or a reachability fence should work for this test. - PR: https://git.openjdk.java.net/jdk/pull/8438
