search

tim-group / java-statsd-client

a java statsd client library
Other
275 stars 139 forks source link

Using async queue instead of submit new Runnable each time #25

Open maurofran opened 10 years ago

maurofran commented 10 years ago

Looking at the code, I noticed that every time a message is sent to the statsd server, a new Runnable instance is created. I think better performance can be achieved using a single Runnable and a Queue in which messages are put, waiting to be sent to the statsd server.

scarytom commented 10 years ago

Thanks for submitting this. I was thinking of doing something similar myself as part of issue #16.

Would it be possible for you to force-push a better pull request without tangential changes to the signature of messageFor (where you have changed value back to being an Object) and leaving (Locale)null for the String.format call. A lot of your commits seem to make changes then reverse them, so it would be good if you could flatten the commits, rebase, then force-push back.

I'm happy to review and consider pulling your request, but I don't like multiple things changing at once. I haven't looked in depth yet, but by using a BlockingQueue, you are probably violating the non-blocking contract of this library.

maurofran commented 10 years ago

Yes... I'have done few experiments (one used in order to also use StringBuilder instead of String.format) but it breaks the unit tests. I will make the changes you ask for in the next days: I will make a single pull request with only the changes using the Queue.

scarytom commented 10 years ago

OK, Thanks.

Do have a check before you start, as I may make some changes for #16 in the next few days.

michaelsembwever commented 10 years ago

A faster approach is using the lmax-disruptor

See https://github.com/finn-no/statsd-lmax-disruptor-client/commit/5b0e893bbd7cc2a968333b3007e294f7adea97f6

Or the branch for this work, https://github.com/finn-no/statsd-lmax-disruptor-client/tree/feature/lmax-disruptor

we've been using this in production at Norway's busiest website for 5 months now.

scarytom commented 10 years ago

I'm not sure I want to introduce a dependency on a 3rd party library like the lmax-disruptor. I like to think of the java-statsd-client as a small self-contained library without dependencies outside core Java.

waynr commented 7 years ago

https://github.com/DataDog/java-dogstatsd-client appears to be a more actively maintained version of this project, I recommend submitting your patch there if you have not already done so.