Re: RFR: 8271356: Modify jdb to treat an empty command as a repeat of the previous command [v5]

2021-10-18 Thread Chris Plummer
On Thu, 30 Sep 2021 19:22:40 GMT, Chris Plummer  wrote:

>> Hey Chris, is there any update on this?  I've revised the Problem section so 
>> I think we're waiting on the CSR ticket to be updated accordingly and for 
>> someone to be assigned for the CSR review.
>
>> Hey Chris, is there any update on this? I've revised the Problem section so 
>> I think we're waiting on the CSR ticket to be updated accordingly and for 
>> someone to be assigned for the CSR review.
> 
> Ok. I updated the CSR with your changes. I'll try to find someone to review 
> it.

> @plummercj Looks like Serguei Spitsyn approved this about a week ago, so I 
> wanted to make sure this is moving along. Is the CSR proposal now approved or 
> is there a need for another reviewer?

It looks like it needed for me to click on "finalize" so I've done that. Now it 
is in the "Finalized" state and is I believe waiting final approval by the CSR 
committee, which I think will result in closing the CSR as "Approved", and you 
can then complete the review of the PR and push the changes.

-

PR: https://git.openjdk.java.net/jdk/pull/5290


Re: RFR: 8271356: Modify jdb to treat an empty command as a repeat of the previous command [v5]

2021-10-18 Thread Jakob Cornell
On Thu, 30 Sep 2021 19:22:40 GMT, Chris Plummer  wrote:

>> Hey Chris, is there any update on this?  I've revised the Problem section so 
>> I think we're waiting on the CSR ticket to be updated accordingly and for 
>> someone to be assigned for the CSR review.
>
>> Hey Chris, is there any update on this? I've revised the Problem section so 
>> I think we're waiting on the CSR ticket to be updated accordingly and for 
>> someone to be assigned for the CSR review.
> 
> Ok. I updated the CSR with your changes. I'll try to find someone to review 
> it.

@plummercj Looks like Serguei Spitsyn approved this about a week ago, so I 
wanted to make sure this is moving along.  Is the CSR proposal now approved or 
is there a need for another reviewer?

-

PR: https://git.openjdk.java.net/jdk/pull/5290


Re: RFR: 8275385: Change nested classes in jdk.jdi to static nested classes

2021-10-18 Thread Alex Menkov
On Sun, 17 Oct 2021 17:28:37 GMT, Andrey Turbanov  wrote:

> Non-static classes hold a link to their parent classes, which can be avoided.

Marked as reviewed by amenkov (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/5978


Re: RFR: 8274687: JDWP deadlocks if some Java thread reaches wait in blockOnDebuggerSuspend [v4]

2021-10-18 Thread Richard Reingruber
On Fri, 15 Oct 2021 11:52:09 GMT, Richard Reingruber  wrote:

> 
> 
> Hm, I think this can be simplified by swaping blockOnDebuggerSuspend() and 
> trackAppResume(). Can't try it today but will on Monday.

I've done that with commit 
https://github.com/openjdk/jdk/pull/5849/commits/0b0fef0e6670c20a0e1e34323847c5a622878469.
 The locking is clearer I would say. Since `resumeFrameDepth` is now set before 
resumee's suspendCount is 0 we must block debugger suspends only if 
`!handlingAppResume` because then we know resumee's suspendCount actually is 0.

-

PR: https://git.openjdk.java.net/jdk/pull/5849


Re: RFR: 8274687: JDWP deadlocks if some Java thread reaches wait in blockOnDebuggerSuspend [v7]

2021-10-18 Thread Richard Reingruber
> 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:

  Call blockOnDebuggerSuspend() after setup of the resumer tracking.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/5849/files
  - new: https://git.openjdk.java.net/jdk/pull/5849/files/20f1f31b..0b0fef0e

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=5849=06
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5849=05-06

  Stats: 95 lines in 2 files changed: 31 ins; 36 del; 28 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


Integrated: JDK-8275436 [BACKOUT] JDK-8271949 dumppath in -XX:FlightRecorderOptions does not affect

2021-10-18 Thread Daniel D . Daugherty
On Mon, 18 Oct 2021 17:58:25 GMT, Daniel D. Daugherty  
wrote:

> This reverts commit 31500692d1503cb73249e0425e693049258a.

This pull request has now been integrated.

Changeset: a03119ce
Author:Daniel D. Daugherty 
URL:   
https://git.openjdk.java.net/jdk/commit/a03119ce1a34642565c669bd2471f52eec088b96
Stats: 170 lines in 10 files changed: 3 ins; 138 del; 29 mod

8275436: [BACKOUT] JDK-8271949 dumppath in -XX:FlightRecorderOptions does not 
affect

Reviewed-by: mgronlun

-

PR: https://git.openjdk.java.net/jdk/pull/5994


Re: RFR: JDK-8275436 [BACKOUT] JDK-8271949 dumppath in -XX:FlightRecorderOptions does not affect

2021-10-18 Thread Markus Grönlund
On Mon, 18 Oct 2021 17:58:25 GMT, Daniel D. Daugherty  
wrote:

> This reverts commit 31500692d1503cb73249e0425e693049258a.

Thanks for creating the backout Dan, looks good.

-

Marked as reviewed by mgronlun (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/5994


Re: RFR: JDK-8275436 [BACKOUT] JDK-8271949 dumppath in -XX:FlightRecorderOptions does not affect

2021-10-18 Thread Daniel D . Daugherty
On Mon, 18 Oct 2021 18:14:16 GMT, Markus Grönlund  wrote:

>> This reverts commit 31500692d1503cb73249e0425e693049258a.
>
> Thanks for creating the backout Dan, looks good.

@mgronlun - Thanks for the fast review!

-

PR: https://git.openjdk.java.net/jdk/pull/5994


RFR: JDK-8275436 [BACKOUT] JDK-8271949 dumppath in -XX:FlightRecorderOptions does not affect

2021-10-18 Thread Daniel D . Daugherty
This reverts commit 31500692d1503cb73249e0425e693049258a.

-

Commit messages:
 - JDK-8275436 [BACKOUT] JDK-8271949 dumppath in -XX:FlightRecorderOptions does 
not affect

Changes: https://git.openjdk.java.net/jdk/pull/5994/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5994=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8275436
  Stats: 170 lines in 10 files changed: 3 ins; 138 del; 29 mod
  Patch: https://git.openjdk.java.net/jdk/pull/5994.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/5994/head:pull/5994

PR: https://git.openjdk.java.net/jdk/pull/5994


Integrated: 8266936: Add a finalization JFR event

2021-10-18 Thread Markus Grönlund
On Thu, 8 Jul 2021 19:47:26 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

This pull request has now been integrated.

Changeset: 72a976ef
Author:Markus Grönlund 
URL:   
https://git.openjdk.java.net/jdk/commit/72a976ef05fc2c62657920a560a0abc60b27c852
Stats: 1917 lines in 36 files changed: 1375 ins; 409 del; 133 mod

8266936: Add a finalization JFR event

Reviewed-by: coleenp, mchung, egahlin

-

PR: https://git.openjdk.java.net/jdk/pull/4731


Re: RFR: 8266936: Add a finalization JFR event [v12]

2021-10-18 Thread Markus Grönlund
On Fri, 24 Sep 2021 22:31:18 GMT, Mandy Chung  wrote:

>> Markus Grönlund has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   no precompiled headers and mtServiceability nmt classification
>
> Hi Markus,
> 
> It's a little surprised to see Finalizer.c to depend JMM interface which is 
> used by `java.management` and `jdk.management` modules only.   It's more 
> appropriate for it to be a JVM_* entry point for Finalizer to report 
> completion of the finalization instead.I understand that you want to make 
> FinalizerService to be a conditional feature which is a good idea.  Such JVM 
> entry can be made as a nop if not INCLUDE_SERVICES.

Thank you for staying around with this protracted PR. Thanks @mlchung , 
@coleenp and @egahlin for your reviews!

-

PR: https://git.openjdk.java.net/jdk/pull/4731


Re: RFR: 8266936: Add a finalization JFR event [v20]

2021-10-18 Thread Markus Grönlund
> 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 updated the pull request incrementally with one additional 
commit since the last revision:

  no constexpr for constant values

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4731/files
  - new: https://git.openjdk.java.net/jdk/pull/4731/files/85a46263..b3268c90

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4731=19
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4731=18-19

  Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4731.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4731/head:pull/4731

PR: https://git.openjdk.java.net/jdk/pull/4731


Re: RFR: 8266936: Add a finalization JFR event [v10]

2021-10-18 Thread Markus Grönlund
On Tue, 31 Aug 2021 08:45:54 GMT, Erik Gahlin  wrote:

>> Markus Grönlund has updated the pull request incrementally with three 
>> additional commits since the last revision:
>> 
>>  - localize
>>  - cleanup
>>  - FinalizerStatistics
>
> src/java.base/share/classes/java/lang/ref/Finalizer.java line 71:
> 
>> 69: }
>> 70: 
>> 71: 
> 
> extraneous whitespace?

I think this version is outdated, and the extra whitespace was removed in later 
versions.

-

PR: https://git.openjdk.java.net/jdk/pull/4731


Re: RFR: 8266936: Add a finalization JFR event [v10]

2021-10-18 Thread Markus Grönlund
On Tue, 31 Aug 2021 08:42:58 GMT, Erik Gahlin  wrote:

>> Markus Grönlund has updated the pull request incrementally with three 
>> additional commits since the last revision:
>> 
>>  - localize
>>  - cleanup
>>  - FinalizerStatistics
>
> test/jdk/jdk/jfr/event/runtime/TestFinalizerStatisticsEvent.java line 98:
> 
>> 96:   case TEST_CLASS_NAME: {
>> 97:   
>> Asserts.assertTrue(event.getString("codeSource").startsWith("file://"));
>> 98:   foundTestClassName = true;
> 
> Could we (sanity) check "objects" and "totalFinalzersRun" fields as well?

It's risky to do because of the non-deterministic nature of when the Finalizer 
thread runs (if at all). The best I could think of is to check if either field 
is 0 or more, but that becomes so weak it's not much of a sanity check.

-

PR: https://git.openjdk.java.net/jdk/pull/4731


Re: RFR: 8266936: Add a finalization JFR event [v10]

2021-10-18 Thread Erik Gahlin
On Fri, 27 Aug 2021 15:23:35 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 updated the pull request incrementally with three 
> additional commits since the last revision:
> 
>  - localize
>  - cleanup
>  - FinalizerStatistics

Marked as reviewed by egahlin (Reviewer).

src/java.base/share/classes/java/lang/ref/Finalizer.java line 71:

> 69: }
> 70: 
> 71: 

extraneous whitespace?

test/jdk/jdk/jfr/event/runtime/TestFinalizerStatisticsEvent.java line 98:

> 96:   case TEST_CLASS_NAME: {
> 97:   
> Asserts.assertTrue(event.getString("codeSource").startsWith("file://"));
> 98:   foundTestClassName = true;

Could we (sanity) check "objects" and "totalFinalzersRun" fields as well?

-

PR: https://git.openjdk.java.net/jdk/pull/4731


Re: RFR: 8275384: Change nested classes in jdk.jconsole to static nested classes

2021-10-18 Thread Mandy Chung
On Sat, 16 Oct 2021 20:32:43 GMT, Andrey Turbanov  wrote:

> Non-static classes hold a link to their parent classes, which can be avoided.

Marked as reviewed by mchung (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/5974


Some call sites don't pass 'size' to sharedRuntime::dtrace_object_alloc/dtrace_object_alloc_base

2021-10-18 Thread Denghui Dong
Hi,

A new param named 'size' was added to 
sharedRuntime::dtrace_object_alloc/dtrace_object_alloc_base
in JDK-8039904("dtrace/hotspot/Monitors/Monitors001 fails with "assert(s > 0) 
failed: Bad size calculated").

But in some places where these two functions are called, this new parameter is 
not passed,
such as TemplateTable::_new and PhaseMacroExpand::expand_dtrace_alloc_probe.

Was it intentional or should we fix it?

Thanks,
Denghui

Integrated: 8275322: Change nested classes in java.management to static nested classes

2021-10-18 Thread Andrey Turbanov
On Fri, 15 Oct 2021 06:43:13 GMT, Andrey Turbanov  wrote:

> Non-static classes hold a link to their parent classes, which can be avoided.

This pull request has now been integrated.

Changeset: 1afddb25
Author:Andrey Turbanov 
Committer: Daniel Fuchs 
URL:   
https://git.openjdk.java.net/jdk/commit/1afddb2560504c533d30b2f79d735f59f519e7c5
Stats: 6 lines in 3 files changed: 0 ins; 1 del; 5 mod

8275322: Change nested classes in java.management to static nested classes

Reviewed-by: alanb, dfuchs, mchung, sspitsyn

-

PR: https://git.openjdk.java.net/jdk/pull/5962


Re: RFR: 8275385: Change nested classes in jdk.jdi to static nested classes

2021-10-18 Thread Serguei Spitsyn
On Sun, 17 Oct 2021 17:28:37 GMT, Andrey Turbanov  wrote:

> Non-static classes hold a link to their parent classes, which can be avoided.

Marked as reviewed by sspitsyn (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/5978


Re: RFR: 8275384: Change nested classes in jdk.jconsole to static nested classes

2021-10-18 Thread Serguei Spitsyn
On Sat, 16 Oct 2021 20:32:43 GMT, Andrey Turbanov  wrote:

> Non-static classes hold a link to their parent classes, which can be avoided.

Marked as reviewed by sspitsyn (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/5974


Re: RFR: 8266936: Add a finalization JFR event [v19]

2021-10-18 Thread Markus Grönlund
> 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 updated the pull request incrementally with one additional 
commit since the last revision:

  relax memory ordering constraint

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4731/files
  - new: https://git.openjdk.java.net/jdk/pull/4731/files/d10eb309..85a46263

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4731=18
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4731=17-18

  Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4731.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4731/head:pull/4731

PR: https://git.openjdk.java.net/jdk/pull/4731


Re: RFR: 8275384: Change nested classes in jdk.jconsole to static nested classes

2021-10-18 Thread Alan Bateman
On Sat, 16 Oct 2021 20:32:43 GMT, Andrey Turbanov  wrote:

> Non-static classes hold a link to their parent classes, which can be avoided.

Marked as reviewed by alanb (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/5974


RFR: 8275385: Change nested classes in jdk.jdi to static nested classes

2021-10-18 Thread Andrey Turbanov
Non-static classes hold a link to their parent classes, which can be avoided.

-

Commit messages:
 - [PATCH] Change nested classes in jdk.jdi to static nested classes

Changes: https://git.openjdk.java.net/jdk/pull/5978/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5978=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8275385
  Stats: 10 lines in 3 files changed: 0 ins; 0 del; 10 mod
  Patch: https://git.openjdk.java.net/jdk/pull/5978.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/5978/head:pull/5978

PR: https://git.openjdk.java.net/jdk/pull/5978


RFR: 8275384: Change nested classes in jdk.jconsole to static nested classes

2021-10-18 Thread Andrey Turbanov
Non-static classes hold a link to their parent classes, which can be avoided.

-

Commit messages:
 - [PATCH] Change nested classes in jdk.jconsole to static nested classes

Changes: https://git.openjdk.java.net/jdk/pull/5974/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5974=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8275384
  Stats: 12 lines in 5 files changed: 0 ins; 0 del; 12 mod
  Patch: https://git.openjdk.java.net/jdk/pull/5974.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/5974/head:pull/5974

PR: https://git.openjdk.java.net/jdk/pull/5974