RFR: 8275185: Remove dead code and clean up jvmstat LocalVmManager
LocalVmManager and PerfDataFile have APIs that are supposed to look for VMs owned by a specific user. No one uses these APIs, and they don't work anyway. The current code is very confusing to look at. Since we're likely to change code in this area for further container support, it's better to clean up the code now. - Remove all APIs that take a user name - Also removed PerfDataFile.getFile() methods that are unused - Cleaned up the code that looks up the hsperfdata_xxx files - Fix comments to explain what's happening - Avoid using Matcher.reset which is not thread-safe - Renamed confusing variables such as `userFilter` to make the code more readable - LocalVmManager.activeVms() probably doesn't need to be synchronized, but I kept it anyway to avoid unnecessary risks. Testing with Oracle CI: tiers1-4, plus hs-tier5-rt (which tests containers and have extensive use of the management tools). - Commit messages: - 8275185: Remove dead code and clean up jvmstat LocalVmManager Changes: https://git.openjdk.java.net/jdk/pull/5923/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5923=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8275185 Stats: 319 lines in 2 files changed: 9 ins; 278 del; 32 mod Patch: https://git.openjdk.java.net/jdk/pull/5923.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5923/head:pull/5923 PR: https://git.openjdk.java.net/jdk/pull/5923
Re: RFR: 8275075: Remove unnecessary conversion to String in jdk.hotspot.agent
On Tue, 12 Oct 2021 05:37:53 GMT, Chris Plummer wrote:
>> Cleanup unnecessary toString() calls when conversion will happen implicitly
>> anyway
>
> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/ui/classbrowser/HTMLGenerator.java
> line 1012:
>
>> 1010: tmpBuf.append(Long.toHexString(startPc +
>> visitor.getInstructionSize()));
>> 1011: tmpBuf.append(",0x");
>> 1012: tmpBuf.append(Long.toHexString(startPc));
>
> Overall these changes look good, although I do wonder what motivated to
> `toString()` calls in the first place, especially this example where lines
> 1010 and 1012 are similar, but only 1010 used `toString()`.
Often reason for such mistakes is pretty trivial: copy-paste error, refactoring
stopped in the middle or incorrect merge.
Unfortunately this code is from pre-OpenJDK and it's impossible to find
original motifaction.
-
PR: https://git.openjdk.java.net/jdk/pull/5800
Integrated: 8268764: Use Long.hashCode() instead of int-cast where applicable
On Tue, 15 Jun 2021 12:15:11 GMT, Сергей Цыпанов wrote:
> In some JDK classes there's still the following hashCode() implementation:
>
> long objNum;
>
> public int hashCode() {
> return (int) objNum;
> }
>
> This outdated expression should be replaced with Long.hashCode(long) as it
>
> - uses all bits of the original value, does not discard any information
> upfront. For example, depending on how you are generating the IDs, the upper
> bits could change more frequently (or the opposite).
>
> - does not introduce any bias towards values with more ones (zeros), as it
> would be the case if the two halves were combined with an OR (AND) operation.
>
> See https://stackoverflow.com/a/4045083
>
> This is related to https://github.com/openjdk/jdk/pull/4309
This pull request has now been integrated.
Changeset: 124f8237
Author:Sergey Tsypanov
Committer: Sergey Bylokhov
URL:
https://git.openjdk.java.net/jdk/commit/124f82377ba93359bc59118ee315ba194080fa92
Stats: 21 lines in 9 files changed: 6 ins; 0 del; 15 mod
8268764: Use Long.hashCode() instead of int-cast where applicable
Reviewed-by: kevinw, prr, kizune, serb
-
PR: https://git.openjdk.java.net/jdk/pull/4491
Integrated: 8271514: support JFR use of new ThreadsList::Iterator
On Fri, 30 Jul 2021 20:20:48 GMT, Daniel D. Daugherty wrote: > A trivial fix to support JFR use of new ThreadsList::Iterator. > > This fix was tested with Mach5 Tier[1-3]. This pull request has now been integrated. Changeset: 8657f776 Author:Daniel D. Daugherty URL: https://git.openjdk.java.net/jdk/commit/8657f77608f37d7ff5254032858f2f16c7c204d5 Stats: 39 lines in 2 files changed: 20 ins; 11 del; 8 mod 8271514: support JFR use of new ThreadsList::Iterator Co-authored-by: Kim Barrett Reviewed-by: sspitsyn, mgronlun - PR: https://git.openjdk.java.net/jdk/pull/4949
Re: RFR: 8271514: support JFR use of new ThreadsList::Iterator [v2]
On Wed, 4 Aug 2021 22:57:40 GMT, Serguei Spitsyn wrote: >> Daniel D. Daugherty 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: >> >> - Merge branch 'pull/4948' into JDK-8271514 >> - Merge branch 'pull/4948' into JDK-8271514 >> - 8271514: support JFR use of new ThreadsList::Iterator > > Hi Dan, > It looks good to me modulo the question from David. > Thanks, > Serguei @sspitsyn and @mgronlun - Thanks for the reviews. - PR: https://git.openjdk.java.net/jdk/pull/4949
Re: RFR: 8272992: Replace usages of Collections.sort with List.sort call in jdk.* modules [v4]
On Sat, 25 Sep 2021 11:23:18 GMT, Andrey Turbanov wrote: >> Collections.sort is just a wrapper, so it is better to use an instance >> method directly. >> Affected modules: jdk.javadoc, jdk.jcmd, jdk.jconsole > > Andrey Turbanov has updated the pull request incrementally with one > additional commit since the last revision: > > 8272992: Replace usages of Collections.sort with List.sort call in jdk.* > modules > extracted jdk.hotspot.agent changes into separate PR. Rollback them here. I've submitted an extended test job with the 5f75485 commit; if that job shows no issues, I'll sponsor this PR. - PR: https://git.openjdk.java.net/jdk/pull/5230
Re: RFR: 8272992: Replace usages of Collections.sort with List.sort call in jdk.* modules [v4]
On Sat, 25 Sep 2021 11:23:18 GMT, Andrey Turbanov wrote: >> Collections.sort is just a wrapper, so it is better to use an instance >> method directly. >> Affected modules: jdk.javadoc, jdk.jcmd, jdk.jconsole > > Andrey Turbanov has updated the pull request incrementally with one > additional commit since the last revision: > > 8272992: Replace usages of Collections.sort with List.sort call in jdk.* > modules > extracted jdk.hotspot.agent changes into separate PR. Rollback them here. Changes to the jdk.javadoc module look good; I can sponsor this PR. - Marked as reviewed by prappo (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5230
Re: RFR: 8271514: support JFR use of new ThreadsList::Iterator [v5]
On Mon, 11 Oct 2021 22:19:59 GMT, Daniel D. Daugherty wrote: >> A trivial fix to support JFR use of new ThreadsList::Iterator. >> >> This fix was tested with Mach5 Tier[1-3]. > > Daniel D. Daugherty has updated the pull request with a new target base due > to a merge or a rebase. The pull request now contains 14 commits: > > - Merge branch 'master' into JDK-8271514 > - Merge branch 'master' into JDK-8271514 > - Merge branch 'master' into JDK-8271514 > - Merge branch 'pull/4948' into JDK-8271514 > - Merge branch 'master' into JDK-8271513 > - Merge branch 'pull/4948' into JDK-8271514 > - Merge branch 'pull/4671' into JDK-8271513 > - kbarrett CR - simplify 'ThreadsList::Iterator::operator!=(Iterator i)' > - 8271514: support JFR use of new ThreadsList::Iterator > - 8271513: support JavaThreadIteratorWithHandle replacement by new > ThreadsList::Iterator > - ... and 4 more: > https://git.openjdk.java.net/jdk/compare/829dea45...a2e39c36 Looks good Dan, thank you. - Marked as reviewed by mgronlun (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/4949
Re: RFR: 8274687: JDWP deadlocks if some Java thread reaches wait in blockOnDebuggerSuspend [v4]
On Tue, 12 Oct 2021 05:59:28 GMT, Chris Plummer wrote: >> Richard Reingruber has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Improve @summary section of test. > > test/jdk/com/sun/jdi/ResumeAfterThreadResumeCallTest.java line 31: > >> 29: * the JDWP agent (in blockOnDebuggerSuspend()) because it >> called >> 30: * j.l.Thread.resume() on a thread R that was suspended by the >> 31: * debugger. > > This is hard to follow. Maybe instead document the steps the test takes. For > example maybe something like: Suspend Thread R via breakpoint. Thread T calls > j.l.Thread.resume() on Thread R, resulting in Thread T blocking in > blockOnDebuggerSuspend. Resume Thread R using ThreadReference.resume(). > Verify that Thread T is no longer blocked in blockOnDebuggerSuspend. > > Also, it would be nice if thread names in the description matched the names > used in the implementation. Thanks for looking at this. I've updated the test summary. - PR: https://git.openjdk.java.net/jdk/pull/5849
Re: RFR: 8274687: JDWP deadlocks if some Java thread reaches wait in blockOnDebuggerSuspend [v4]
> This change fixes deadlocks described in the JBS-bug by: > > * Releasing `handlerLock` before waiting on `threadLock` in > `blockOnDebuggerSuspend()` > > * Notifying on `threadLock` in `threadControl_reset()` > > Also the actions in handleAppResumeBreakpoint() are moved/deferred until > doPendingTasks() runs. This is necessary because an event handler must not > release handlerLock first and foremost because handlers are called while > iterating the handler chain for an event type which is protected by > handlerLock > (see https://github.com/openjdk/jdk/pull/5805) > > The first commit delays the cleanup actions after leaving the loop in > `debugLoop_run()`. It allows to reproduce the deadlock running the dispose003 > test with the command > > > make run-test > TEST=test/hotspot/jtreg/vmTestbase/nsk/jdi/VirtualMachine/dispose/dispose003 > > > The second commit adds a new test that reproduces the deadlock when calling > threadControl_resumeThread() while a thread is waiting in > blockOnDebuggerSuspend(). > > The third commit contains the fix described above. With it the deadlocks > cannot be reproduced anymore. > > The forth commit removes the diagnostic code introduced with the first commit > again. > > The fix passed > > test/hotspot/jtreg/serviceability/jdwp > test/jdk/com/sun/jdi > test/hotspot/jtreg/vmTestbase/nsk/jdwp > test/hotspot/jtreg/vmTestbase/nsk/jdi Richard Reingruber has updated the pull request incrementally with one additional commit since the last revision: Improve @summary section of test. - Changes: - all: https://git.openjdk.java.net/jdk/pull/5849/files - new: https://git.openjdk.java.net/jdk/pull/5849/files/e198e5ea..8c51e71f Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5849=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5849=02-03 Stats: 3 lines in 1 file changed: 2 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/5849.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5849/head:pull/5849 PR: https://git.openjdk.java.net/jdk/pull/5849
Re: RFR: 8274687: JDWP deadlocks if some Java thread reaches wait in blockOnDebuggerSuspend [v3]
> This change fixes deadlocks described in the JBS-bug by: > > * Releasing `handlerLock` before waiting on `threadLock` in > `blockOnDebuggerSuspend()` > > * Notifying on `threadLock` in `threadControl_reset()` > > Also the actions in handleAppResumeBreakpoint() are moved/deferred until > doPendingTasks() runs. This is necessary because an event handler must not > release handlerLock first and foremost because handlers are called while > iterating the handler chain for an event type which is protected by > handlerLock > (see https://github.com/openjdk/jdk/pull/5805) > > The first commit delays the cleanup actions after leaving the loop in > `debugLoop_run()`. It allows to reproduce the deadlock running the dispose003 > test with the command > > > make run-test > TEST=test/hotspot/jtreg/vmTestbase/nsk/jdi/VirtualMachine/dispose/dispose003 > > > The second commit adds a new test that reproduces the deadlock when calling > threadControl_resumeThread() while a thread is waiting in > blockOnDebuggerSuspend(). > > The third commit contains the fix described above. With it the deadlocks > cannot be reproduced anymore. > > The forth commit removes the diagnostic code introduced with the first commit > again. > > The fix passed > > test/hotspot/jtreg/serviceability/jdwp > test/jdk/com/sun/jdi > test/hotspot/jtreg/vmTestbase/nsk/jdwp > test/hotspot/jtreg/vmTestbase/nsk/jdi Richard Reingruber has updated the pull request incrementally with one additional commit since the last revision: Improve @summary section of test. - Changes: - all: https://git.openjdk.java.net/jdk/pull/5849/files - new: https://git.openjdk.java.net/jdk/pull/5849/files/6cc006b1..e198e5ea Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5849=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5849=01-02 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/5849.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5849/head:pull/5849 PR: https://git.openjdk.java.net/jdk/pull/5849
Re: RFR: 8268764: Use Long.hashCode() instead of int-cast where applicable [v4]
On Thu, 1 Jul 2021 12:19:53 GMT, Сергей Цыпанов wrote:
>> In some JDK classes there's still the following hashCode() implementation:
>>
>> long objNum;
>>
>> public int hashCode() {
>> return (int) objNum;
>> }
>>
>> This outdated expression should be replaced with Long.hashCode(long) as it
>>
>> - uses all bits of the original value, does not discard any information
>> upfront. For example, depending on how you are generating the IDs, the upper
>> bits could change more frequently (or the opposite).
>>
>> - does not introduce any bias towards values with more ones (zeros), as it
>> would be the case if the two halves were combined with an OR (AND) operation.
>>
>> See https://stackoverflow.com/a/4045083
>>
>> This is related to https://github.com/openjdk/jdk/pull/4309
>
> Сергей Цыпанов has updated the pull request incrementally with one additional
> commit since the last revision:
>
> 8268764: Update copy-right year
Marked as reviewed by serb (Reviewer).
-
PR: https://git.openjdk.java.net/jdk/pull/4491
Re: RFR: 8274687: JDWP deadlocks if some Java thread reaches wait in blockOnDebuggerSuspend [v2]
> This change fixes deadlocks described in the JBS-bug by: > > * Releasing `handlerLock` before waiting on `threadLock` in > `blockOnDebuggerSuspend()` > > * Notifying on `threadLock` in `threadControl_reset()` > > Also the actions in handleAppResumeBreakpoint() are moved/deferred until > doPendingTasks() runs. This is necessary because an event handler must not > release handlerLock first and foremost because handlers are called while > iterating the handler chain for an event type which is protected by > handlerLock > (see https://github.com/openjdk/jdk/pull/5805) > > The first commit delays the cleanup actions after leaving the loop in > `debugLoop_run()`. It allows to reproduce the deadlock running the dispose003 > test with the command > > > make run-test > TEST=test/hotspot/jtreg/vmTestbase/nsk/jdi/VirtualMachine/dispose/dispose003 > > > The second commit adds a new test that reproduces the deadlock when calling > threadControl_resumeThread() while a thread is waiting in > blockOnDebuggerSuspend(). > > The third commit contains the fix described above. With it the deadlocks > cannot be reproduced anymore. > > The forth commit removes the diagnostic code introduced with the first commit > again. > > The fix passed > > test/hotspot/jtreg/serviceability/jdwp > test/jdk/com/sun/jdi > test/hotspot/jtreg/vmTestbase/nsk/jdwp > test/hotspot/jtreg/vmTestbase/nsk/jdi Richard Reingruber has updated the pull request incrementally with one additional commit since the last revision: Improve @summary section of test. - Changes: - all: https://git.openjdk.java.net/jdk/pull/5849/files - new: https://git.openjdk.java.net/jdk/pull/5849/files/8c1b5cec..6cc006b1 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5849=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5849=00-01 Stats: 30 lines in 1 file changed: 18 ins; 0 del; 12 mod Patch: https://git.openjdk.java.net/jdk/pull/5849.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5849/head:pull/5849 PR: https://git.openjdk.java.net/jdk/pull/5849
Re: RFR: 8274687: JDWP deadlocks if some Java thread reaches wait in blockOnDebuggerSuspend
On Thu, 7 Oct 2021 13:50:49 GMT, Richard Reingruber wrote: > This change fixes deadlocks described in the JBS-bug by: > > * Releasing `handlerLock` before waiting on `threadLock` in > `blockOnDebuggerSuspend()` > > * Notifying on `threadLock` in `threadControl_reset()` > > Also the actions in handleAppResumeBreakpoint() are moved/deferred until > doPendingTasks() runs. This is necessary because an event handler must not > release handlerLock first and foremost because handlers are called while > iterating the handler chain for an event type which is protected by > handlerLock > (see https://github.com/openjdk/jdk/pull/5805) > > The first commit delays the cleanup actions after leaving the loop in > `debugLoop_run()`. It allows to reproduce the deadlock running the dispose003 > test with the command > > > make run-test > TEST=test/hotspot/jtreg/vmTestbase/nsk/jdi/VirtualMachine/dispose/dispose003 > > > The second commit adds a new test that reproduces the deadlock when calling > threadControl_resumeThread() while a thread is waiting in > blockOnDebuggerSuspend(). > > The third commit contains the fix described above. With it the deadlocks > cannot be reproduced anymore. > > The forth commit removes the diagnostic code introduced with the first commit > again. > > The fix passed > > test/hotspot/jtreg/serviceability/jdwp > test/jdk/com/sun/jdi > test/hotspot/jtreg/vmTestbase/nsk/jdwp > test/hotspot/jtreg/vmTestbase/nsk/jdi test/jdk/com/sun/jdi/ResumeAfterThreadResumeCallTest.java line 31: > 29: * the JDWP agent (in blockOnDebuggerSuspend()) because it called > 30: * j.l.Thread.resume() on a thread R that was suspended by the > 31: * debugger. This is hard to follow. Maybe instead document the steps the test takes. For example maybe something like: Suspend Thread R via breakpoint. Thread T calls j.l.Thread.resume() on Thread R, resulting in Thread T blocking in blockOnDebuggerSuspend. Resume Thread R using ThreadReference.resume(). Verify that Thread T is no longer blocked in blockOnDebuggerSuspend. Also, it would be nice if thread names in the description matched the names used in the implementation. - PR: https://git.openjdk.java.net/jdk/pull/5849
