Re: RFR: 8275259: Add support for Java level DCmd
On Fri, 15 Oct 2021 01:54:16 GMT, Denghui Dong wrote: > Hi Thomas, > > > Hi, > > Interesting proposal. I have some questions. > > > > * Will hanging java user code block the attach listener thread? If yes, how > > would you solve that? > > In my current implementation, I use a simple timeout mechanism to solve this > problem. > > Create a Thread to run the command, and use > `java.util.concurrent.Future#get(long, java.util.concurrent.TimeUnit)` to get > the result. > > For more details please refer to Executor.java. > > > * Will this require changes to the jcmd client, or will this work with any > > jcmd client, up- and downward the JDK versions? (the nice thing about jcmd > > is that all the logic resides at the hotspot and I can use any client to > > talk to any hotspot) > > At present, I only extend the DCMD framework and do not modify the code of > jcmd. So it works with any jcmd client and jmx client, up and downward JDK > versions I think. > > > Cheers, Thomas > > P.S. I think it may be worthwhile to discuss this on the serviceability-dev > > mailing list first. > > Denghui Hi Denghui, thanks for your answers. I'm ambivalent here. I worry about the growing complexity of jcmd, and about unforeseen consequences if we open this door. OTOH I can see this as a useful feature. I'll wait for a consensus. Thanks, Thomas - PR: https://git.openjdk.java.net/jdk/pull/5938
Re: RFR: 8275259: Add support for Java level DCmd
On Thu, 14 Oct 2021 05:42:09 GMT, Denghui Dong wrote: > I proposed to extend DCmd to allow Java developers to customize their own > diagnostic commands last week. > > At present, I have implemented a preliminary version. > > In the current implementation, I provided some simple APIs in the Java layer > (under sun.management.cmd) for defining and registering commands. > > - Executable interface > Java diagnostic commands need to implement this interface, the interface > only contains a simple method: > > /** > * @param output the output when this executable is running > */ > void execute(PrintWriter output); > > > - Add two annotations (@Command and @Parameter) to describe the command meta > info > > - Use Factory API to register command, the following forms are supported > > @Command(name = "My.Echo", description = "Echo description") > class Echo implements Executable { > > @Parameter(name = "text", ordinal=0, isMandatory = true) > String text; > > @Parameter(name = "repeat", isMandatory = true, defaultValue = "1") > int repeat; > > @Override > public void execute(PrintWriter out) { > for (int i = 0 ; i < repeat; i++) { > out.println(text); > } > } > } > > Factory.register(Echo.class); > > > > Factory.register("My.Date", output -> { > output.println(new Date()); > }); > > > - When the command is running, the VM will call `Executor.executeCommand` to > execute the command. In the implementation of Executor, I introduced a simple > timeout mechanism to prevent the command channel from being blocked. > > At the VM layer, I extended the existing DCmd framework(implemented JavaDCmd > and JavaDCmdFactoryImpl) to be compatible with existing functions (jmx, help, > etc.). > > In terms of security, considering that the SecurityManager will be > deprecated, there are not too many considerations for the time being. > > Any input is appreciated. > > Thanks, > Denghui Hi Thomas, > Hi, > > Interesting proposal. I have some questions. > > * Will hanging java user code block the attach listener thread? If yes, how > would you solve that? In my current implementation, I use a simple timeout mechanism to solve this problem. Create a Thread to run the command, and use `java.util.concurrent.Future#get(long, java.util.concurrent.TimeUnit)` to get the result. For more details please refer to Executor.java. > * Will this require changes to the jcmd client, or will this work with any > jcmd client, up- and downward the JDK versions? (the nice thing about jcmd is > that all the logic resides at the hotspot and I can use any client to talk to > any hotspot) > At present, I only extend the DCMD framework and do not modify the code of jcmd. So it works with any jcmd client and jmx client, up and downward JDK versions I think. > Cheers, Thomas > > P.S. I think it may be worthwhile to discuss this on the serviceability-dev > mailing list first. Denghui Hi Erik, > To understand what is needed (namespacing, error handling, safety, lifecycle > etc.) and if this is actually useful for Java, I think you need to write 2-3 > commands for a few popular frameworks, for example Spring. > > If we can't come up with 5-10 commands where this functionality would solve > real problems users face, it's unlikely somebody will use it. It will just be > a maintenance burden in the JDK. Once an API is added, it's hard to remove. > > The scope of this seem to be JEP level. > > (JFR is not a valid use case, since we don't want to add a dependency on > annotation classes in the jdk.jcmd module). Thanks for the comments. I will investigate whether there is a scenario to apply this extension in the current mainstream framework. Denghui - PR: https://git.openjdk.java.net/jdk/pull/5938
Integrated: 8275240: Change nested classes in jdk.attach to static nested classes
On Tue, 12 Oct 2021 07:20:14 GMT, Andrey Turbanov wrote: > Non-static classes hold a link to their parent classes, which in many cases > can be avoided. > Similar cleanup in java.base - > [JDK-8261880](https://bugs.openjdk.java.net/browse/JDK-8261880) This pull request has now been integrated. Changeset: 21df412b Author:Andrey Turbanov Committer: Serguei Spitsyn URL: https://git.openjdk.java.net/jdk/commit/21df412bd9a02f0c3f351467951415141d920e03 Stats: 7 lines in 4 files changed: 0 ins; 2 del; 5 mod 8275240: Change nested classes in jdk.attach to static nested classes Reviewed-by: cjplummer, dholmes, sspitsyn - PR: https://git.openjdk.java.net/jdk/pull/5904
Re: RFR: 8266936: Add a finalization JFR event [v16]
On Thu, 14 Oct 2021 21:46:36 GMT, Coleen Phillimore wrote: >> Markus Grönlund has refreshed the contents of this pull request, and >> previous commits have been removed. The incremental views will show >> differences compared to the previous content of the PR. > > src/hotspot/share/jfr/support/jfrSymbolTable.hpp line 68: > >> 66: template class, >> typename, size_t> >> 67: friend class HashTableHost; >> 68: typedef HashTableHost> JfrSymbolTable> SymbolTable; > > Oh here it is. Since it's an enclosed type, can you rename it something > simpler like Symbols and the other Strings? Maybe :) - PR: https://git.openjdk.java.net/jdk/pull/4731
Re: RFR: 8266936: Add a finalization JFR event [v10]
On Thu, 14 Oct 2021 22:02:43 GMT, Coleen Phillimore wrote: >> Ok, I see - grow the table. >> >> I'm not sure if you need to ask ik->is_loader_alive() or not, but I think >> you do. The InstanceKlass is removed from your table during class unloading >> but before that during concurrent class unloading, the class might not be >> alive while you look at it and concurrent class unloading hasn't gotten >> around to removing it yet. Since you save classes regardless of CLD, you >> have to check if it's alive. See classLoaderDataGraph.cpp >> ClassLoaderDataGraphIterator for example. The CLDG_lock only keeps the >> graph from not getting modified, but the classes in it might be dead. > > That said, I don't see where you return an InstanceKlass in the table, which > would need this check. Not in class_unloading_do because this follows the > _unloading list. So there is already support for concurrent class unloading today in JFR, and the protocol is built around the CLDG_lock. If JFR holds it, concurrent class unloading cannot run. If GC holds it, JFR cannot inspect classes. That's why the table inspection is guarded via the CLDG_lock, for mutual exclusion to avoid this problem. I.e. if concurrent class unloading is in progress, JFR will not inspect the table. - PR: https://git.openjdk.java.net/jdk/pull/4731
Re: RFR: 8266936: Add a finalization JFR event [v10]
On Thu, 14 Oct 2021 21:58:42 GMT, Coleen Phillimore wrote: >> "Since you remove entries on unloading, I don't see any reason to have any >> concurrent cleanup." >> Thank you, that is true. The only concurrent work required will be to grow >> the table. >> >> "You do however need in the lookup code, some code that doesn't find the >> InstanceKlass if !ik->is_loader_alive()" >> >> A precondition is that the code doing the lookup hold the >> ClassLoaderDataGraph_lock or is at a safepoint. Is that still the case? >> Would not purge_unloaded() take out the InstanceKlass from the table, as >> part of unloading, before !ik->is_loader_alive() is published to the outside >> world? > > Ok, I see - grow the table. > > I'm not sure if you need to ask ik->is_loader_alive() or not, but I think you > do. The InstanceKlass is removed from your table during class unloading but > before that during concurrent class unloading, the class might not be alive > while you look at it and concurrent class unloading hasn't gotten around to > removing it yet. Since you save classes regardless of CLD, you have to check > if it's alive. See classLoaderDataGraph.cpp ClassLoaderDataGraphIterator for > example. The CLDG_lock only keeps the graph from not getting modified, but > the classes in it might be dead. That said, I don't see where you return an InstanceKlass in the table, which would need this check. Not in class_unloading_do because this follows the _unloading list. - PR: https://git.openjdk.java.net/jdk/pull/4731
Re: RFR: 8266936: Add a finalization JFR event [v10]
On Mon, 13 Sep 2021 10:54:18 GMT, Markus Grönlund wrote: >> src/hotspot/share/services/finalizerService.cpp line 44: >> >>> 42: _ik(ik), >>> 43: _objects_on_heap(0), >>> 44: _total_finalizers_run(0) {} >> >> Is this hashtable for every InstanceKlass that defines a finalizer? How >> many are there generally? Why couldn't this be a simple hashtable like >> ResourceHashtable (soon to be renamed) which you can write in only a few >> lines of code? > > This hashtable holds a FinalizerEntry for every InstanceKlass that provides a > non-empty finalizer method and have allocated at least one object. How many > there are in general depends on the application. A ResourceHashtable does not > have the concurrency property required, as lookups and inserts will happen as > part of object allocation. ok. >> src/hotspot/share/services/finalizerService.cpp line 331: >> >>> 329: } >>> 330: Thread* const thread = Thread::current(); >>> 331: // We use current size >> >> Since you remove entries on unloading, I don't see any reason to have any >> concurrent cleanup. >> You do however need in the lookup code, some code that doesn't find the >> InstanceKlass if !ik->is_loader_alive() > > "Since you remove entries on unloading, I don't see any reason to have any > concurrent cleanup." > Thank you, that is true. The only concurrent work required will be to grow > the table. > > "You do however need in the lookup code, some code that doesn't find the > InstanceKlass if !ik->is_loader_alive()" > > A precondition is that the code doing the lookup hold the > ClassLoaderDataGraph_lock or is at a safepoint. Is that still the case? Would > not purge_unloaded() take out the InstanceKlass from the table, as part of > unloading, before !ik->is_loader_alive() is published to the outside world? Ok, I see - grow the table. I'm not sure if you need to ask ik->is_loader_alive() or not, but I think you do. The InstanceKlass is removed from your table during class unloading but before that during concurrent class unloading, the class might not be alive while you look at it and concurrent class unloading hasn't gotten around to removing it yet. Since you save classes regardless of CLD, you have to check if it's alive. See classLoaderDataGraph.cpp ClassLoaderDataGraphIterator for example. The CLDG_lock only keeps the graph from not getting modified, but the classes in it might be dead. - PR: https://git.openjdk.java.net/jdk/pull/4731
Re: RFR: 8266936: Add a finalization JFR event [v16]
On Wed, 13 Oct 2021 18:03:25 GMT, Markus Grönlund wrote: >> Greetings, >> >> Object.finalize() was deprecated in JDK9. There is an ongoing effort to >> replace and mitigate Object.finalize() uses in the JDK libraries; please see >> https://bugs.openjdk.java.net/browse/JDK-8253568 for more information. >> >> We also like to assist users in replacing and mitigating uses in non-JDK >> code. >> >> Hence, this changeset adds a periodic JFR event to help identify which >> classes are overriding Object.finalize(). >> >> Thanks >> Markus > > Markus Grönlund has refreshed the contents of this pull request, and previous > commits have been removed. The incremental views will show differences > compared to the previous content of the PR. src/hotspot/share/jfr/support/jfrSymbolTable.cpp line 75: > 73: > 74: JfrSymbolTable::JfrSymbolTable() : > 75: _symbol_table(new SymbolTable(this)), I'm confused about which symbol table this is. Is this the same SymbolTable as classfile/symbolTable.cpp? that instance is assumed to be a singleton. Is this a different SymbolTable and can it have a different name (thought it was JfrSymbolTable). src/hotspot/share/jfr/support/jfrSymbolTable.hpp line 68: > 66: template class, > typename, size_t> > 67: friend class HashTableHost; > 68: typedef HashTableHost JfrSymbolTable> SymbolTable; Oh here it is. Since it's an enclosed type, can you rename it something simpler like Symbols and the other Strings? - PR: https://git.openjdk.java.net/jdk/pull/4731
Re: RFR: 8274687: JDWP deadlocks if some Java thread reaches wait in blockOnDebuggerSuspend [v4]
On Thu, 14 Oct 2021 07:44:36 GMT, Richard Reingruber wrote: >> Ok, so you need to hold threadLock from the time `blockOnDebuggerSuspend()` >> is done waiting on it until after `trackAppResume()` is done, and since >> `trackAppResume()` needs to grab the handlerLock, and you need to grab the >> handerLock before the threadLock, you need to jump through some lock >> grabbing/release hoops. However, you are in fact releasing the threadLock >> for a short time in `blockOnDebuggerSuspend()` after the wait completes. >> Doesn't this expose the resumee to potential suspending after the wait has >> completed and before `trackAppResume()` has been called? > >> Ok, so you need to hold threadLock from the time `blockOnDebuggerSuspend()` >> is done waiting on it until after `trackAppResume()` is done, and since >> `trackAppResume()` needs to grab the handlerLock, and you need to grab the >> handerLock before the threadLock, you need to jump through some lock >> grabbing/release hoops. > > That's correct. > >> However, you are in fact releasing the threadLock for a short time in >> `blockOnDebuggerSuspend()` after the wait completes. Doesn't this expose the >> resumee to potential suspending after the wait has completed and before >> `trackAppResume()` has been called? > > That's correct too. I wouldn't see an issue with it though. I think this is > even a preexisting condition. The current thread can lose the race grabbing > threadLock after it was notified to the debugger trying to suspend again (if > there wasn't the deadlock of course) and consequently suspendCount can > (again) be greater than 0 right after the wait. In that case we simply retry. Ok. So you just need to reacquire the threadLock before the `findThread()` call and before exiting the while loop, and hold it until after the `trackAppResume()` call. I guess it ok then. But this exiting of the handlerLock here and in `blockOnDebuggerSuspend()` is always going to arouse suspicion for anyone that reads the code. The first question will be if the lock does not need to be held here, why grab it in the first place? We know the answer is this lock ordering issue that turns up when `trackAppResume()` is later called, but this will be far from obvious to the reader. It might be possible to make this a bit clearer with some code restructuring, like maybe combining `blockOnDebuggerSuspend()` and `trackAppResume()` into one function (I'm not sure that this will actually help, but maybe something to consider), but at the very least we need some comments calling out why handlerLock is held in the first place and why it is ok to release it. - PR: https://git.openjdk.java.net/jdk/pull/5849
Re: RFR: 8249004: Reduce ThreadsListHandle overhead in relation to direct handshakes
On Sun, 4 Jul 2021 23:39:00 GMT, David Holmes wrote: >> A fix to reduce ThreadsListHandle overhead in relation to handshakes and >> we add sanity checks for ThreadsListHandles higher in the call stack. >> >> This fix was tested with Mach5 Tier[1-8]; Tier8 is still running. > > Hi Dan, > > I just updated the bug report. This really isn't addressing the reasons the > RFE was filed. > > David @dholmes-ora, @coleenp and @sspitsyn - I've finished reworking the patch and I'm finally happy with how it works. This version has passed Mach5 Tier[1-7] and has mostly finished Mach5 Tier8 (it's in the 24-hour test phase)... - PR: https://git.openjdk.java.net/jdk/pull/4677
Re: RFR: 8249004: Reduce ThreadsListHandle overhead in relation to direct handshakes [v6]
> A fix to reduce ThreadsListHandle overhead in relation to handshakes and > we add sanity checks for ThreadsListHandles higher in the call stack. > > This fix was tested with Mach5 Tier[1-8]; Tier8 is still running. Daniel D. Daugherty has updated the pull request incrementally with one additional commit since the last revision: 8249004.cr1.patch - Changes: - all: https://git.openjdk.java.net/jdk/pull/4677/files - new: https://git.openjdk.java.net/jdk/pull/4677/files/a214b284..4e207e13 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4677=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4677=04-05 Stats: 83 lines in 5 files changed: 65 ins; 2 del; 16 mod Patch: https://git.openjdk.java.net/jdk/pull/4677.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4677/head:pull/4677 PR: https://git.openjdk.java.net/jdk/pull/4677
Re: RFR: 8275240: Change nested classes in jdk.attach to static nested classes [v2]
On Thu, 14 Oct 2021 06:59:12 GMT, Andrey Turbanov wrote: >> Non-static classes hold a link to their parent classes, which in many cases >> can be avoided. >> Similar cleanup in java.base - >> [JDK-8261880](https://bugs.openjdk.java.net/browse/JDK-8261880) > > Andrey Turbanov has updated the pull request incrementally with one > additional commit since the last revision: > > 8275240: Change nested classes in jdk.attach to static nested classes > update copyright year Marked as reviewed by sspitsyn (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5904
Re: RFR: 8275035: Clean up worker thread infrastructure [v2]
On Thu, 14 Oct 2021 09:03:18 GMT, Per Liden wrote: >> I propose that we clean up our GangWorker/WorkGang and related classes, to >> remove abstractions we no longer need (after CMS was removed, >> MutexDispatcher was removed, Parallel is now using WorkGang, etc) and >> adjusting names as follows: >> >> * Rename AbstractGangTask to WorkerTask >> * Rename WorkGang to WorkerThreads >> * Fold GangWorker into WorkerThread >> * Fold WorkManager into WorkerThreads >> * Move SubTaskDone and friends to a new workerUtils.hpp/cpp >> >> I've split things up into several commits to make it easier to review. >> >> Testing: Passes Tier 1-3 on all Oracle platforms. > > Per Liden has updated the pull request with a new target base due to a merge > or a rebase. The pull request now contains 21 commits: > > - Merge master > - Clean up naming in ReferenceProcessor > - Clean up naming in Shenandoah > - Clean up naming in HeapDumper/HeapInspector > - Clean up naming in G1 > - Clean up naming in pretouch > - Clean up naming in WeakProcessor > - Clean up naming in ZGC > - Remove unused log tag > - Rename workgroup.hpp/cpp to workerThread.hpp/cpp > - ... and 11 more: > https://git.openjdk.java.net/jdk/compare/8b1b6f9f...9a89f785 Thanks all for reviewing! - PR: https://git.openjdk.java.net/jdk/pull/5886
Integrated: 8275035: Clean up worker thread infrastructure
On Mon, 11 Oct 2021 09:21:21 GMT, Per Liden wrote: > I propose that we clean up our GangWorker/WorkGang and related classes, to > remove abstractions we no longer need (after CMS was removed, MutexDispatcher > was removed, Parallel is now using WorkGang, etc) and adjusting names as > follows: > > * Rename AbstractGangTask to WorkerTask > * Rename WorkGang to WorkerThreads > * Fold GangWorker into WorkerThread > * Fold WorkManager into WorkerThreads > * Move SubTaskDone and friends to a new workerUtils.hpp/cpp > > I've split things up into several commits to make it easier to review. > > Testing: Passes Tier 1-3 on all Oracle platforms. This pull request has now been integrated. Changeset: 54b88707 Author:Per Liden URL: https://git.openjdk.java.net/jdk/commit/54b887076612c0eaa410a849178f8ba0c4ed3eeb Stats: 2415 lines in 105 files changed: 881 ins; 1083 del; 451 mod 8275035: Clean up worker thread infrastructure Reviewed-by: stefank, ayang - PR: https://git.openjdk.java.net/jdk/pull/5886
Re: RFR: 8275259: Add support for Java level DCmd
On Thu, 14 Oct 2021 05:42:09 GMT, Denghui Dong wrote: > I proposed to extend DCmd to allow Java developers to customize their own > diagnostic commands last week. > > At present, I have implemented a preliminary version. > > In the current implementation, I provided some simple APIs in the Java layer > (under sun.management.cmd) for defining and registering commands. > > - Executable interface > Java diagnostic commands need to implement this interface, the interface > only contains a simple method: > > /** > * @param output the output when this executable is running > */ > void execute(PrintWriter output); > > > - Add two annotations (@Command and @Parameter) to describe the command meta > info > > - Use Factory API to register command, the following forms are supported > > @Command(name = "My.Echo", description = "Echo description") > class Echo implements Executable { > > @Parameter(name = "text", ordinal=0, isMandatory = true) > String text; > > @Parameter(name = "repeat", isMandatory = true, defaultValue = "1") > int repeat; > > @Override > public void execute(PrintWriter out) { > for (int i = 0 ; i < repeat; i++) { > out.println(text); > } > } > } > > Factory.register(Echo.class); > > > > Factory.register("My.Date", output -> { > output.println(new Date()); > }); > > > - When the command is running, the VM will call `Executor.executeCommand` to > execute the command. In the implementation of Executor, I introduced a simple > timeout mechanism to prevent the command channel from being blocked. > > At the VM layer, I extended the existing DCmd framework(implemented JavaDCmd > and JavaDCmdFactoryImpl) to be compatible with existing functions (jmx, help, > etc.). > > In terms of security, considering that the SecurityManager will be > deprecated, there are not too many considerations for the time being. > > Any input is appreciated. > > Thanks, > Denghui To understand what is needed (namespacing, error handling, safety, lifecycle etc.) and if this is actually useful for Java, I think you need to write 2-3 commands for a few popular frameworks, for example Spring. If we can't come up with 5-10 commands where this functionality would solve real problems users face, it's unlikely somebody will use it. It will just be a maintenance burden in the JDK. Once an API is added, it's hard to remove. The scope of this seem to be JEP level. (JFR is not a valid use case, since we don't want to add a dependency on annotation classes in the jdk.jcmd module). - PR: https://git.openjdk.java.net/jdk/pull/5938
Re: RFR: 8275259: Add support for Java level DCmd
On Thu, 14 Oct 2021 05:42:09 GMT, Denghui Dong wrote: > I proposed to extend DCmd to allow Java developers to customize their own > diagnostic commands last week. > > At present, I have implemented a preliminary version. > > In the current implementation, I provided some simple APIs in the Java layer > (under sun.management.cmd) for defining and registering commands. > > - Executable interface > Java diagnostic commands need to implement this interface, the interface > only contains a simple method: > > /** > * @param output the output when this executable is running > */ > void execute(PrintWriter output); > > > - Add two annotations (@Command and @Parameter) to describe the command meta > info > > - Use Factory API to register command, the following forms are supported > > @Command(name = "My.Echo", description = "Echo description") > class Echo implements Executable { > > @Parameter(name = "text", ordinal=0, isMandatory = true) > String text; > > @Parameter(name = "repeat", isMandatory = true, defaultValue = "1") > int repeat; > > @Override > public void execute(PrintWriter out) { > for (int i = 0 ; i < repeat; i++) { > out.println(text); > } > } > } > > Factory.register(Echo.class); > > > > Factory.register("My.Date", output -> { > output.println(new Date()); > }); > > > - When the command is running, the VM will call `Executor.executeCommand` to > execute the command. In the implementation of Executor, I introduced a simple > timeout mechanism to prevent the command channel from being blocked. > > At the VM layer, I extended the existing DCmd framework(implemented JavaDCmd > and JavaDCmdFactoryImpl) to be compatible with existing functions (jmx, help, > etc.). > > In terms of security, considering that the SecurityManager will be > deprecated, there are not too many considerations for the time being. > > Any input is appreciated. > > Thanks, > Denghui Hi, Interesting proposal. I have some questions. - Will hanging java user code block the attach listener thread? If yes, how would you solve that? - Will this require changes to the jcmd client, or will this work with any jcmd client, up- and downward the JDK versions? (the nice thing about jcmd is that all the logic resides at the hotspot and I can use any client to talk to any hotspot) Cheers, Thomas P.S. I think it may be worthwhile to discuss this on the serviceability-dev mailing list first. - PR: https://git.openjdk.java.net/jdk/pull/5938
Re: RFR: 8275035: Clean up worker thread infrastructure [v2]
> I propose that we clean up our GangWorker/WorkGang and related classes, to > remove abstractions we no longer need (after CMS was removed, MutexDispatcher > was removed, Parallel is now using WorkGang, etc) and adjusting names as > follows: > > * Rename AbstractGangTask to WorkerTask > * Rename WorkGang to WorkerThreads > * Fold GangWorker into WorkerThread > * Fold WorkManager into WorkerThreads > * Move SubTaskDone and friends to a new workerUtils.hpp/cpp > > I've split things up into several commits to make it easier to review. > > Testing: Passes Tier 1-3 on all Oracle platforms. Per Liden has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 21 commits: - Merge master - Clean up naming in ReferenceProcessor - Clean up naming in Shenandoah - Clean up naming in HeapDumper/HeapInspector - Clean up naming in G1 - Clean up naming in pretouch - Clean up naming in WeakProcessor - Clean up naming in ZGC - Remove unused log tag - Rename workgroup.hpp/cpp to workerThread.hpp/cpp - ... and 11 more: https://git.openjdk.java.net/jdk/compare/8b1b6f9f...9a89f785 - Changes: https://git.openjdk.java.net/jdk/pull/5886/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5886=01 Stats: 2415 lines in 105 files changed: 881 ins; 1083 del; 451 mod Patch: https://git.openjdk.java.net/jdk/pull/5886.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5886/head:pull/5886 PR: https://git.openjdk.java.net/jdk/pull/5886
Re: RFR: 8275259: Add support for Java level DCmd
On Thu, 14 Oct 2021 07:40:20 GMT, David Holmes wrote: > Ah right - so you're really looking at extending the capabilities of the > DiagnosticCommandMBean to add a way to register a Java diagnostic command. Yes, but we could not add registration API to DiagnosticCommandMBean directly. There are already some VM diagnostic commands that depend on Java methods directly, this extension could also bring benefit to this scenario I think. Hope folks from serviceability could take a look at it:) Denghui - PR: https://git.openjdk.java.net/jdk/pull/5938
Re: RFR: 8274687: JDWP deadlocks if some Java thread reaches wait in blockOnDebuggerSuspend [v4]
On Wed, 13 Oct 2021 20:59:44 GMT, Chris Plummer wrote: > Ok, so you need to hold threadLock from the time `blockOnDebuggerSuspend()` > is done waiting on it until after `trackAppResume()` is done, and since > `trackAppResume()` needs to grab the handlerLock, and you need to grab the > handerLock before the threadLock, you need to jump through some lock > grabbing/release hoops. That's correct. > However, you are in fact releasing the threadLock for a short time in > `blockOnDebuggerSuspend()` after the wait completes. Doesn't this expose the > resumee to potential suspending after the wait has completed and before > `trackAppResume()` has been called? That's correct too. I wouldn't see an issue with it though. I think this is even a preexisting condition. The current thread can lose the race grabbing threadLock after it was notified to the debugger trying to suspend again (if there wasn't the deadlock of course) and consequently suspendCount can (again) be greater than 0 right after the wait. In that case we simply retry. - PR: https://git.openjdk.java.net/jdk/pull/5849
Re: RFR: 8275259: Add support for Java level DCmd
On 14/10/2021 5:14 pm, Denghui Dong wrote: On Thu, 14 Oct 2021 06:43:20 GMT, David Holmes wrote: I'm not sure exactly where this API would need to go. IIUC jcmd doesn't exist at the Java level it is just a tool, so introducing an API to interact with it seems problematic to me. IMO, `jcmd` is a client of `dcmd`, the user also can use JMX to execute `dcmd`, this means there already are Java APIs interact with `dcmd`. Ah right - so you're really looking at extending the capabilities of the DiagnosticCommandMBean to add a way to register a Java diagnostic command. David Denghui - PR: https://git.openjdk.java.net/jdk/pull/5938
Re: RFR: 8275259: Add support for Java level DCmd
On Thu, 14 Oct 2021 06:43:20 GMT, David Holmes wrote: > I'm not sure exactly where this API would need to go. IIUC jcmd doesn't exist at the Java level it is just a tool, so introducing an API to interact with it seems problematic to me. IMO, `jcmd` is a client of `dcmd`, the user also can use JMX to execute `dcmd`, this means there already are Java APIs interact with `dcmd`. Denghui - PR: https://git.openjdk.java.net/jdk/pull/5938
Re: RFR: 8275240: Change nested classes in jdk.attach to static nested classes [v2]
> Non-static classes hold a link to their parent classes, which in many cases > can be avoided. > Similar cleanup in java.base - > [JDK-8261880](https://bugs.openjdk.java.net/browse/JDK-8261880) Andrey Turbanov has updated the pull request incrementally with one additional commit since the last revision: 8275240: Change nested classes in jdk.attach to static nested classes update copyright year - Changes: - all: https://git.openjdk.java.net/jdk/pull/5904/files - new: https://git.openjdk.java.net/jdk/pull/5904/files/70cb33a9..06fcaa48 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5904=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5904=00-01 Stats: 3 lines in 1 file changed: 0 ins; 2 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/5904.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5904/head:pull/5904 PR: https://git.openjdk.java.net/jdk/pull/5904
Re: RFR: 8275259: Add support for Java level DCmd
On 14/10/2021 4:32 pm, Denghui Dong wrote: On Thu, 14 Oct 2021 06:17:13 GMT, David Holmes wrote: In the current implementation, I provided some simple APIs in the Java layer (under sun.management.cmd) An API that is expected to be used by application code can't go in the unsupported/internal-use-only sun.management namespace. This would have to be a new external facing supported API. Cheers, David Thanks. I consider putting the API into `javax.management.dcmd`. What do you think? There are very specific rules related to the use of the javax namespace. I'm not sure exactly where this API would need to go. IIUC jcmd doesn't exist at the Java level it is just a tool, so introducing an API to interact with it seems problematic to me. Other folk in serviceability (both VM and JDK sides) will need to weigh in here. David - PR: https://git.openjdk.java.net/jdk/pull/5938
Re: RFR: 8275259: Add support for Java level DCmd
On Thu, 14 Oct 2021 06:17:13 GMT, David Holmes wrote: > > In the current implementation, I provided some simple APIs in the Java > > layer (under sun.management.cmd) > > An API that is expected to be used by application code can't go in the > unsupported/internal-use-only sun.management namespace. This would have to be > a new external facing supported API. > > Cheers, David Thanks. I consider putting the API into `javax.management.dcmd`. What do you think? - PR: https://git.openjdk.java.net/jdk/pull/5938
Re: RFR: 8275259: Add support for Java level DCmd
On Thu, 14 Oct 2021 05:42:09 GMT, Denghui Dong wrote: > In the current implementation, I provided some simple APIs in the Java layer > (under sun.management.cmd) An API that is expected to be used by application code can't go in the unsupported/internal-use-only sun.management namespace. This would have to be a new external facing supported API. Cheers, David - PR: https://git.openjdk.java.net/jdk/pull/5938