RE: [External] : Re: RFD: Can we remove per-thread compiler stats now?
Would that be okay with you? Absolutely!
Re: [External] : Re: RFD: Can we remove per-thread compiler stats now?
On Mon, Mar 18, 2024 at 2:31 PM Kevin Walls wrote: > Right, the existing Deprecated tag had me thinking this was a Java SE > interface that had begun the deprecation process. > > But it's not a published API. > Great, it seems we have consensus that we can go ahead and remove these deprecated methods which return no useful data. Maybe we can go further: HotspotCompilationMBean and HotspotInternalMBean > look unused. They are not exposed in the standard PlatformMBeanServer. > > It seems we can! But since (a) this is my first serviceability contribution and (b) the motivations for removing other unused beans may differ and also be more controversial, I'd prefer to warm up with a PR narrowly focused on the deprecated per-thread stats. We can then follow up with a broader discussion of removal of unused Hotspot internal beans. Would that be okay with you? While looking for who/what uses these, I also found a recent mailing list > thread: > > "Question on why sun.management MBeans are not exported?" > > https://www.mail-archive.com/core-libs-dev@openjdk.org/msg19878.html > Thanks, that provides some very useful context. Cheers, Eirik.
RE: [External] : Re: RFD: Can we remove per-thread compiler stats now?
Right, the existing Deprecated tag had me thinking this was a Java SE interface that had begun the deprecation process. But it's not a published API. Maybe we can go further: HotspotCompilationMBean and HotspotInternalMBean look unused. They are not exposed in the standard PlatformMBeanServer. While looking for who/what uses these, I also found a recent mailing list thread: "Question on why sun.management MBeans are not exported?" https://www.mail-archive.com/core-libs-dev@openjdk.org/msg19878.html ..where the idea of removal also came up. Thanks Kevin From: Eirik Bjørsnøs Sent: 17 March 2024 18:43 To: Kevin Walls Cc: serviceability-dev@openjdk.org Subject: [External] : Re: RFD: Can we remove per-thread compiler stats now? On Fri, Mar 15, 2024 at 7:54 PM Kevin Walls mailto:kevin.wa...@oracle.com>> wrote: https://openjdk.org/jeps/277 "An API element should not be removed from the Java SE specification unless it has been delivered with an annotation of @Deprecated(forRemoval=true) in a previous version of Java SE." So deprecations need to be "upgraded" to forRemoval, this will take a few updates to get them removed. Hmm.. But if sun.management.HotspotCompilationMBean is an internal class, it's not defined in the Java SE Specification, right? My understanding when working on similar core-libs cleanups is that internal APIs can be removed directly without the need for a forRemoval step. HotspotCompilationMBean: @Deprecated public java.util.List getCompilerThreadStats(); .. need to consider whether a method is removed or just "degraded" (not return any information, or throw). I think this is already "degraded" in the sense that it does not return useful information (underlying counters are long removed). My intention here is to remove crufty leftover code. I think the best outcome is if we can remove the code so maintainers no longer need to see or reason about it. That said, I see sun.management.CompilationImpl[java.lang:type=Compilation] ..rather than HotspotCompilation. ManagementFactoryHelper.java: getCompilationMXBean() specifically creates a CompilationImpl, users don’t normally see the HotspotCompilation mbean. This and the rest goes into code and concepts I'm not very familiar with (yet!), so I'm not sure I understand the implications well. But do you think perhaps the whole HotspotCompilationMBean with implementation could be removed? I'm not sure I see any consumers within OpenJDK of this interface? The role of HotspotCompilationMBean needs more investigation, but basically yes to the idea. 8-) Good! It seems there is potential for some level of cleanup here, so I'll go ahead and make a PR where we can continue the discussion. Thanks, Eirik.
Re: RFD: Can we remove per-thread compiler stats now?
On Fri, Mar 15, 2024 at 7:54 PM Kevin Walls wrote: > https://openjdk.org/jeps/277 > > "An API element should not be removed from the Java SE specification > unless it has been delivered with an annotation of > @Deprecated(forRemoval=true) in a previous version of Java SE." > > > > So deprecations need to be "upgraded" to forRemoval, this will take a few > updates to get them removed. > Hmm.. But if sun.management.HotspotCompilationMBean is an internal class, it's not defined in the Java SE Specification, right? My understanding when working on similar core-libs cleanups is that internal APIs can be removed directly without the need for a forRemoval step. HotspotCompilationMBean: > > > > @Deprecated > > public java.util.List getCompilerThreadStats(); > > > > .. need to consider whether a method is removed or just "degraded" (not > return any information, or throw). > I think this is already "degraded" in the sense that it does not return useful information (underlying counters are long removed). My intention here is to remove crufty leftover code. I think the best outcome is if we can remove the code so maintainers no longer need to see or reason about it. > That said, I see > sun.management.CompilationImpl[java.lang:type=Compilation] ..rather than > HotspotCompilation. > > > > ManagementFactoryHelper.java: getCompilationMXBean() specifically creates > a CompilationImpl, users don’t normally see the HotspotCompilation mbean. > This and the rest goes into code and concepts I'm not very familiar with (yet!), so I'm not sure I understand the implications well. But do you think perhaps the whole HotspotCompilationMBean with implementation could be removed? I'm not sure I see any consumers within OpenJDK of this interface? > The role of HotspotCompilationMBean needs more investigation, but > basically yes to the idea. 8-) > Good! It seems there is potential for some level of cleanup here, so I'll go ahead and make a PR where we can continue the discussion. Thanks, Eirik.
RE: RFD: Can we remove per-thread compiler stats now?
Hi, Sounds like a good plan. I don't think it causes any problems, we aren't creating them now. These counters are in jdk8u. The jcmd "jcmd PID PerfCounter.print" shows them when attaching to a JDK8 process. But if nothing relies on them being there… https://openjdk.org/jeps/277 "An API element should not be removed from the Java SE specification unless it has been delivered with an annotation of @Deprecated(forRemoval=true) in a previous version of Java SE." So deprecations need to be "upgraded" to forRemoval, this will take a few updates to get them removed. HotspotCompilationMBean: @Deprecated public java.util.List getCompilerThreadStats(); .. need to consider whether a method is removed or just "degraded" (not return any information, or throw). That said, I see sun.management.CompilationImpl[java.lang:type=Compilation] ..rather than HotspotCompilation. ManagementFactoryHelper.java: getCompilationMXBean() specifically creates a CompilationImpl, users don’t normally see the HotspotCompilation mbean. Elsewhere in that file, registerInternalMBeans() does: 517 // CompilationMBean may not exist 518 if (getCompilationMXBean() != null) { <-- returns new CompilationImpl(jvm); 519 addMBean(mbs, getHotspotCompilationMBean(), <-- returns new HotspotCompilation(jvm) 520 HOTSPOT_COMPILATION_MBEAN_NAME); <-- which is "sun.management:type=HotspotCompilation" 521 } HotspotInternal (HotspotInternalMBean) calls ManagementFactoryHelper.registerInternalMBeans(server). HotspotInternal is a non-public API, and might be the way of acquiring a HotspotCompilation. The role of HotspotCompilationMBean needs more investigation, but basically yes to the idea. 8-) Thanks Kevin From: serviceability-dev On Behalf Of Eirik Bjørsnøs Sent: 14 March 2024 17:06 To: serviceability-dev@openjdk.org Subject: RFD: Can we remove per-thread compiler stats now? Hello, Per-thread compiler performance counters were removed in JDK-8134607 [1] (JDK 9). The corresponding sun.management API was marked @Deprecated since it no longer returns any useful counter data. Has time come to remove this API now? My understanding is that since sun.management is an internal package this should be low risk. But this being JMX-related, is there maybe some concern to this being used in some remote/RMI scenario? If not, I'd like to propose a PR to remove the following: * Class sun.management.CompilerThreadStat * Nested class sun.management.HotspotCompilation.CompilerThreadInfo * Field sun.management.HotspotCompilation.threads and its initialization * Method sun.management.HotspotCompilation.getCompilerThreadStats() and its definition in HotspotCompilationMBean Let me know what you think Cheers, Eirik :-) [1] https://bugs.openjdk.org/browse/JDK-8134607
RFD: Can we remove per-thread compiler stats now?
Hello, Per-thread compiler performance counters were removed in JDK-8134607 [1] (JDK 9). The corresponding sun.management API was marked @Deprecated since it no longer returns any useful counter data. Has time come to remove this API now? My understanding is that since sun.management is an internal package this should be low risk. But this being JMX-related, is there maybe some concern to this being used in some remote/RMI scenario? If not, I'd like to propose a PR to remove the following: * Class sun.management.CompilerThreadStat * Nested class sun.management.HotspotCompilation.CompilerThreadInfo * Field sun.management.HotspotCompilation.threads and its initialization * Method sun.management.HotspotCompilation.getCompilerThreadStats() and its definition in HotspotCompilationMBean Let me know what you think Cheers, Eirik :-) [1] https://bugs.openjdk.org/browse/JDK-8134607