Re: RFR: 8041626: [Event Request] Shutdown reason
Hi again, Here’s the (hopefully) final version: Full: http://cr.openjdk.java.net/~rwestberg/8041626/webrev.02/ <http://cr.openjdk.java.net/~rwestberg/8041626/webrev.02/> Incremental: http://cr.openjdk.java.net/~rwestberg/8041626/webrev.01-02/ <http://cr.openjdk.java.net/~rwestberg/8041626/webrev.01-02/> Best regards, Robin
On 14 Feb 2018, at 16:15, Robin Westberg <robin.westberg@oracle.com> wrote:
Hi Roger,
On 13 Feb 2018, at 16:17, Roger Riggs <Roger.Riggs@Oracle.com> wrote:
Hi Robin,
It looks like the status argument to BeforeHalt is discarded in JVM_BeforeHalt and is not inserted into the event. That suggests it should be removed all the way back to Shutdown.beforeHalt.
You are right, my thinking was that the interface wouldn’t need to be changed if we decided to revisit the event in the future and add the status code. But that is perhaps unlikely, so can certainly remove the argument for now.
Best regards, Robin
Roger
On 2/13/2018 9:59 AM, Robin Westberg wrote:
Hi Alan,
On 12 Feb 2018, at 09:02, Alan Bateman <Alan.Bateman@oracle.com> wrote:
On 12/02/2018 07:07, David Holmes wrote:
Okay, I’ve removed the code related to the status field, certainly makes the change a bit less intrusive.
Updated webrev: http://cr.openjdk.java.net/~rwestberg/8041626/webrev.01/ Incremental: http://cr.openjdk.java.net/~rwestberg/8041626/webrev.00-01/ This looks much cleaner/neater to me - thanks.
The updates to Runtime/Shutdown seems okay. Thanks for reviewing!
Best regards, Robin
-Alan
Hi Robin, Looks fine to me. (How is this tested?, Normal, exceptional, etc.) Thanks, Roger On 2/15/2018 8:35 AM, Robin Westberg wrote:
Hi again,
Here’s the (hopefully) final version:
Full: http://cr.openjdk.java.net/~rwestberg/8041626/webrev.02/ <http://cr.openjdk.java.net/%7Erwestberg/8041626/webrev.02/> Incremental: http://cr.openjdk.java.net/~rwestberg/8041626/webrev.01-02/ <http://cr.openjdk.java.net/%7Erwestberg/8041626/webrev.01-02/>
Best regards, Robin
On 14 Feb 2018, at 16:15, Robin Westberg <robin.westberg@oracle.com <mailto:robin.westberg@oracle.com>> wrote:
Hi Roger,
On 13 Feb 2018, at 16:17, Roger Riggs <Roger.Riggs@Oracle.com <mailto:Roger.Riggs@Oracle.com>> wrote:
Hi Robin,
It looks like the status argument to BeforeHalt is discarded in JVM_BeforeHalt and is not inserted into the event. That suggests it should be removed all the way back to Shutdown.beforeHalt.
You are right, my thinking was that the interface wouldn’t need to be changed if we decided to revisit the event in the future and add the status code. But that is perhaps unlikely, so can certainly remove the argument for now.
Best regards, Robin
Roger
On 2/13/2018 9:59 AM, Robin Westberg wrote:
Hi Alan,
On 12 Feb 2018, at 09:02, Alan Bateman <Alan.Bateman@oracle.com <mailto:Alan.Bateman@oracle.com>> wrote:
On 12/02/2018 07:07, David Holmes wrote:
> Okay, I’ve removed the code related to the status field, > certainly makes the change a bit less intrusive. > > Updated webrev: > http://cr.openjdk.java.net/~rwestberg/8041626/webrev.01/ > <http://cr.openjdk.java.net/%7Erwestberg/8041626/webrev.01/> > Incremental: > http://cr.openjdk.java.net/~rwestberg/8041626/webrev.00-01/ > <http://cr.openjdk.java.net/%7Erwestberg/8041626/webrev.00-01/> This looks much cleaner/neater to me - thanks.
The updates to Runtime/Shutdown seems okay. Thanks for reviewing!
Best regards, Robin
-Alan
Hi Roger,
On 15 Feb 2018, at 17:00, Roger Riggs <Roger.Riggs@Oracle.com> wrote:
Hi Robin,
Looks fine to me.
Thanks for reviewing!
(How is this tested?, Normal, exceptional, etc.)
The tests are part of the (currently closed) JFR tests. Best regards, Robin
Thanks, Roger
On 2/15/2018 8:35 AM, Robin Westberg wrote:
Hi again,
Here’s the (hopefully) final version:
Full: http://cr.openjdk.java.net/~rwestberg/8041626/webrev.02/ <http://cr.openjdk.java.net/%7Erwestberg/8041626/webrev.02/> Incremental: http://cr.openjdk.java.net/~rwestberg/8041626/webrev.01-02/ <http://cr.openjdk.java.net/%7Erwestberg/8041626/webrev.01-02/>
Best regards, Robin
On 14 Feb 2018, at 16:15, Robin Westberg <robin.westberg@oracle.com <mailto:robin.westberg@oracle.com>> wrote:
Hi Roger,
On 13 Feb 2018, at 16:17, Roger Riggs <Roger.Riggs@Oracle.com <mailto:Roger.Riggs@Oracle.com>> wrote:
Hi Robin,
It looks like the status argument to BeforeHalt is discarded in JVM_BeforeHalt and is not inserted into the event. That suggests it should be removed all the way back to Shutdown.beforeHalt.
You are right, my thinking was that the interface wouldn’t need to be changed if we decided to revisit the event in the future and add the status code. But that is perhaps unlikely, so can certainly remove the argument for now.
Best regards, Robin
Roger
On 2/13/2018 9:59 AM, Robin Westberg wrote:
Hi Alan,
On 12 Feb 2018, at 09:02, Alan Bateman <Alan.Bateman@oracle.com <mailto:Alan.Bateman@oracle.com>> wrote:
On 12/02/2018 07:07, David Holmes wrote: >> Okay, I’ve removed the code related to the status field, certainly makes the change a bit less intrusive. >> >> Updated webrev: http://cr.openjdk.java.net/~rwestberg/8041626/webrev.01/ <http://cr.openjdk.java.net/%7Erwestberg/8041626/webrev.01/> >> Incremental: http://cr.openjdk.java.net/~rwestberg/8041626/webrev.00-01/ <http://cr.openjdk.java.net/%7Erwestberg/8041626/webrev.00-01/> > This looks much cleaner/neater to me - thanks. > The updates to Runtime/Shutdown seems okay. Thanks for reviewing!
Best regards, Robin
-Alan
Hi Robin, Do you want a shutdown event for every call to Runtime::exit regardless where it is in the shutdown sequence? or do you expect to get an event of the first thread calling JVM_Halt? or the first thread starting the shutdown sequence but it may not be the thread halting? Mandy On 2/15/18 5:35 AM, Robin Westberg wrote:
Hi again,
Here’s the (hopefully) final version:
Full: http://cr.openjdk.java.net/~rwestberg/8041626/webrev.02/ <http://cr.openjdk.java.net/%7Erwestberg/8041626/webrev.02/> Incremental: http://cr.openjdk.java.net/~rwestberg/8041626/webrev.01-02/ <http://cr.openjdk.java.net/%7Erwestberg/8041626/webrev.01-02/>
Best regards, Robin
Hi Mandy,
On 15 Feb 2018, at 20:10, mandy chung <mandy.chung@oracle.com> wrote:
Hi Robin,
Do you want a shutdown event for every call to Runtime::exit regardless where it is in the shutdown sequence? or do you expect to get an event of the first thread calling JVM_Halt? or the first thread starting the shutdown sequence but it may not be the thread halting?
I was aiming for an event from the first thread starting the shutdown sequence, as I think that’s probably the one you are looking for if you are diagnosing an unexpected shutdown. Any event generated after that point may not make it into the recording as the tracing framework shuts down from a shutdown hook. If finalizers on exit go away, is it still possible that the thread starting the shutdown sequence isn’t the one eventually calling JVM_Halt (in the absence of calls to Runtime.halt)? Best regards, Robin
Mandy
On 2/15/18 5:35 AM, Robin Westberg wrote:
Hi again,
Here’s the (hopefully) final version:
Full: http://cr.openjdk.java.net/~rwestberg/8041626/webrev.02/ <http://cr.openjdk.java.net/%7Erwestberg/8041626/webrev.02/> Incremental: http://cr.openjdk.java.net/~rwestberg/8041626/webrev.01-02/ <http://cr.openjdk.java.net/%7Erwestberg/8041626/webrev.01-02/>
Best regards, Robin
Looks fine to me. Thanks, David ----- On 15/02/2018 11:35 PM, Robin Westberg wrote:
Hi again,
Here’s the (hopefully) final version:
Full: http://cr.openjdk.java.net/~rwestberg/8041626/webrev.02/ <http://cr.openjdk.java.net/~rwestberg/8041626/webrev.02/> Incremental: http://cr.openjdk.java.net/~rwestberg/8041626/webrev.01-02/ <http://cr.openjdk.java.net/~rwestberg/8041626/webrev.01-02/>
Best regards, Robin
On 14 Feb 2018, at 16:15, Robin Westberg <robin.westberg@oracle.com> wrote:
Hi Roger,
On 13 Feb 2018, at 16:17, Roger Riggs <Roger.Riggs@Oracle.com> wrote:
Hi Robin,
It looks like the status argument to BeforeHalt is discarded in JVM_BeforeHalt and is not inserted into the event. That suggests it should be removed all the way back to Shutdown.beforeHalt.
You are right, my thinking was that the interface wouldn’t need to be changed if we decided to revisit the event in the future and add the status code. But that is perhaps unlikely, so can certainly remove the argument for now.
Best regards, Robin
Roger
On 2/13/2018 9:59 AM, Robin Westberg wrote:
Hi Alan,
On 12 Feb 2018, at 09:02, Alan Bateman <Alan.Bateman@oracle.com> wrote:
On 12/02/2018 07:07, David Holmes wrote:
> Okay, I’ve removed the code related to the status field, certainly makes the change a bit less intrusive. > > Updated webrev: http://cr.openjdk.java.net/~rwestberg/8041626/webrev.01/ > Incremental: http://cr.openjdk.java.net/~rwestberg/8041626/webrev.00-01/ This looks much cleaner/neater to me - thanks.
The updates to Runtime/Shutdown seems okay. Thanks for reviewing!
Best regards, Robin
-Alan
participants (4)
-
David Holmes
-
mandy chung
-
Robin Westberg
-
Roger Riggs