martint / jmxutils

Exporting JMX mbeans made easy
Apache License 2.0
173 stars 47 forks source link

Manually search methods instead of using Class.getDeclaredMethod() #56

Closed Randgalt closed 8 months ago

Randgalt commented 8 months ago

Class.getDeclaredMethod() will throw NoSuchMethodException when not found which requires the JDK to build a stacktrace, etc. This causes an unnecessary performance loss.

Note: this is currently a speculative change. This new code behaves somewhat differently than Class.getDeclaredMethod() but all tests pass.

martint commented 8 months ago

How measurable is the performance impact? I find it surprising that it would matter in practice. Generally, the number of classes that an application exposes via JMX is "small".

jklamer commented 8 months ago

In a guice context in an application with a large number of nested injectors and separate class loaders, the amount of exported metrics has started to ballon, and the CPU impact starts to have a more noticeable impact. Something on order of 80% of the CPU used by GuiceMBeanExporter in many cases goes to filling up the stack trace

Randgalt commented 8 months ago

We're not certain exactly what's going on tbh. A few thoughts:

I believe there is a significant potential for 1000s of stacktraces to be generated during Guice processing.

Maybe we can build a one-off version of JMXUtils that we can test with?

Randgalt commented 8 months ago

@martint and @jklamer I added a cache of a class's declared methods to avoid some redundant lookups.

Randgalt commented 8 months ago

Thanks @martint - do you mind doing a release soon?