RFD: Remove Hotspot-internal MBeans from sun.management
Hi, Last September, Volker shared the observation that we have Hotspot-internal MBeans in sun.management which are strongly encapsulated and not used internally by OpenJDK besides their unit tests: https://www.mail-archive.com/core-libs-dev@openjdk.org/msg19878.html A summary of the email thread: Mandy pointed out: We added these HotSpot internal MBeans in JDK 5 to expose the internal > metrics. Most of these internal metrics are exposed via jstat tool too. > We didn't receive much feedback regarding these HotSpot internal MBeans. > Removing them is fine and good cleanup effort. Alan made a similar point: It's left over from experiments on exposing some internal metrics, I think > during JDK 5. It's code that should probably have been removed a long time > ago. Kirk P raised a concern: It would be a shame to lose these metrics because many of them have been > very > useful over time and some would be even more useful with some > modifications. > To which Mandy responded: What we're referring to is to remove sun.management.Hotspot*, the internal > MBeans which are never exposed and registered in the platform MBeanServer. > The internal metrics in HotSpot VM should be retained as they are exposed > through other ways like jstat, GC logs, etc. The email thread seems to have ended here without further action taken. My interpretation of the above is that we have a consensus that these Hotspot-internal MBeans can be removed. Since I was not part of the initial discussion and some time has passed, I'd like some confirmation that my interpretation is correct. Would a PR to remove these APIs be welcome? (This would remove HotspotInternalMBean, HotspotMemoryMBean, HotspotRuntimeMBean, HotspotThreadMBean, with associated implementation, factory methods, tests and probably also some native code in libmanagement. Details can be discussed in a PR) Cheers, Eirik.
Integrated: 8328341: Remove deprecated per-thread compiler stats in sun.management
On Mon, 18 Mar 2024 09:42:13 GMT, Eirik Bjørsnøs wrote: > Please review this cleanup PR which removes per-thread compiler stats from > `sun.management` > > This removes: > > * The deprecated interface method > `HotspotCompilationMBean.getCompilerThreadStats()` along with the > implementation method in `HotspotCompilation` > * The class returned by these methods, `CompilerThreadStat` > * The nested class `HotspotCompilation.CompilerThreadInfo` which now falls > out of use > * The field `HotspotCompilation.threads`, along with its initialization in > `initCompilerCounters` > > This was initially discussed here: > https://mail.openjdk.org/pipermail/serviceability-dev/2024-March/054589.html > > Testing and verification: As this is purely a removal and cleanup PR of > unused code, no updates are made on the testing side. I have verified that > the string `getCompilerThreadStats` is not found in OpenJDK after this PR. This pull request has now been integrated. Changeset: 9214a62f Author:Eirik Bjørsnøs URL: https://git.openjdk.org/jdk/commit/9214a62f26917162b3ff2144a1f3f9cde3b808fa Stats: 149 lines in 3 files changed: 0 ins; 147 del; 2 mod 8328341: Remove deprecated per-thread compiler stats in sun.management Reviewed-by: kevinw - PR: https://git.openjdk.org/jdk/pull/18344
Re: RFR: 8328341: Remove deprecated per-thread compiler stats in sun.management
On Mon, 18 Mar 2024 09:42:13 GMT, Eirik Bjørsnøs wrote: > Please review this cleanup PR which removes per-thread compiler stats from > `sun.management` > > This removes: > > * The deprecated interface method > `HotspotCompilationMBean.getCompilerThreadStats()` along with the > implementation method in `HotspotCompilation` > * The class returned by these methods, `CompilerThreadStat` > * The nested class `HotspotCompilation.CompilerThreadInfo` which now falls > out of use > * The field `HotspotCompilation.threads`, along with its initialization in > `initCompilerCounters` > > This was initially discussed here: > https://mail.openjdk.org/pipermail/serviceability-dev/2024-March/054589.html > > Testing and verification: As this is purely a removal and cleanup PR of > unused code, no updates are made on the testing side. I have verified that > the string `getCompilerThreadStats` is not found in OpenJDK after this PR. Thanks for reviewing, Kevin! - PR Comment: https://git.openjdk.org/jdk/pull/18344#issuecomment-2007634465
RFR: 8328341: Remove deprecated per-thread compiler stats in sun.management
Please review this cleanup PR which removes per-thread compiler stats from `sun.management` This removes: * The deprecated interface method `HotspotCompilationMBean.getCompilerThreadStats()` along with the implementation method in `HotspotCompilation` * The class returned by these methods, `CompilerThreadStat` * The nested class `HotspotCompilation.CompilerThreadInfo` which now falls out of use * The field `HotspotCompilation.threads`, along with its initialization in `initCompilerCounters` Testing and verification: As this is purely a removal and cleanup PR of unused code, no updates are made on the testing side. I have verified that the string `getCompilerThreadStats` is not found in OpenJDK after this PR. - Commit messages: - Remove HotspotCompilationMBean.getCompilerThreadStats() with associated implementation code Changes: https://git.openjdk.org/jdk/pull/18344/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=18344&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8328341 Stats: 149 lines in 3 files changed: 0 ins; 147 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/18344.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18344/head:pull/18344 PR: https://git.openjdk.org/jdk/pull/18344
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: 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.
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
Re: [External] : Re: JEP draft: Disallow the Dynamic Loading of Agents by Default
>
> Keep in mind two things:
>
> 1. Dynamically loaded agents are more limited in their capabilities than
> agents loaded at startup because redefinition/retransformation is limited
> to changing the body of existing methods. Redefinition can only fix issues
> if you’re lucky.
>
> 2. Java offers no general mechanism to make patches applied through
> redefinition persistent. They are reverted at the next startup.
>
Ron,
My concern was more the observation that the Summary of the draft JEP can
be misunderstood, (It seems to have happened in this thread):
Disallow the dynamic loading of agents into a running JVM by default.
> Agents are used by profiling tools to instrument Java applications, but
> agents can also be misused to undermine the integrity of the Java Platform.
I think the second sentence here ("agents can also be misused..") was meant
to refer specifically to dynamically loaded agents, not agents in general.
To help avoid confusion, this sentence could be updated to clarify that
it's the dynamic loading that is misused, not the agent mechanism per se.
Something like: "but *dynamically loaded agents* can also be misused to.."
I know the leading sentence defines the context, but I think a bit of
redunancy here would help reduce misunderstandings and prevent people from
thinking "now they're taking away agents too".
Thanks,
Eirik.
Re: JEP draft: Disallow the Dynamic Loading of Agents by Default
> > >Agents are used by profiling tools to instrument Java applications, > >but agents can also be misused to undermine the integrity of the > >Java Platform. > > I don't think it is fair to assume that profilers are the only "valid" > use case for agents and imply that all other use cases are a mis-use > of the API. > First, I don't read the JEP as implying that all non-profiler use cases are misuse. Having said that, I do think that agents can in fact strengthen the integrity of the platform. Case in point is that when the Java serialization vulnerabilities hit around 2015, I could very quickly ( a few hours) whip together the "NotSoSerial" serialization firewall agent [1] to efficiently prevent exploits. I later got word that a large CMS vendor deployed it to their platform which included some of the world's busiest websites. I don't know if they used the attach mechanism to plug their serialization holes, but they surely could at the time. With microservices gaining popularity over the years, restarts are probably more common and automated now, including configuration of JVM options. So attaching to long-running instances to prevent restarts is probably becoming less useful over time. The agent misuse that the JEP is referring to here is perhaps mostly concerning libraries using the attach mechanism to get access they otherwise would not have in a running JVM? Perhaps the JEP could be updated to be more clear on this? Cheers, Eirik. [1] https://github.com/kantega/notsoserial/
