Re: RFR: JDK-8315575: Retransform of record class with record component annotation fails with CFE [v3]
On Thu, 14 Mar 2024 02:12:29 GMT, Serguei Spitsyn wrote: > True. It should know how to put the `attribute_count` value into the class > file but it does not need to know how to calculate its value. What I do not > like in your model is that there is no one single place which knows how to > calculate this value and the existing and potential consumers should have > this knowledge. It **must** know how to calculate the value. Because this is number of `attribute_info_attributes` attributes that follow. And only `JvmtiClassFileReconstituter` knows how many attributes it's going to write. > The bug is that the `attributute_count` field value from the class file > stored in the `RecordComponent` does not match the calculated value because > the invisible attribute was not saved (was ignored). I'd consider this as design issue with the current implementation - `JvmtiClassFileReconstituter` gets the value from external source (`RecordComponent`) and uses it without validation. This approach is inconsistent with other `JvmtiClassFileReconstituter` code. For other similar cases it calculates record counts in place (from size of arrays, string length, from list enumerator, etc.). - PR Review Comment: https://git.openjdk.org/jdk/pull/18161#discussion_r1525542056
Re: RFR: 8326666: Remove the Java Management Extension (JMX) Subject Delegation feature [v14]
> The deprecated Subject Delegation feature in JMX will be removed. > > This was marked in JDK 21 as deprecated for removal (JDK-8298966). Kevin Walls has updated the pull request incrementally with one additional commit since the last revision: Missing code doc nit. - Changes: - all: https://git.openjdk.org/jdk/pull/18025/files - new: https://git.openjdk.org/jdk/pull/18025/files/64c9c64a..43b10a12 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18025=13 - incr: https://webrevs.openjdk.org/?repo=jdk=18025=12-13 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/18025.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18025/head:pull/18025 PR: https://git.openjdk.org/jdk/pull/18025
Re: RFR: 8326666: Remove the Java Management Extension (JMX) Subject Delegation feature [v13]
> The deprecated Subject Delegation feature in JMX will be removed. > > This was marked in JDK 21 as deprecated for removal (JDK-8298966). Kevin Walls has updated the pull request incrementally with one additional commit since the last revision: Missing code doc nit. - Changes: - all: https://git.openjdk.org/jdk/pull/18025/files - new: https://git.openjdk.org/jdk/pull/18025/files/91ec015f..64c9c64a Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18025=12 - incr: https://webrevs.openjdk.org/?repo=jdk=18025=11-12 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/18025.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18025/head:pull/18025 PR: https://git.openjdk.org/jdk/pull/18025
Re: RFR: 8327704: Update nsk/jdi tests to use driver instead of othervm [v3]
On Sat, 9 Mar 2024 05:27:43 GMT, Leonid Mesnik wrote: >> vmtestbase nsk/jdi tests run 2 processes: debugger and debugee. >> There is not need to start debugger in the separate process for each test. >> Also, no need to run it with "-Xcomp" because the main goal is to test >> debugee behavior with different VM flags. >> This fix updates tests to run debugger as driver to optimize execution time. >> The fix also eliminates System.exit() which is not compatible with >> driver/agentvm mode and update shared classes to correctly add classpath >> when running debugee. >> There were few tests which still execute using othervm mode. They require >> some specific classpath/settings. >> Tested by running all tests, with '-Xcomp' and virtual thread test factory. > > Leonid Mesnik has updated the pull request incrementally with one additional > commit since the last revision: > > reverted failing tests. That's a more generic question. The driver and main (default mode) actions might be executed in 2 modes: agentvm or othervm. So each driver (or main) action might reuse jvm or start a new one each time. This is controlled by -agetnvm flag in jtreg. While othervm mode in tests is used for tests that couldn't be executed in agentvm mode (use some specific VM options). We don't force /othrevm option in tests because something might be broken and affect other tests execution. (Really it means that every test that doesn't start new process should be othrevm). Each time when we run jtreg in agentvm we allow tests to reuse the same JVM and taking risks of possible incosistency of the future tests execution. So if test manages to break some part of JVM/JDK (it might be thread state, compile, gc, debugger,classoader, java.util or javac) we have a risk that next tests have jdk in inconsistent state. I think that the debugger might be treated as any other component. If we need more isolation then it is needed to use othervm mode for jtreg. And using of agentvm/othervm mode for VM testing might be separate topic. I can do more testing in different modes to see if we have any new failures after update. - PR Comment: https://git.openjdk.org/jdk/pull/18174#issuecomment-1998190351
Re: RFR: 8325187: JVMTI GetThreadState says virtual thread is JVMTI_THREAD_STATE_INTERRUPTED when it no longer is [v10]
> Please, review this fix correcting the JVMTI `RawMonitorWait()` > implementation. > The `RawMonitorWait()` is using the the `jt->is_interrupted(true)` to update > the interrupt status of the interrupted waiting thread. The issue is that > when it calls `jt->is_interrupted(true)` to fetch and clear the `interrupt > status` of the virtual thread, this information is not propagated to the > `java.lang.Thread` instance. > In the `VirtualThread` class implementation the `interrupt status` for > virtual thread is stored for both virtual and carrier threads. It is because > the implementation of object monitors for virtual threads is based on pinning > virtual threads, and so, always operates on carrier thread. The fix is to > clear the interrupt status for both virtual and carrier `java.lang.Thread` > instances. > > Testing: > - tested with new test > `hotspot/jtreg/serviceability/jvmti/vthread/InterruptRawMonitor` which is > passed with the fix and failed without it > - ran mach5 tiers 1-6 Serguei Spitsyn has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 11 additional commits since the last revision: - Merge - Merge - review: dropped the catch of InterruptedException in Object.wait - optimize holding the interruptLock in JavaThread::is_interrupted - reordered JavaThread::is_interrupted code to use lock when needed only - removed trailing spaces in javaClasses.cpp and libInterruptRawMonitor.cpp - review: added ObjectLocker of interruptLock for sync - review: improved sync in new test InterruptRawMonitor - review: addressed a couple of comments on new test - fix trailing space in libInterruptRawMonitor.cpp - ... and 1 more: https://git.openjdk.org/jdk/compare/86a4542e...b13d9c51 - Changes: - all: https://git.openjdk.org/jdk/pull/18093/files - new: https://git.openjdk.org/jdk/pull/18093/files/df3b6383..b13d9c51 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18093=09 - incr: https://webrevs.openjdk.org/?repo=jdk=18093=08-09 Stats: 16508 lines in 443 files changed: 9214 ins; 5600 del; 1694 mod Patch: https://git.openjdk.org/jdk/pull/18093.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18093/head:pull/18093 PR: https://git.openjdk.org/jdk/pull/18093
Re: RFR: 8296244: Alternate implementation of user-based authorization Subject APIs that doesn’t depend on Security Manager APIs [v6]
On Wed, 13 Mar 2024 19:53:40 GMT, Sean Mullan wrote: >> Weijun Wang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> revert changes to MBeanServerFileAccessController.java > > test/jdk/javax/security/auth/Subject/CallAsWithScopedValue.java line 29: > >> 27: * @enablePreview >> 28: * @summary Implement Subject.current and Subject.callAs using scoped >> values. >> 29: * Need @enablePreview to use StructuredTaskScope. > > jtreg throws a `ParseException` because I think it tries to interpret it as > an `@enablePreview` action. Suggest enclosing it in double quotes. Yes, I noticed that. Thanks. > test/jdk/javax/security/auth/Subject/CallAsWithScopedValue.java line 55: > >> 53: Subject.callAs(subject, () -> check(0, Subject.current(), >> "Duke")); >> 54: >> 55: // Observable in the same thread in ACC mode, but not in the SV >> mode > > Should this comment actually say "Observable in a new platform thread in ACC > mode, but not in the SV mode". Yes. - PR Review Comment: https://git.openjdk.org/jdk/pull/17472#discussion_r1525308150 PR Review Comment: https://git.openjdk.org/jdk/pull/17472#discussion_r1525308836
Integrated: JDK-8326898: NSK tests should listen on loopback addresses only
On Thu, 29 Feb 2024 01:50:02 GMT, Alex Menkov wrote: > Many NSK tests create socket channel for test/target interprocess > communication. > The change updates server side to listen only on loopback interface. > > Testing - all tests that use then functionality: > - test/hotspot/jtreg/vmTestbase/nsk/jdb, > test/hotspot/jtreg/vmTestbase/nsk/jdi, > test/hotspot/jtreg/vmTestbase/nsk/jdwp, > test/hotspot/jtreg/vmTestbase/nsk/aod, > test/hotspot/jtreg/vmTestbase/nsk/jvmti/AttachOnDemand This pull request has now been integrated. Changeset: 2482a505 Author:Alex Menkov URL: https://git.openjdk.org/jdk/commit/2482a505e5c898cc6365aa4fb8ca3e8b758b3a97 Stats: 22 lines in 6 files changed: 2 ins; 0 del; 20 mod 8326898: NSK tests should listen on loopback addresses only Reviewed-by: sspitsyn, cjplummer, dholmes, lmesnik - PR: https://git.openjdk.org/jdk/pull/18053
RFD: Can we remove per-thread compiler stats now?
Hello, Per-thread compiler performance counters were removed in JDK-8134607 [1] (JDK 9). The corresponding sun.management API was marked @Deprecated since it no longer returns any useful counter data. Has time come to remove this API now? My understanding is that since sun.management is an internal package this should be low risk. But this being JMX-related, is there maybe some concern to this being used in some remote/RMI scenario? If not, I'd like to propose a PR to remove the following: * Class sun.management.CompilerThreadStat * Nested class sun.management.HotspotCompilation.CompilerThreadInfo * Field sun.management.HotspotCompilation.threads and its initialization * Method sun.management.HotspotCompilation.getCompilerThreadStats() and its definition in HotspotCompilationMBean Let me know what you think Cheers, Eirik :-) [1] https://bugs.openjdk.org/browse/JDK-8134607
Re: RFR: 8326666: Remove the Java Management Extension (JMX) Subject Delegation feature [v12]
On Thu, 14 Mar 2024 12:23:09 GMT, Kevin Walls wrote:
>> The deprecated Subject Delegation feature in JMX will be removed.
>>
>> This was marked in JDK 21 as deprecated for removal (JDK-8298966).
>
> Kevin Walls has updated the pull request incrementally with one additional
> commit since the last revision:
>
> RMIConnectionImpl_Stub also should explicity inherit the unchecked UOE.
The spec updates are good with me. Caught one formatting nit in the
`RMIConnection` class spec.
src/java.management.rmi/share/classes/javax/management/remote/rmi/RMIConnection.java
line 85:
> 83: *
> 84: * JMX Subject Delegation has been removed. All methods that take a
> 85: * delegationSubject parameter will throw UnsupportedOperationException if
Suggestion:
* delegation subject parameter will throw {@code
UnsupportedOperationException} if
-
Marked as reviewed by mchung (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/18025#pullrequestreview-1937202285
PR Review Comment: https://git.openjdk.org/jdk/pull/18025#discussion_r1525173122
Re: RFR: JDK-8327769: jcmd GC.heap_dump without options should write to location given by -XX:HeapDumpPath, if set [v4]
On Thu, 14 Mar 2024 13:43:09 GMT, Matthias Baesken wrote:
>> Currently jcmd command GC.heap_dump only works with an additionally provided
>> file name.
>> Syntax : GC.heap_dump [options]
>>
>> In case the JVM has the XX - flag HeapDumpPath set, we should support an
>> additional mode where the is optional.
>> In case the filename is NOT set, we take the HeapDumpPath (file or
>> directory);
>>
>> new syntax :
>> GC.heap_dump [options] .. has precedence over second option
>> GC.heap_dump [options] …in case -XX: HeapDumpPath=p is set
>>
>> This would be a simplification e.g. for support cases where a filename or
>> directory is set at JVM startup with -XX: HeapDumpPath=p and writing to the
>> path is intended/recommended for usage also in the jcmd case.
>
> Matthias Baesken has updated the pull request incrementally with one
> additional commit since the last revision:
>
> add test HeapDumpJcmdPresetPathTest
Changes requested by cjplummer (Reviewer).
src/hotspot/share/runtime/globals.hpp line 565:
> 563: "triggered by jcmd GC.heap_dump without specifying a path, "
>\
> 564: "the path (filename or directory) of the dump file "
>\
> 565: "(defaults to java_pid.hprof in the working directory)")
>\
This incorrectly leads one to conclude that if HeapDumpPath is not specified
and GC.heap_dump is used without specifying a path, the default will be
java_pid.hprof in the working directory. That's not the case. The jcmd
will produce an error because it requires that either HeapDumpPath be specified
or a filename be specified as a jcmd argument (I'm not sure why the jcmd does
not default to java_pid.hprof)
Also, if you are cleaning up this text, I would suggest changing "is on" to "is
enabled". Same for HeapDumpGzipLevel below.
src/hotspot/share/services/diagnosticCommand.cpp line 475:
> 473: HeapDumpDCmd::HeapDumpDCmd(outputStream* output, bool heap) :
> 474:DCmdWithParser(output, heap),
> 475: _filename("filename","Name of the dump file", "STRING", false,
> "-XX:HeapDumpPath"),
I can't say I like using the "default" in this manner, but I understand why it
was done. I wish there was a better way. I ran into a similar problem with
Compiler.perfmap in #17359, but this case is a bit worse. For Compiler.perfmap
the default had to be specified as `/tmp/perf-.map`, but at least it
somewhat resembled the actual default.
Maybe it would help if you used something a bit more descriptive, like "The
path specified by "-XX:HeapDumpPath".
You also should update jcmd.l to describe this argument much like I did for
Compiler.perfmap. You especially need to call out that if -XX:HeapDumpPath is
not used then the "filename" argument must be speified.
-
PR Review: https://git.openjdk.org/jdk/pull/18190#pullrequestreview-1937265589
PR Review Comment: https://git.openjdk.org/jdk/pull/18190#discussion_r1525211803
PR Review Comment: https://git.openjdk.org/jdk/pull/18190#discussion_r1525223292
Integrated: 8328135: javax/management/remote/mandatory/loading/MissingClassTest.java fails on libgraal
On Thu, 14 Mar 2024 10:46:10 GMT, Doug Simon wrote: > This PR increases a timeout in `MissingClassTest.java` to handle slight > slower compilation on a fastdebug build when using `-Xcomp`. > Testing on mach5 shows that the increase from 60 s to 90 s resolves the > timeouts. This pull request has now been integrated. Changeset: e6a8fdd8 Author:Doug Simon URL: https://git.openjdk.org/jdk/commit/e6a8fdd82c2b97f7ae74dfe8fbd3402718c9161c Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod 8328135: javax/management/remote/mandatory/loading/MissingClassTest.java fails on libgraal Reviewed-by: kevinw, never - PR: https://git.openjdk.org/jdk/pull/18297
Re: RFR: 8328135: javax/management/remote/mandatory/loading/MissingClassTest.java fails on libgraal
On Thu, 14 Mar 2024 10:46:10 GMT, Doug Simon wrote: > This PR increases a timeout in `MissingClassTest.java` to handle slight > slower compilation on a fastdebug build when using `-Xcomp`. > Testing on mach5 shows that the increase from 60 s to 90 s resolves the > timeouts. Thanks for the reviews. - PR Comment: https://git.openjdk.org/jdk/pull/18297#issuecomment-1997817640
Re: RFR: 8328135: javax/management/remote/mandatory/loading/MissingClassTest.java fails on libgraal
On Thu, 14 Mar 2024 10:46:10 GMT, Doug Simon wrote: > This PR increases a timeout in `MissingClassTest.java` to handle slight > slower compilation on a fastdebug build when using `-Xcomp`. > Testing on mach5 shows that the increase from 60 s to 90 s resolves the > timeouts. Marked as reviewed by never (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/18297#pullrequestreview-1937156027
Re: RFR: 8328135: javax/management/remote/mandatory/loading/MissingClassTest.java fails on libgraal
On Thu, 14 Mar 2024 10:46:10 GMT, Doug Simon wrote: > This PR increases a timeout in `MissingClassTest.java` to handle slight > slower compilation on a fastdebug build when using `-Xcomp`. > Testing on mach5 shows that the increase from 60 s to 90 s resolves the > timeouts. Here we are again. I see in a good run it would print "known=50; lost=100" if we made it. So in the failing test runs it has only had time to lose e.g. 89 or 99 notifications. - Marked as reviewed by kevinw (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18297#pullrequestreview-1936981847
Re: RFR: 8327990: [macosx-aarch64] JFR enters VM without WXWrite
On Tue, 12 Mar 2024 15:05:00 GMT, Richard Reingruber wrote: > This pr changes `JfrJvmtiAgent::retransform_classes()` and `jfr_set_enabled` > to switch to `WXWrite` before transitioning to the vm. > > Testing: > make test TEST=jdk/jfr/event/runtime/TestClassLoadEvent.java > TEST_VM_OPTS=-XX:+AssertWXAtThreadSync > make test TEST=compiler/intrinsics/klass/CastNullCheckDroppingsTest.java > TEST_VM_OPTS=-XX:+AssertWXAtThreadSync > > More tests are pending. I noticed a few more asserts (assert(_wx_state == expected) failed: wrong state) in the jfr area (jdk tier3 jfr tests). E.g. V [libjvm.dylib+0x8a5d94] JavaThread::check_for_valid_safepoint_state()+0x0 V [libjvm.dylib+0x3e95b4] ThreadStateTransition::transition_from_native(JavaThread*, JavaThreadState, bool)+0x174 V [libjvm.dylib+0x3e93d0] ThreadInVMfromNative::ThreadInVMfromNative(JavaThread*)+0x70 V [libjvm.dylib+0x91a578] JfrRecorderService::emit_leakprofiler_events(long long, bool, bool)+0xcc and V [libjvm.dylib+0x8a5d94] JavaThread::check_for_valid_safepoint_state()+0x0 V [libjvm.dylib+0x3e95b4] ThreadStateTransition::transition_from_native(JavaThread*, JavaThreadState, bool)+0x174 V [libjvm.dylib+0x3e93d0] ThreadInVMfromNative::ThreadInVMfromNative(JavaThread*)+0x70 V [libjvm.dylib+0x8d7f74] JfrJavaEventWriter::flush(_jobject*, int, int, JavaThread*)+0xf8 j jdk.jfr.internal.JVM.flush(Ljdk/jfr/internal/event/EventWriter;II)V+0 jdk.jfr@23-internal j jdk.jfr.internal.event.EventWriter.flush(II)V+3 jdk.jfr@23-internal - PR Comment: https://git.openjdk.org/jdk/pull/18238#issuecomment-1997635467
Re: RFR: JDK-8327769: jcmd GC.heap_dump without options should write to location given by -XX:HeapDumpPath, if set [v4]
On Thu, 14 Mar 2024 13:43:09 GMT, Matthias Baesken wrote: >> Currently jcmd command GC.heap_dump only works with an additionally provided >> file name. >> Syntax : GC.heap_dump [options] >> >> In case the JVM has the XX - flag HeapDumpPath set, we should support an >> additional mode where the is optional. >> In case the filename is NOT set, we take the HeapDumpPath (file or >> directory); >> >> new syntax : >> GC.heap_dump [options] .. has precedence over second option >> GC.heap_dump [options] …in case -XX: HeapDumpPath=p is set >> >> This would be a simplification e.g. for support cases where a filename or >> directory is set at JVM startup with -XX: HeapDumpPath=p and writing to the >> path is intended/recommended for usage also in the jcmd case. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > add test HeapDumpJcmdPresetPathTest This is really helpful from a support point of view. - Marked as reviewed by asteiner (Author). PR Review: https://git.openjdk.org/jdk/pull/18190#pullrequestreview-1936916215
Re: RFR: JDK-8327769: jcmd GC.heap_dump without options should write to location given by -XX:HeapDumpPath, if set [v4]
On Thu, 14 Mar 2024 13:43:09 GMT, Matthias Baesken wrote: >> Currently jcmd command GC.heap_dump only works with an additionally provided >> file name. >> Syntax : GC.heap_dump [options] >> >> In case the JVM has the XX - flag HeapDumpPath set, we should support an >> additional mode where the is optional. >> In case the filename is NOT set, we take the HeapDumpPath (file or >> directory); >> >> new syntax : >> GC.heap_dump [options] .. has precedence over second option >> GC.heap_dump [options] …in case -XX: HeapDumpPath=p is set >> >> This would be a simplification e.g. for support cases where a filename or >> directory is set at JVM startup with -XX: HeapDumpPath=p and writing to the >> path is intended/recommended for usage also in the jcmd case. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > add test HeapDumpJcmdPresetPathTest Hi Kevin, > (and btw regarding your comment on a test, yes I agree there should be a > separate test or at least an adjustment/addition to the existing tests) I added a test HeapDumpJcmdPresetPathTest.java . - PR Comment: https://git.openjdk.org/jdk/pull/18190#issuecomment-1997514632
Re: RFR: JDK-8327769: jcmd GC.heap_dump without options should write to location given by -XX:HeapDumpPath, if set [v4]
> Currently jcmd command GC.heap_dump only works with an additionally provided > file name. > Syntax : GC.heap_dump [options] > > In case the JVM has the XX - flag HeapDumpPath set, we should support an > additional mode where the is optional. > In case the filename is NOT set, we take the HeapDumpPath (file or directory); > > new syntax : > GC.heap_dump [options] .. has precedence over second option > GC.heap_dump [options] …in case -XX: HeapDumpPath=p is set > > This would be a simplification e.g. for support cases where a filename or > directory is set at JVM startup with -XX: HeapDumpPath=p and writing to the > path is intended/recommended for usage also in the jcmd case. Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: add test HeapDumpJcmdPresetPathTest - Changes: - all: https://git.openjdk.org/jdk/pull/18190/files - new: https://git.openjdk.org/jdk/pull/18190/files/7420fb47..61165f55 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18190=03 - incr: https://webrevs.openjdk.org/?repo=jdk=18190=02-03 Stats: 47 lines in 1 file changed: 47 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/18190.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18190/head:pull/18190 PR: https://git.openjdk.org/jdk/pull/18190
Re: RFR: 8326666: Remove the Java Management Extension (JMX) Subject Delegation feature [v5]
On Mon, 11 Mar 2024 15:19:08 GMT, Daniel Fuchs wrote: >>> Is there any value in keeping `SubjectDelegationPermission` after this >>> change? If so, I would mark that API deprecated for removal, so that it can >>> be removed in the next release or two. >> >> No, nothing uses SubjectDelegationPermission. That can be deprecated. >> >>> Are there remaining tests that test the JMX fine grained permissions >>> feature w/o depending on subject delegation? >> >> Yes there are some tests other than those changed here, which use >> MBeanPermission in policies and become irrelevant post-SM. > >> Is there any value in keeping `SubjectDelegationPermission` after this >> change? If so, I would mark that API deprecated for removal, so that it can >> be removed in the next release or two. > > No issue with deprecation. I guess it can be removed once > `java.security.Policy` is removed? Thanks @dfuch for the comments and review. 8-) - PR Comment: https://git.openjdk.org/jdk/pull/18025#issuecomment-1997329701
Integrated: 8309271: A way to align already compiled methods with compiler directives
On Wed, 24 May 2023 00:38:27 GMT, Dmitry Chuyko wrote: > Compiler Control (https://openjdk.org/jeps/165) provides method-context > dependent control of the JVM compilers (C1 and C2). The active directive > stack is built from the directive files passed with the > `-XX:CompilerDirectivesFile` diagnostic command-line option and the > Compiler.add_directives diagnostic command. It is also possible to clear all > directives or remove the top from the stack. > > A matching directive will be applied at method compilation time when such > compilation is started. If directives are added or changed, but compilation > does not start, then the state of compiled methods doesn't correspond to the > rules. This is not an error, and it happens in long running applications when > directives are added or removed after compilation of methods that could be > matched. For example, the user decides that C2 compilation needs to be > disabled for some method due to a compiler bug, issues such a directive but > this does not affect the application behavior. In such case, the target > application needs to be restarted, and such an operation can have high costs > and risks. Another goal is testing/debugging compilers. > > It would be convenient to optionally reconcile at least existing matching > nmethods to the current stack of compiler directives (so bypass inlined > methods). > > Natural way to eliminate the discrepancy between the result of compilation > and the broken rule is to discard the compilation result, i.e. > deoptimization. Prior to that we can try to re-compile the method letting > compile broker to perform it taking new directives stack into account. > Re-compilation helps to prevent hot methods from execution in the interpreter. > > A new flag `-r` has beed introduced for some directives related to compile > commands: `Compiler.add_directives`, `Compiler.remove_directives`, > `Compiler.clear_directives`. The default behavior has not changed (no flag). > If the new flag is present, the command scans already compiled methods and > puts methods that have any active non-default matching compiler directives to > re-compilation if possible, otherwise marks them for deoptimization. There is > currently no distinction which directives are found. In particular, this > means that if there are rules for inlining into some method, it will be > refreshed. On the other hand, if there are rules for a method and it was > inlined, top-level methods won't be refreshed, but this can be achieved by > having rules for them. > > In addition, a new diagnostic command `Compiler.replace_directives`, has been > added for ... This pull request has now been integrated. Changeset: c879627d Author:Dmitry Chuyko URL: https://git.openjdk.org/jdk/commit/c879627dbd7e9295d44f19ef237edb5de10805d5 Stats: 381 lines in 15 files changed: 348 ins; 3 del; 30 mod 8309271: A way to align already compiled methods with compiler directives Reviewed-by: apangin, sspitsyn, tholenstein - PR: https://git.openjdk.org/jdk/pull/14111
RFR: 8328157: Move C[XX]FLAGS_JDK[LIB/EXE] to JdkNativeCompilation.gmk
We are setting one of the flags `CFLAGS_JDKLIB`, `CXXFLAGS_JDKLIB`, `CFLAGS_JDKEXE` or `CXXFLAGS_JDKEXE` to `CFLAGS` or `CXXFLAGS`, respectively, in basically all calls to `SetupJdkLibrary` and `SetupJdkExecutable`. These flag variables contain a lot of duplication. The first step towards bringing some sanity to this mess is to move the setting of these variables into `SetupJdkLibrary/SetupJdkExecutable`. In a few places these standard flags are not set, partially or fullly. This is handled by the new arguments `DEFAULT_CFLAGS := false` (to disable the entire setting) and `C[XX]FLAGS_FILTER_OUT` (which excludes some specific flag) to `SetupJdkLibrary/SetupJdkExecutable`. - Commit messages: - 8328157: Move C[XX]FLAGS_JDK[LIB/EXE] to JdkNativeCompilation.gmk Changes: https://git.openjdk.org/jdk/pull/18301/files Webrev: https://webrevs.openjdk.org/?repo=jdk=18301=00 Issue: https://bugs.openjdk.org/browse/JDK-8328157 Stats: 170 lines in 31 files changed: 45 ins; 66 del; 59 mod Patch: https://git.openjdk.org/jdk/pull/18301.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18301/head:pull/18301 PR: https://git.openjdk.org/jdk/pull/18301
Re: RFR: 8328157: Move C[XX]FLAGS_JDK[LIB/EXE] to JdkNativeCompilation.gmk
On Thu, 14 Mar 2024 12:36:05 GMT, Magnus Ihse Bursie wrote: > We are setting one of the flags `CFLAGS_JDKLIB`, `CXXFLAGS_JDKLIB`, > `CFLAGS_JDKEXE` or `CXXFLAGS_JDKEXE` to `CFLAGS` or `CXXFLAGS`, respectively, > in basically all calls to `SetupJdkLibrary` and `SetupJdkExecutable`. > > These flag variables contain a lot of duplication. > > The first step towards bringing some sanity to this mess is to move the > setting of these variables into `SetupJdkLibrary/SetupJdkExecutable`. > > In a few places these standard flags are not set, partially or fullly. This > is handled by the new arguments `DEFAULT_CFLAGS := false` (to disable the > entire setting) and `C[XX]FLAGS_FILTER_OUT` (which excludes some specific > flag) to `SetupJdkLibrary/SetupJdkExecutable`. I have verified with `COMPARE_BUILD` that there is no significant difference in the build result with or without this patch on Oracle default CI platforms (linux x64/aarch64, macos x64/aarch64, windows x64). - PR Comment: https://git.openjdk.org/jdk/pull/18301#issuecomment-1997359912
Re: RFR: 8326666: Remove the Java Management Extension (JMX) Subject Delegation feature [v12]
> The deprecated Subject Delegation feature in JMX will be removed. > > This was marked in JDK 21 as deprecated for removal (JDK-8298966). Kevin Walls has updated the pull request incrementally with one additional commit since the last revision: RMIConnectionImpl_Stub also should explicity inherit the unchecked UOE. - Changes: - all: https://git.openjdk.org/jdk/pull/18025/files - new: https://git.openjdk.org/jdk/pull/18025/files/418b635c..91ec015f Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18025=11 - incr: https://webrevs.openjdk.org/?repo=jdk=18025=10-11 Stats: 25 lines in 1 file changed: 24 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/18025.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18025/head:pull/18025 PR: https://git.openjdk.org/jdk/pull/18025
Re: RFR: 8326666: Remove the Java Management Extension (JMX) Subject Delegation feature [v11]
On Thu, 14 Mar 2024 11:53:11 GMT, Kevin Walls wrote: >> The deprecated Subject Delegation feature in JMX will be removed. >> >> This was marked in JDK 21 as deprecated for removal (JDK-8298966). > > Kevin Walls has updated the pull request incrementally with two additional > commits since the last revision: > > - Clarify JMXConnector equivalence comment. > - RMIConnectionImpl needs to explicity inherit the unchecked UOE. Marked as reviewed by dfuchs (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/18025#pullrequestreview-1936454653
Re: RFR: 8326666: Remove the Java Management Extension (JMX) Subject Delegation feature [v11]
> The deprecated Subject Delegation feature in JMX will be removed. > > This was marked in JDK 21 as deprecated for removal (JDK-8298966). Kevin Walls has updated the pull request incrementally with two additional commits since the last revision: - Clarify JMXConnector equivalence comment. - RMIConnectionImpl needs to explicity inherit the unchecked UOE. - Changes: - all: https://git.openjdk.org/jdk/pull/18025/files - new: https://git.openjdk.org/jdk/pull/18025/files/a3e09e90..418b635c Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18025=10 - incr: https://webrevs.openjdk.org/?repo=jdk=18025=09-10 Stats: 32 lines in 2 files changed: 31 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/18025.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18025/head:pull/18025 PR: https://git.openjdk.org/jdk/pull/18025
RFR: 8328146: Set LIBCXX automatically
We are adding LIBCXX to LIBS in calls to SetupJdkLibrary whenever LINK_TYPE is C++. We should do this automatically in SetupJdkLibrary for C++ linking. I also removed the superfluous `-lc` from some places where it had been added. - Commit messages: - 8328146: Set LIBCXX automatically Changes: https://git.openjdk.org/jdk/pull/18298/files Webrev: https://webrevs.openjdk.org/?repo=jdk=18298=00 Issue: https://bugs.openjdk.org/browse/JDK-8328146 Stats: 27 lines in 10 files changed: 8 ins; 4 del; 15 mod Patch: https://git.openjdk.org/jdk/pull/18298.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18298/head:pull/18298 PR: https://git.openjdk.org/jdk/pull/18298
RFR: 8328135: javax/management/remote/mandatory/loading/MissingClassTest.java fails on libgraal
This PR increases a timeout in `MissingClassTest.java` to handle slight slower compilation on a fastdebug build when using `-Xcomp`. Testing on mach5 shows that the increase from 60 s to 90 s resolves the timeouts. - Commit messages: - increase timeout in loop waiting for listeners to receive notifs Changes: https://git.openjdk.org/jdk/pull/18297/files Webrev: https://webrevs.openjdk.org/?repo=jdk=18297=00 Issue: https://bugs.openjdk.org/browse/JDK-8328135 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/18297.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18297/head:pull/18297 PR: https://git.openjdk.org/jdk/pull/18297
Re: RFR: 8309271: A way to align already compiled methods with compiler directives [v32]
> Compiler Control (https://openjdk.org/jeps/165) provides method-context > dependent control of the JVM compilers (C1 and C2). The active directive > stack is built from the directive files passed with the > `-XX:CompilerDirectivesFile` diagnostic command-line option and the > Compiler.add_directives diagnostic command. It is also possible to clear all > directives or remove the top from the stack. > > A matching directive will be applied at method compilation time when such > compilation is started. If directives are added or changed, but compilation > does not start, then the state of compiled methods doesn't correspond to the > rules. This is not an error, and it happens in long running applications when > directives are added or removed after compilation of methods that could be > matched. For example, the user decides that C2 compilation needs to be > disabled for some method due to a compiler bug, issues such a directive but > this does not affect the application behavior. In such case, the target > application needs to be restarted, and such an operation can have high costs > and risks. Another goal is testing/debugging compilers. > > It would be convenient to optionally reconcile at least existing matching > nmethods to the current stack of compiler directives (so bypass inlined > methods). > > Natural way to eliminate the discrepancy between the result of compilation > and the broken rule is to discard the compilation result, i.e. > deoptimization. Prior to that we can try to re-compile the method letting > compile broker to perform it taking new directives stack into account. > Re-compilation helps to prevent hot methods from execution in the interpreter. > > A new flag `-r` has beed introduced for some directives related to compile > commands: `Compiler.add_directives`, `Compiler.remove_directives`, > `Compiler.clear_directives`. The default behavior has not changed (no flag). > If the new flag is present, the command scans already compiled methods and > puts methods that have any active non-default matching compiler directives to > re-compilation if possible, otherwise marks them for deoptimization. There is > currently no distinction which directives are found. In particular, this > means that if there are rules for inlining into some method, it will be > refreshed. On the other hand, if there are rules for a method and it was > inlined, top-level methods won't be refreshed, but this can be achieved by > having rules for them. > > In addition, a new diagnostic command `Compiler.replace_directives`, has been > added for ... Dmitry Chuyko has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 50 commits: - Merge branch 'openjdk:master' into compiler-directives-force-update - No dots in -r descriptions - Merge branch 'openjdk:master' into compiler-directives-force-update - Resolved master conflicts - Merge branch 'openjdk:master' into compiler-directives-force-update - Merge branch 'openjdk:master' into compiler-directives-force-update - Merge branch 'openjdk:master' into compiler-directives-force-update - Merge branch 'openjdk:master' into compiler-directives-force-update - Merge branch 'openjdk:master' into compiler-directives-force-update - Merge branch 'openjdk:master' into compiler-directives-force-update - ... and 40 more: https://git.openjdk.org/jdk/compare/49ce85fa...eb4ed2ea - Changes: https://git.openjdk.org/jdk/pull/14111/files Webrev: https://webrevs.openjdk.org/?repo=jdk=14111=31 Stats: 381 lines in 15 files changed: 348 ins; 3 del; 30 mod Patch: https://git.openjdk.org/jdk/pull/14111.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14111/head:pull/14111 PR: https://git.openjdk.org/jdk/pull/14111
Re: RFR: 8327990: [macosx-aarch64] JFR enters VM without WXWrite
On Wed, 13 Mar 2024 16:40:33 GMT, Richard Reingruber wrote: > @MBaesken found 2 more locations in jvmti that need switching to `WXWrite` > > JvmtiExport::get_jvmti_interface GetCarrierThread > > Both use `ThreadInVMfromNative`. Should we address those 2 more findings in this PR ? Or open a separate JBS issue ? btw those were the jtreg tests triggering the 2 additional findings / asserts runtime/Thread/AsyncExceptionOnMonitorEnter.java runtime/Thread/AsyncExceptionTest.java serviceability/jvmti/RedefineClasses/RedefineSharedClassJFR.java runtime/handshake/HandshakeDirectTest.java runtime/handshake/SuspendBlocked.java runtime/jni/terminatedThread/TestTerminatedThread.java runtime/lockStack/TestStackWalk.java serviceability/jvmti/vthread/GetThreadState/GetThreadStateTest.java#default serviceability/jvmti/vthread/GetThreadState/GetThreadStateTest.java#no-vmcontinuations serviceability/jvmti/vthread/GetThreadStateMountedTest/GetThreadStateMountedTest.java serviceability/jvmti/vthread/RawMonitorTest/RawMonitorTest.java serviceability/jvmti/vthread/SuspendWithInterruptLock/SuspendWithInterruptLock.java#default serviceability/jvmti/vthread/SuspendWithInterruptLock/SuspendWithInterruptLock.java#xint serviceability/jvmti/vthread/ThreadStateTest/ThreadStateTest.java serviceability/jvmti/vthread/WaitNotifySuspendedVThreadTest/WaitNotifySuspendedVThreadTest.java - PR Comment: https://git.openjdk.org/jdk/pull/18238#issuecomment-1996996689
Re: RFR: 8309271: A way to align already compiled methods with compiler directives [v31]
On Wed, 13 Mar 2024 17:31:32 GMT, Dmitry Chuyko wrote: >> Compiler Control (https://openjdk.org/jeps/165) provides method-context >> dependent control of the JVM compilers (C1 and C2). The active directive >> stack is built from the directive files passed with the >> `-XX:CompilerDirectivesFile` diagnostic command-line option and the >> Compiler.add_directives diagnostic command. It is also possible to clear all >> directives or remove the top from the stack. >> >> A matching directive will be applied at method compilation time when such >> compilation is started. If directives are added or changed, but compilation >> does not start, then the state of compiled methods doesn't correspond to the >> rules. This is not an error, and it happens in long running applications >> when directives are added or removed after compilation of methods that could >> be matched. For example, the user decides that C2 compilation needs to be >> disabled for some method due to a compiler bug, issues such a directive but >> this does not affect the application behavior. In such case, the target >> application needs to be restarted, and such an operation can have high costs >> and risks. Another goal is testing/debugging compilers. >> >> It would be convenient to optionally reconcile at least existing matching >> nmethods to the current stack of compiler directives (so bypass inlined >> methods). >> >> Natural way to eliminate the discrepancy between the result of compilation >> and the broken rule is to discard the compilation result, i.e. >> deoptimization. Prior to that we can try to re-compile the method letting >> compile broker to perform it taking new directives stack into account. >> Re-compilation helps to prevent hot methods from execution in the >> interpreter. >> >> A new flag `-r` has beed introduced for some directives related to compile >> commands: `Compiler.add_directives`, `Compiler.remove_directives`, >> `Compiler.clear_directives`. The default behavior has not changed (no flag). >> If the new flag is present, the command scans already compiled methods and >> puts methods that have any active non-default matching compiler directives >> to re-compilation if possible, otherwise marks them for deoptimization. >> There is currently no distinction which directives are found. In particular, >> this means that if there are rules for inlining into some method, it will be >> refreshed. On the other hand, if there are rules for a method and it was >> inlined, top-level methods won't be refreshed, but this can be achieved by >> having rules for them. >> >> In addition, a new diagnostic command `Compiler.replace_directives... > > Dmitry Chuyko has updated the pull request incrementally with one additional > commit since the last revision: > > No dots in -r descriptions Thank you, Seguei and Tobias. - PR Comment: https://git.openjdk.org/jdk/pull/14111#issuecomment-1996995661
Re: RFR: JDK-8327769: jcmd GC.heap_dump without options should write to location given by -XX:HeapDumpPath, if set [v3]
On Thu, 14 Mar 2024 09:13:03 GMT, Matthias Baesken wrote: >> Currently jcmd command GC.heap_dump only works with an additionally provided >> file name. >> Syntax : GC.heap_dump [options] >> >> In case the JVM has the XX - flag HeapDumpPath set, we should support an >> additional mode where the is optional. >> In case the filename is NOT set, we take the HeapDumpPath (file or >> directory); >> >> new syntax : >> GC.heap_dump [options] .. has precedence over second option >> GC.heap_dump [options] …in case -XX: HeapDumpPath=p is set >> >> This would be a simplification e.g. for support cases where a filename or >> directory is set at JVM startup with -XX: HeapDumpPath=p and writing to the >> path is intended/recommended for usage also in the jcmd case. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > Adjust text related to HeapDumpPath in globals.hpp Hi Kevin, > globals.hpp documents HeapDumpPath as relating to HeapDumpOnOutOfMemoryError > (so that will need changing if this change is happening). I adjusted the related text in globals.hpp . Please check. - PR Comment: https://git.openjdk.org/jdk/pull/18190#issuecomment-1996977356
Re: RFR: JDK-8327769: jcmd GC.heap_dump without options should write to location given by -XX:HeapDumpPath, if set [v3]
> Currently jcmd command GC.heap_dump only works with an additionally provided > file name. > Syntax : GC.heap_dump [options] > > In case the JVM has the XX - flag HeapDumpPath set, we should support an > additional mode where the is optional. > In case the filename is NOT set, we take the HeapDumpPath (file or directory); > > new syntax : > GC.heap_dump [options] .. has precedence over second option > GC.heap_dump [options] …in case -XX: HeapDumpPath=p is set > > This would be a simplification e.g. for support cases where a filename or > directory is set at JVM startup with -XX: HeapDumpPath=p and writing to the > path is intended/recommended for usage also in the jcmd case. Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: Adjust text related to HeapDumpPath in globals.hpp - Changes: - all: https://git.openjdk.org/jdk/pull/18190/files - new: https://git.openjdk.org/jdk/pull/18190/files/e335d9ee..7420fb47 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18190=02 - incr: https://webrevs.openjdk.org/?repo=jdk=18190=01-02 Stats: 4 lines in 1 file changed: 1 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/18190.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18190/head:pull/18190 PR: https://git.openjdk.org/jdk/pull/18190
Re: RFR: JDK-8327769: jcmd GC.heap_dump without options should write to location given by -XX:HeapDumpPath, if set [v2]
On Thu, 14 Mar 2024 02:39:28 GMT, Yi Yang wrote: > Looks reasonable, this is a harmless change, but the name > `dump_to_heapdump_path` looks too details(and somewhat strange) to me Do you maybe have a good suggestion for a new name ? - PR Comment: https://git.openjdk.org/jdk/pull/18190#issuecomment-1996891933
Re: RFR: 8309271: A way to align already compiled methods with compiler directives [v31]
On Wed, 13 Mar 2024 17:31:32 GMT, Dmitry Chuyko wrote: >> Compiler Control (https://openjdk.org/jeps/165) provides method-context >> dependent control of the JVM compilers (C1 and C2). The active directive >> stack is built from the directive files passed with the >> `-XX:CompilerDirectivesFile` diagnostic command-line option and the >> Compiler.add_directives diagnostic command. It is also possible to clear all >> directives or remove the top from the stack. >> >> A matching directive will be applied at method compilation time when such >> compilation is started. If directives are added or changed, but compilation >> does not start, then the state of compiled methods doesn't correspond to the >> rules. This is not an error, and it happens in long running applications >> when directives are added or removed after compilation of methods that could >> be matched. For example, the user decides that C2 compilation needs to be >> disabled for some method due to a compiler bug, issues such a directive but >> this does not affect the application behavior. In such case, the target >> application needs to be restarted, and such an operation can have high costs >> and risks. Another goal is testing/debugging compilers. >> >> It would be convenient to optionally reconcile at least existing matching >> nmethods to the current stack of compiler directives (so bypass inlined >> methods). >> >> Natural way to eliminate the discrepancy between the result of compilation >> and the broken rule is to discard the compilation result, i.e. >> deoptimization. Prior to that we can try to re-compile the method letting >> compile broker to perform it taking new directives stack into account. >> Re-compilation helps to prevent hot methods from execution in the >> interpreter. >> >> A new flag `-r` has beed introduced for some directives related to compile >> commands: `Compiler.add_directives`, `Compiler.remove_directives`, >> `Compiler.clear_directives`. The default behavior has not changed (no flag). >> If the new flag is present, the command scans already compiled methods and >> puts methods that have any active non-default matching compiler directives >> to re-compilation if possible, otherwise marks them for deoptimization. >> There is currently no distinction which directives are found. In particular, >> this means that if there are rules for inlining into some method, it will be >> refreshed. On the other hand, if there are rules for a method and it was >> inlined, top-level methods won't be refreshed, but this can be achieved by >> having rules for them. >> >> In addition, a new diagnostic command `Compiler.replace_directives... > > Dmitry Chuyko has updated the pull request incrementally with one additional > commit since the last revision: > > No dots in -r descriptions Looks good to me too (Compiler Team) - Marked as reviewed by tholenstein (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/14111#pullrequestreview-1936031298
Integrated: JDK-8327468: Do not restart close if errno is EINTR [macOS/linux]
On Fri, 8 Mar 2024 10:12:06 GMT, Matthias Baesken wrote: > There are a number of places remaining in the linux/macOS native JDK codebase > where we use the RESTARTABLE macro with close, but this is unwanted on > Linux/macOS. This pull request has now been integrated. Changeset: 481c866d Author:Matthias Baesken URL: https://git.openjdk.org/jdk/commit/481c866df87c693a90a1da20e131e5654b084ddd Stats: 6 lines in 2 files changed: 0 ins; 3 del; 3 mod 8327468: Do not restart close if errno is EINTR [macOS/linux] Reviewed-by: dholmes, sspitsyn - PR: https://git.openjdk.org/jdk/pull/18164
Re: RFR: JDK-8327468: Do not restart close if errno is EINTR [macOS/linux] [v2]
On Mon, 11 Mar 2024 08:50:09 GMT, Matthias Baesken wrote: >> There are a number of places remaining in the linux/macOS native JDK >> codebase where we use the RESTARTABLE macro with close, but this is unwanted >> on Linux/macOS. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > do not throw exception on close error Thanks for the reviews ! - PR Comment: https://git.openjdk.org/jdk/pull/18164#issuecomment-1996775638
