prometheus / jmx_exporter

A process for exposing JMX Beans via HTTP for Prometheus consumption
Apache License 2.0
3.06k stars 1.2k forks source link

Estimate StringBuilder capacity for exported names #919

Open nicktelford opened 9 months ago

nicktelford commented 9 months ago

When we generate metric names, we use the default StringBuilder constructor, which allocates a 16 character buffer.

Most metrics will exceed this, especially when we start adding attributes to it.

This causes StringBuilder to "grow", automatically, by allocating a new array and copying its contents over to it. Each time it only grows enough to contain the appended String, which means we need to grow on almost every append call.

While allocating new memory is cheap in the JVM, copying memory is not. These copies add up, especially with a large number of long-named metrics with many attributes.

Instead, we can calculate the necessary capacity of the StringBuilder up-front, which should avoid doing any copying during append.

Signed-off-by: Nick Telford nick.telford@gmail.com

nicktelford commented 9 months ago

See the attached flame graph, which demonstrates the memory being allocated and copied by StringBuilder.append() under recordBean.

Note: I'm pretty sure the JVM is inlining Receiver#defaultExport into JmxCollector#recordBean, which is why defaultExport is missing from the stack.

image

nicktelford commented 9 months ago

I'm no longer convinced that the lack of estimating StringBuilder size is the main reason for all these copies. When StringBuilder expands, it always at least doubles its current size, which should result in relatively minimal copies in this particular case.

I think the primary cause of the flame graph above is actually simply poor optimizations of String concatenation in code compiled under Java 8. See #920 for more details.

Since computing the exact size up-front requires iterating the attributes, it might not actually be much more efficient to do this.

However, since copying memory is generally less efficient than iterating a linked list, I'm going to leave this PR open, as it will probably provide a very modest performance improvement.