Re: RFR (M): 8231209: [REDO] ThreadMXBean::getThreadAllocatedBytes() can be quicker for self thread

2019-09-19 Thread serguei.spit...@oracle.com
Hi Paul, I have almost the same comments as David:  - the same two spots of changes identified  - the addition of the default method was expected  - the change in test is a surprise (I also doubt, it is really needed)  - new CSR is needed Sorry, I forgot to remind about running the vmTestbase m

Re: RFR(S): 8228625: [TESTBUG] sun/tools/jhsdb/JShellHeapDumpTest.java fails with RuntimeException 'JShellToolProvider' missing from stdout/stderr

2019-09-19 Thread Chris Plummer
Here's a new webrev: http://cr.openjdk.java.net/~cjplummer/8228625/webrev.01/ I decided to add a 2 second delay to make sure the test always passes and avoids the issues in JDK-8230872. After some internal discussion there seems to be some consensus that SA is probably not very stable when at

Re: RFC: 8229160: Reimplement JvmtiRawMonitor to use PlatformMonitor

2019-09-19 Thread David Holmes
I've abandoned this effort and am instead taking a simpler approach under a new bug id: https://bugs.openjdk.java.net/browse/JDK-8231289 RFR coming in a few days. David - On 10/09/2019 6:52 pm, David Holmes wrote: On 10/09/2019 5:54 pm, David Holmes wrote: It turns out that polling for i

Re: RFR: 8185005: Improve performance of ThreadMXBean.getThreadInfo(long ids[], int maxDepth)

2019-09-19 Thread serguei.spit...@oracle.com
Hi Daniil, I think, it is better to grab the thread_lock just once at lazy initialization. It would look simpler, something, like this would work: void ThreadTable::lazy_initialize(const ThreadsList *threads) { if (_is_initialized) { return;

Re: RFR: 8185005: Improve performance of ThreadMXBean.getThreadInfo(long ids[], int maxDepth)

2019-09-19 Thread David Holmes
On 20/09/2019 12:54 pm, Daniil Titov wrote: Hi David, Thank you for reviewing this version of the fix. I agree with previous comments about the general race with is_exiting in terms of how this API behaves. But there's no change in that behaviour with your changes AFAICS. Could y

Re: RFR: 8185005: Improve performance of ThreadMXBean.getThreadInfo(long ids[], int maxDepth)

2019-09-19 Thread Daniil Titov
Hi David, Thank you for reviewing this version of the fix. >I agree with previous comments about the general race with is_exiting in >terms of how this API behaves. But there's no change in that behaviour >with your changes AFAICS. Could you please say am I right that you are refer

Re: RFR: 8185005: Improve performance of ThreadMXBean.getThreadInfo(long ids[], int maxDepth)

2019-09-19 Thread David Holmes
Hi Daniil, On 20/09/2019 10:30 am, Daniil Titov wrote: Hi David and Serguei, Please review new version of the fix that includes the changes Serguei suggested: 1. If racing threads initialize the thread table only one of these threads will populate the table with the threads from the thread

Re: RFR: 8185005: Improve performance of ThreadMXBean.getThreadInfo(long ids[], int maxDepth)

2019-09-19 Thread Daniil Titov
Hi David and Serguei, Please review new version of the fix that includes the changes Serguei suggested: 1. If racing threads initialize the thread table only one of these threads will populate the table with the threads from the thread list 2. The code that adds the thread to the tread table

Re: RFR (M): 8231209: [REDO] ThreadMXBean::getThreadAllocatedBytes() can be quicker for self thread

2019-09-19 Thread David Holmes
On 20/09/2019 9:33 am, Mandy Chung wrote: On 9/19/19 4:06 PM, David Holmes wrote: 2. You implemented the new method in the test class. I don't understand why you did that. The test can't be calling the new method. Now that it is a default method we will get past the compilation failure that

Re: RFR (M): 8231209: [REDO] ThreadMXBean::getThreadAllocatedBytes() can be quicker for self thread

2019-09-19 Thread Mandy Chung
On 9/19/19 4:06 PM, David Holmes wrote: 2. You implemented the new method in the test class. I don't understand why you did that. The test can't be calling the new method. Now that it is a default method we will get past the compilation failure that caused the problem. So no change to the

Re: RFR (M): 8231209: [REDO] ThreadMXBean::getThreadAllocatedBytes() can be quicker for self thread

2019-09-19 Thread David Holmes
Hi Paul, On 20/09/2019 2:52 am, Hohensee, Paul wrote: More formally, Bug: https://bugs.openjdk.java.net/browse/JDK-8231209 Webrev: http://cr.openjdk.java.net/~phh/8231209/webrev.00/ I'm assuming there are only two changes here: 1. The new method is now a default method that throws UOE. That

Re: RFR(S) 8230677: Should disable Escape Analysis if JVMTI capability can_get_owned_monitor_info was taken

2019-09-19 Thread David Holmes
On 20/09/2019 2:42 am, Reingruber, Richard wrote: Hi David, thanks for looking at this issue. And my appologies for the lengthy mail... > > The JVMTI functions GetOwnedMonitorInfo() and GetOwnedMonitorStackDepthInfo() can be used to > > retrieve objects locked by a thread. In terms of es

Re: RFR (M): 8231209: [REDO] ThreadMXBean::getThreadAllocatedBytes() can be quicker for self thread

2019-09-19 Thread Mandy Chung
On 9/19/19 9:52 AM, Hohensee, Paul wrote: More formally, Bug: https://bugs.openjdk.java.net/browse/JDK-8231209 Webrev: http://cr.openjdk.java.net/~phh/8231209/webrev.00/ The new metric you added is an attribute (not an operation).  So the change in ServerThreadMXBeanNew should call getLon

Re: RFR(S): 8228625: [TESTBUG] sun/tools/jhsdb/JShellHeapDumpTest.java fails with RuntimeException 'JShellToolProvider' missing from stdout/stderr

2019-09-19 Thread Chris Plummer
One thing I didn't mention before is that using "jshell> " as the line predicate does not work because jshell does not produce a \n after generating this prompt, thus it's not actually a line and no attempt will be made to match it, so eventually it times out. I fixed this by using a snippet fr

Re: RFR (M): 8231209: [REDO] ThreadMXBean::getThreadAllocatedBytes() can be quicker for self thread

2019-09-19 Thread Hohensee, Paul
More formally, Bug: https://bugs.openjdk.java.net/browse/JDK-8231209 Webrev: http://cr.openjdk.java.net/~phh/8231209/webrev.00/ Thanks, On 9/19/19, 9:44 AM, "serviceability-dev on behalf of Hohensee, Paul" wrote: Off by 2 error. Changed the subject to reflect 8231209. http://cr

RFR (M): 8231209: [REDO] ThreadMXBean::getThreadAllocatedBytes() can be quicker for self thread

2019-09-19 Thread Hohensee, Paul
Off by 2 error. Changed the subject to reflect 8231209. http://cr.openjdk.java.net/~phh/8231209/webrev.00/ Paul On 9/19/19, 6:31 AM, "Daniel D. Daugherty" wrote: > http://cr.openjdk.java.net/~phh/8231211/webrev.00/ The redo bug is 8231209. 8231211 is closed as a dup of 8231210.

RE: RFR(S) 8230677: Should disable Escape Analysis if JVMTI capability can_get_owned_monitor_info was taken

2019-09-19 Thread Reingruber, Richard
Hi David, thanks for looking at this issue. And my appologies for the lengthy mail... > > The JVMTI functions GetOwnedMonitorInfo() and GetOwnedMonitorStackDepthInfo() can be used to > > retrieve objects locked by a thread. In terms of escape analysis those references escape and > > optim

Re: RFR (M): 8207266: ThreadMXBean::getThreadAllocatedBytes() can be quicker for self thread

2019-09-19 Thread Daniel D. Daugherty
> http://cr.openjdk.java.net/~phh/8231211/webrev.00/ The redo bug is 8231209. 8231211 is closed as a dup of 8231210. Dan On 9/19/19 9:17 AM, Hohensee, Paul wrote: I'll have the default method throw UOE. That's the same as the other default methods do. The necessary test fix is in test/hots

Re: RFR (M): 8207266: ThreadMXBean::getThreadAllocatedBytes() can be quicker for self thread

2019-09-19 Thread Hohensee, Paul
I'll have the default method throw UOE. That's the same as the other default methods do. The necessary test fix is in test/hotspot/jtreg/vmTestbase/nsk/monitoring/share/server/ServerThreadMXBeanNew.java, which needs a new getCurrentThreadAllocatedBytes method, defined as public long getCur

Re: RFR: 8230857: Avoid reflection in sun.tools.common.ProcessHelper

2019-09-19 Thread David Holmes
Hi Christoph, On 19/09/2019 7:47 pm, Langer, Christoph wrote: Hi, @Erik, Magnus: Thanks for stepping in to explain things. Now back to the actual change: Is this ok then (@David)? Any other reviews from somebody else? http://cr.openjdk.java.net/~clanger/webrevs/8230857.1/ It seems okay. F

RE: RFR: 8230857: Avoid reflection in sun.tools.common.ProcessHelper

2019-09-19 Thread Langer, Christoph
Hi, @Erik, Magnus: Thanks for stepping in to explain things. Now back to the actual change: Is this ok then (@David)? Any other reviews from somebody else? http://cr.openjdk.java.net/~clanger/webrevs/8230857.1/ Thank you! Best regards Christoph > -Original Message- > From: David Holm