On Fri, 24 Nov 2023 06:23:03 GMT, Alan Bateman wrote:
>> Jaikiran Pai has updated the pull request incrementally with two additional
>> commits since the last revision:
>>
>> - Alan's review suggestion - rename GetMonitoredHost to
>> ConcurrentGetMonitoredHost
>> - fix code comment
>
> src/j
> Can I please get a review of this change which proposes to fix the issue
> noted in https://bugs.openjdk.org/browse/JDK-8320687?
>
> As noted in the issue, the
> `sun.jvmstat.monitor.MonitoredHost.getMonitoredHost()` uses a shared instance
> of `java.util.ServiceLoader` to load `MonitoredHost
On Thu, 23 Nov 2023 16:42:35 GMT, Jaroslav Bachorik
wrote:
>> Please, review this fix for a corner case handling of `jmethodID` values.
>>
>> The issue is related to the interplay between `jmethodID` values and method
>> redefinitions. Each `jmethodID` value is effectively a pointer to a `Meth
On Thu, 23 Nov 2023 08:40:55 GMT, Stefan Karlsson wrote:
>> src/hotspot/share/runtime/vmOperations.cpp line 354:
>>
>>> 352: // alive. Filter out monitors with dead objects.
>>> 353: return;
>>> 354: }
>>
>> I don't think we need to do this, but even without this filtering I ran
On Thu, 23 Nov 2023 22:15:35 GMT, Afshin Zafari wrote:
>>> I still approve of this patch as it's better than what we had before. There
>>> are a lot of suggested improvements that can be done either in this PR or
>>> in a future RFE. `git blame` shows that this hasn't been touched since
>>> 20
On Fri, 24 Nov 2023 06:06:16 GMT, Jaikiran Pai wrote:
> Can I please get a review of this change which proposes to fix the issue
> noted in https://bugs.openjdk.org/browse/JDK-8320687?
>
> As noted in the issue, the
> `sun.jvmstat.monitor.MonitoredHost.getMonitoredHost()` uses a shared instanc
On Fri, 24 Nov 2023 06:06:16 GMT, Jaikiran Pai wrote:
> The fix proposes to guard the usage of the shared ServiceLoader instance
> through the monitoredHosts object monitor.
An alternative was to just create a new instance of `ServiceLoader` within the
method implementation whenever a `Monitor
Can I please get a review of this change which proposes to fix the issue noted
in https://bugs.openjdk.org/browse/JDK-8320687?
As noted in the issue, the
`sun.jvmstat.monitor.MonitoredHost.getMonitoredHost()` uses a shared instance
of `java.util.ServiceLoader` to load `MonitoredHostService` ser
On Thu, 23 Nov 2023 16:42:35 GMT, Jaroslav Bachorik
wrote:
>> Please, review this fix for a corner case handling of `jmethodID` values.
>>
>> The issue is related to the interplay between `jmethodID` values and method
>> redefinitions. Each `jmethodID` value is effectively a pointer to a `Meth
On Wed, 9 Aug 2023 07:46:58 GMT, Sergey Bylokhov wrote:
> The test uses this code to create a list of valid addresses for the localhost:
>
> String hostname = "localhost";
> List validAddresses = new LinkedList<>();
> validAddresses.add(hostname);
> Arrays.stream(
On Thu, 24 Aug 2023 14:09:46 GMT, Afshin Zafari wrote:
> The `find` method now is
> ```C++
> template
> int find(T* token, bool f(T*, E)) const {
> ...
>
> Any other functions which use this are also changed.
> Local linux-x64-debug hotspot:tier1 passed. Mach5 tier1 build on linux and
> Window
On Sun, 29 Oct 2023 08:07:55 GMT, Kim Barrett wrote:
>> I still approve of this patch as it's better than what we had before. There
>> are a lot of suggested improvements that can be done either in this PR or in
>> a future RFE. `git blame` shows that this hasn't been touched since 2008, so
>>
On Thu, 23 Nov 2023 18:03:33 GMT, Thomas Stuefe wrote:
> > > I'm not a big fan of this approach. We accumulate more and more "#ifdef
> > > AIX" in shared code because of many recent AIX additions. No other
> > > platform has such a large ifdef footprint in shared code.
> > > I argue that all of
On Thu, 23 Nov 2023 17:05:29 GMT, suchismith1993 wrote:
> > I'm not a big fan of this approach. We accumulate more and more "#ifdef
> > AIX" in shared code because of many recent AIX additions. No other platform
> > has such a large ifdef footprint in shared code.
> > I argue that all of this s
On Thu, 23 Nov 2023 17:17:34 GMT, Daniel Fuchs wrote:
>>> I see Integer.getInteger handles the obvious Exceptions, so specifying a
>>> strange value does not immediately break us.
>>
>> Oh - right. It's `Integer.getInteger`, not `Integer.parseInt` Ok, then - I
>> withdraw my first comment :-)
On Thu, 23 Nov 2023 17:14:52 GMT, Daniel Fuchs wrote:
>>> IIRC, the default agent uses / may use its own socket factories - are we
>>> still coming here even with those factories?
>>
>> We do get here, yes (not saying you can't customise your way out of getting
>> here). This is a stack from
On Thu, 23 Nov 2023 12:43:33 GMT, Kevin Walls wrote:
>>> An exception here will prevent the class from being initialized...
>>
>> Maybe we can break it, but this new property is following the same pattern I
>> think as the neighbouring file
>> src/java.rmi/share/classes/sun/rmi/transport/tcp/T
On Wed, 22 Nov 2023 16:24:24 GMT, suchismith1993 wrote:
>> J2SE agent does not start and throws error when it tries to find the shared
>> library ibm_16_am.
>> After searching for ibm_16_am.so ,the jvm agent throws and error as dll_load
>> fails.It fails to identify the shared library ibm_16_am
> Please, review this fix for a corner case handling of `jmethodID` values.
>
> The issue is related to the interplay between `jmethodID` values and method
> redefinitions. Each `jmethodID` value is effectively a pointer to a `Method`
> instance. Once that method gets redefined, the `jmethodID`
On Thu, 23 Nov 2023 16:02:18 GMT, Thomas Stuefe wrote:
> sorry for not reading the description carefully.
No worries. This is a rather convoluted issue. I don't mind being challenged -
I am quite new to this part of the code and I don't want to accidentally break
something ...
-
On Thu, 23 Nov 2023 15:46:10 GMT, Jaroslav Bachorik
wrote:
>> Sadly, this is not async-profiler specific. The same issue can be observed
>> by JVMTI only code grabbing a stacktrace.
>> What do you mean exactly by 'conditional'? Introducing a new JVM flag or
>> something else?
>
> Ok, I see now
> Please, review this fix for a corner case handling of `jmethodID` values.
>
> The issue is related to the interplay between `jmethodID` values and method
> redefinitions. Each `jmethodID` value is effectively a pointer to a `Method`
> instance. Once that method gets redefined, the `jmethodID`
On Thu, 23 Nov 2023 16:11:32 GMT, Afshin Zafari wrote:
>> The `find` method now is
>> ```C++
>> template
>> int find(T* token, bool f(T*, E)) const {
>> ...
>>
>> Any other functions which use this are also changed.
>> Local linux-x64-debug hotspot:tier1 passed. Mach5 tier1 build on linux and
On Thu, 23 Nov 2023 11:52:38 GMT, Stefan Karlsson wrote:
>> In the rewrites made for:
>> [JDK-8318757](https://bugs.openjdk.org/browse/JDK-8318757) `VM_ThreadDump
>> asserts in interleaved ObjectMonitor::deflate_monitor calls`
>>
>> I removed the filtering of *owned ObjectMonitors with dead obj
On Thu, 23 Nov 2023 14:17:18 GMT, Magnus Ihse Bursie wrote:
>> Stefan Karlsson has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Tweak test comments
>
> Build changes are trivially fine.
Thanks @magicus. I'm removing the build label now.
> In the rewrites made for:
> [JDK-8318757](https://bugs.openjdk.org/browse/JDK-8318757) `VM_ThreadDump
> asserts in interleaved ObjectMonitor::deflate_monitor calls`
>
> I removed the filtering of *owned ObjectMonitors with dead objects*. The
> reasoning was that you should never have an owned
> The `find` method now is
> ```C++
> template
> int find(T* token, bool f(T*, E)) const {
> ...
>
> Any other functions which use this are also changed.
> Local linux-x64-debug hotspot:tier1 passed. Mach5 tier1 build on linux and
> Windows passed.
Afshin Zafari has updated the pull request inc
On Thu, 23 Nov 2023 14:15:54 GMT, Thomas Stuefe wrote:
>> @dholmes-ora
>>> Can't we just check method->method_holder() for null rather than passing in
>>> a parameter like this?
>>
>> I have removed the argument and I am now performing the check for
>> `method_holder() != nullptr` as recommen
On Thu, 23 Nov 2023 15:19:43 GMT, Jaroslav Bachorik
wrote:
>> src/hotspot/share/oops/instanceKlass.cpp line 541:
>>
>>> 539: assert (!method->on_stack(), "shouldn't be called with methods
>>> on stack");
>>> 540: // Do the pointer maintenance before releasing the metadata
>>> 541:
On Thu, 23 Nov 2023 14:15:54 GMT, Thomas Stuefe wrote:
>> @dholmes-ora
>>> Can't we just check method->method_holder() for null rather than passing in
>>> a parameter like this?
>>
>> I have removed the argument and I am now performing the check for
>> `method_holder() != nullptr` as recommen
On Thu, 23 Nov 2023 14:20:48 GMT, Thomas Stuefe wrote:
>> Jaroslav Bachorik has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Add exhaustive check for method holder availability (1)
>
> src/hotspot/share/oops/instanceKlass.cpp line 541:
>
On Mon, 20 Nov 2023 22:08:49 GMT, Coleen Phillimore wrote:
>> Jaroslav Bachorik has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Add exhaustive check for method holder availability (1)
>
> src/hotspot/share/classfile/classFileParser.cpp l
On Thu, 23 Nov 2023 13:37:41 GMT, Jaroslav Bachorik
wrote:
>> Please, review this fix for a corner case handling of `jmethodID` values.
>>
>> The issue is related to the interplay between `jmethodID` values and method
>> redefinitions. Each `jmethodID` value is effectively a pointer to a `Meth
On Thu, 23 Nov 2023 11:52:38 GMT, Stefan Karlsson wrote:
>> In the rewrites made for:
>> [JDK-8318757](https://bugs.openjdk.org/browse/JDK-8318757) `VM_ThreadDump
>> asserts in interleaved ObjectMonitor::deflate_monitor calls`
>>
>> I removed the filtering of *owned ObjectMonitors with dead obj
On Thu, 23 Nov 2023 08:44:08 GMT, Jaroslav Bachorik
wrote:
>> Jaroslav Bachorik has updated the pull request incrementally with three
>> additional commits since the last revision:
>>
>> - Clean up imports
>> - Simplify Method::clear_jmethod_id()
>> - Use correct copyrights
>
> @dholmes-ora
On Thu, 23 Nov 2023 11:52:38 GMT, Stefan Karlsson wrote:
>> In the rewrites made for:
>> [JDK-8318757](https://bugs.openjdk.org/browse/JDK-8318757) `VM_ThreadDump
>> asserts in interleaved ObjectMonitor::deflate_monitor calls`
>>
>> I removed the filtering of *owned ObjectMonitors with dead obj
On Thu, 23 Nov 2023 13:32:23 GMT, Afshin Zafari wrote:
>>> > Thanks for making this change.
>>> > I'd like to suggest the following cleanups, some documentation, and a few
>>> > tests:
>>> > [20d4502](https://github.com/openjdk/jdk/commit/20d4502471ba396ae395512cfa3dab3f87555421)
>>> > I think
On Wed, 22 Nov 2023 23:08:36 GMT, Jonathan Joo wrote:
>> 8315149: Add hsperf counters for CPU time of internal GC threads
>
> Jonathan Joo has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Cleanup and address comments
src/hotspot/share/gc/g1
> Please, review this fix for a corner case handling of `jmethodID` values.
>
> The issue is related to the interplay between `jmethodID` values and method
> redefinitions. Each `jmethodID` value is effectively a pointer to a `Method`
> instance. Once that method gets redefined, the `jmethodID`
On Thu, 23 Nov 2023 11:53:28 GMT, Stefan Karlsson wrote:
>>> Thanks for making this change.
>>>
>>> I'd like to suggest the following cleanups, some documentation, and a few
>>> tests:
>>> [20d4502](https://github.com/openjdk/jdk/commit/20d4502471ba396ae395512cfa3dab3f87555421)
>>>
>>> I thin
> The `find` method now is
> ```C++
> template
> int find(T* token, bool f(T*, E)) const {
> ...
>
> Any other functions which use this are also changed.
> Local linux-x64-debug hotspot:tier1 passed. Mach5 tier1 build on linux and
> Windows passed.
Afshin Zafari has updated the pull request inc
> Please, review this fix for a corner case handling of `jmethodID` values.
>
> The issue is related to the interplay between `jmethodID` values and method
> redefinitions. Each `jmethodID` value is effectively a pointer to a `Method`
> instance. Once that method gets redefined, the `jmethodID`
On Thu, 23 Nov 2023 11:35:47 GMT, Mark Sheppard wrote:
> In any case the change looks fine. Do you have a test c.f. bug comment
Thanks @msheppar yes I agree with the comments.
For testing I got as far as a test that often cannot see the timeout as we are
too quick for a 1ms timeout. I was thin
On Thu, 23 Nov 2023 12:40:18 GMT, Kevin Walls wrote:
>> IIRC, the default agent uses / may use its own socket factories - are we
>> still coming here even with those factories?
>
>> An exception here will prevent the class from being initialized...
>
> Maybe we can break it, but this new proper
On Thu, 23 Nov 2023 11:37:12 GMT, Daniel Fuchs wrote:
>> src/java.rmi/share/classes/sun/rmi/transport/tcp/TCPDirectSocketFactory.java
>> line 46:
>>
>>> 44: private static final int connectTimeout =// default 1 minute
>>> 45: AccessController.doPrivileged((PrivilegedAction) () -
On Thu, 23 Nov 2023 11:00:32 GMT, Afshin Zafari wrote:
> > Thanks for making this change.
> > I'd like to suggest the following cleanups, some documentation, and a few
> > tests:
> > [20d4502](https://github.com/openjdk/jdk/commit/20d4502471ba396ae395512cfa3dab3f87555421)
> > I think it might b
> In the rewrites made for:
> [JDK-8318757](https://bugs.openjdk.org/browse/JDK-8318757) `VM_ThreadDump
> asserts in interleaved ObjectMonitor::deflate_monitor calls`
>
> I removed the filtering of *owned ObjectMonitors with dead objects*. The
> reasoning was that you should never have an owned
On Thu, 23 Nov 2023 00:48:19 GMT, David Holmes wrote:
> > Previously, the locked monitors never got unlocked because the
> > ObjectMonitor iterator never exposed these monitors to the JNI detach code
>
> I had not realized that. It explains some confusion in a separate issue I had
> been looki
On Thu, 23 Nov 2023 02:10:41 GMT, David Holmes wrote:
>> Stefan Karlsson has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - Tweaked comment in test
>> - Rewrite tests
>
> test/hotspot/jtreg/runtime/Monitor/libIterateMonitorWithDeadObject
> In the rewrites made for:
> [JDK-8318757](https://bugs.openjdk.org/browse/JDK-8318757) `VM_ThreadDump
> asserts in interleaved ObjectMonitor::deflate_monitor calls`
>
> I removed the filtering of *owned ObjectMonitors with dead objects*. The
> reasoning was that you should never have an owned
On Tue, 21 Nov 2023 17:57:32 GMT, Kevin Walls wrote:
> RMI Connections (in general) should use a timeout on the Socket connect call
> by default.
>
> JMX connections use RMI and some connection failures never terminate. The
> hang described in 8316649 is hard to reproduce manually: the descri
On Thu, 23 Nov 2023 11:35:11 GMT, Daniel Fuchs wrote:
>> RMI Connections (in general) should use a timeout on the Socket connect call
>> by default.
>>
>> JMX connections use RMI and some connection failures never terminate. The
>> hang described in 8316649 is hard to reproduce manually: the
On Tue, 21 Nov 2023 17:57:32 GMT, Kevin Walls wrote:
> RMI Connections (in general) should use a timeout on the Socket connect call
> by default.
>
> JMX connections use RMI and some connection failures never terminate. The
> hang described in 8316649 is hard to reproduce manually: the descri
On Thu, 23 Nov 2023 02:07:59 GMT, David Holmes wrote:
>> In the rewrites made for:
>> [JDK-8318757](https://bugs.openjdk.org/browse/JDK-8318757) `VM_ThreadDump
>> asserts in interleaved ObjectMonitor::deflate_monitor calls`
>>
>> I removed the filtering of *owned ObjectMonitors with dead object
On Thu, 23 Nov 2023 10:52:35 GMT, Afshin Zafari wrote:
>> The `find` method now is
>> ```C++
>> template
>> int find(T* token, bool f(T*, E)) const {
>> ...
>>
>> Any other functions which use this are also changed.
>> Local linux-x64-debug hotspot:tier1 passed. Mach5 tier1 build on linux and
> Please, review this fix for a corner case handling of `jmethodID` values.
>
> The issue is related to the interplay between `jmethodID` values and method
> redefinitions. Each `jmethodID` value is effectively a pointer to a `Method`
> instance. Once that method gets redefined, the `jmethodID`
> The `find` method now is
> ```C++
> template
> int find(T* token, bool f(T*, E)) const {
> ...
>
> Any other functions which use this are also changed.
> Local linux-x64-debug hotspot:tier1 passed. Mach5 tier1 build on linux and
> Windows passed.
Afshin Zafari has updated the pull request inc
On Thu, 23 Nov 2023 00:48:19 GMT, David Holmes wrote:
>> In the rewrites made for:
>> [JDK-8318757](https://bugs.openjdk.org/browse/JDK-8318757) `VM_ThreadDump
>> asserts in interleaved ObjectMonitor::deflate_monitor calls`
>>
>> I removed the filtering of *owned ObjectMonitors with dead object
On Thu, 23 Nov 2023 06:16:54 GMT, Thomas Stuefe wrote:
> If .a is a valid shared object format on AIX, this should be handled in
> `os::dll_load()`, and be done for all shared objects. If not, why do we try
> to load a static archive via dlload in this case but not in other cases?
In AIX, we
RMI Connections (in general) should use a timeout on the Socket connect call by
default.
JMX connections use RMI and some connection failures never terminate. The hang
described in 8316649 is hard to reproduce manually: the description says it can
be caused by a firewall that never returns a r
On Thu, 23 Nov 2023 01:38:57 GMT, David Holmes wrote:
>> In the rewrites made for:
>> [JDK-8318757](https://bugs.openjdk.org/browse/JDK-8318757) `VM_ThreadDump
>> asserts in interleaved ObjectMonitor::deflate_monitor calls`
>>
>> I removed the filtering of *owned ObjectMonitors with dead object
On Thu, 23 Nov 2023 01:29:24 GMT, David Holmes wrote:
>> In the rewrites made for:
>> [JDK-8318757](https://bugs.openjdk.org/browse/JDK-8318757) `VM_ThreadDump
>> asserts in interleaved ObjectMonitor::deflate_monitor calls`
>>
>> I removed the filtering of *owned ObjectMonitors with dead object
On Thu, 23 Nov 2023 08:08:51 GMT, suchismith1993 wrote:
> The JBS issue with respect to that has been closed. Need to check if that PR
> is required. Currently putting it on hold.
This response on the issue suggest otherwise:
: The JDK does not support dynamically loaded archive files
(.a fi
On Wed, 22 Nov 2023 23:08:36 GMT, Jonathan Joo wrote:
>> 8315149: Add hsperf counters for CPU time of internal GC threads
>
> Jonathan Joo has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Cleanup and address comments
I think this looks good
On Wed, 22 Nov 2023 01:21:15 GMT, David Holmes wrote:
>> Jaroslav Bachorik has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Rewerite the test to use RedefineClassHelper
>
> test/hotspot/jtreg/serviceability/jvmti/thread/GetStackTrace/GetS
On Thu, 23 Nov 2023 01:28:32 GMT, David Holmes wrote:
>> In the rewrites made for:
>> [JDK-8318757](https://bugs.openjdk.org/browse/JDK-8318757) `VM_ThreadDump
>> asserts in interleaved ObjectMonitor::deflate_monitor calls`
>>
>> I removed the filtering of *owned ObjectMonitors with dead object
On Mon, 20 Nov 2023 22:08:49 GMT, Coleen Phillimore wrote:
>> Jaroslav Bachorik has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Rewerite the test to use RedefineClassHelper
>
> src/hotspot/share/classfile/classFileParser.cpp line 5579:
>
On Wed, 22 Nov 2023 01:27:25 GMT, David Holmes wrote:
>> I see, holder is the right word and concept. So the parameter means
>> has_method_holder, in that the InstanceKlass has been fully parsed at the
>> point of clearing the jmethodIDs.
>
> Can't we just check `method->method_holder()` for n
On Thu, 23 Nov 2023 08:32:36 GMT, Jaroslav Bachorik
wrote:
>> Please, review this fix for a corner case handling of `jmethodID` values.
>>
>> The issue is related to the interplay between `jmethodID` values and method
>> redefinitions. Each `jmethodID` value is effectively a pointer to a `Meth
> Please, review this fix for a corner case handling of `jmethodID` values.
>
> The issue is related to the interplay between `jmethodID` values and method
> redefinitions. Each `jmethodID` value is effectively a pointer to a `Method`
> instance. Once that method gets redefined, the `jmethodID`
On Thu, 23 Nov 2023 01:27:24 GMT, David Holmes wrote:
>> In the rewrites made for:
>> [JDK-8318757](https://bugs.openjdk.org/browse/JDK-8318757) `VM_ThreadDump
>> asserts in interleaved ObjectMonitor::deflate_monitor calls`
>>
>> I removed the filtering of *owned ObjectMonitors with dead object
> Please, review this fix for a corner case handling of `jmethodID` values.
>
> The issue is related to the interplay between `jmethodID` values and method
> redefinitions. Each `jmethodID` value is effectively a pointer to a `Method`
> instance. Once that method gets redefined, the `jmethodID`
On Thu, 23 Nov 2023 05:49:41 GMT, Amit Kumar wrote:
> #16490
The JBS issue with respect to that has been closed. Need to check if that PR is
required. Currently putting it on hold.
-
PR Comment: https://git.openjdk.org/jdk/pull/16604#issuecomment-1823952398
73 matches
Mail list logo
