Use string concatenation instead of String.format

[augi]

We measured that if there is a lot of metrics to send then String.format can be bottleneck. The plain string concatenation is much more performant (at least 2 orders of magnitude). I don't use StringBuilder - the JVM decides itself if it's appropriate.

[scarytom]

Thanks for this.

It might be interesting to add a performance test to the codebase, do you have anything that you wrote whilst doing your measurements that might be turned into a test?

[rtyler]

I thought String builders were the way to go still on the JVM?

[augi]

@scarytom Sorry, I don't have any performance test. We made performance profiling on production. As our application doesn't make almost any I/O, we see all cpu-bottlenecks well.

@rtyler Java compiler decides how string concatenation will be compiled. In case of this simple concatenation, it will use StringBuilder. It would be necessary to use StringBuilder explicitly in case of loops, or more complicated code in general. See e.g. http://kaioa.com/node/59

[augi]

Hi guys, would you mind merging this pull-request, please?

[scarytom]

I'm curious: when you tested in prod, were you sending any messages with sampling values other than 1.0? I suspect that your changes will make performance worse where sampling is being used, as you are now using the stringValueOf function, which instantiates a new NumberFormat instance.for each message. I'm not sure how bad this will be, but I wondered if you had tried it?

[augi]

We used 1.0 only. I decided to use stringValueOf because I just needed something that converts double to String and this method is used from other methods in NonBlockingStatsDClient. You are right that it could be better to alter also stringValueOf method - I would prepare formatter in static constructor (if the instance is thread-safe - I don't know now). We haven't measured that.

[scarytom]

I'm pretty sure the formatter is not thread safe (I would need to RTFM again to confirm). As you say, other methods already utilise stringValueOf, but it was on my list of things to improve, so I'm a little disappointed to be making more things call it. However, I think I'll just merge your changes for now, as at least this consolidates things and leaves us one clear place for further improvement.

I'd still rather we established some performance tests before blindly making performance-driven changes, but I can follow your progression here and I'm happy to proceed.