Use ThreadLocalRandom vs Math.random #16
Conversation
There is an atomic compare and set loop in `Math.random` that can cause performance issues. You can find many postings on the performance aspects of this change http://blog.anvard.org/articles/2014/01/12/concurrent-random.html This change requires a minimum version change from Java 1.6 to 1.7
|
@jasonculverhouse The travis build failed because it's building against openjdk6: https://travis-ci.org/DataDog/java-dogstatsd-client/builds/165588084 If you update the PR to remove this line, it will probably succeed: https://github.com/DataDog/java-dogstatsd-client/blob/master/.travis.yml#L6 @masci That raises the question of whether Java 6 should still be supported. Given that it has been EOL for some time now, I think it can be safely removed. What do you think? |
|
@robinst You version your client. I don't see any issue with making the newer version work with java 7+ |
|
@jasonculverhouse I agree. I was just asking @masci's input as well. So, on your branch, can you remove the You can do it online, click edit here and remove the line: https://github.com/jasonculverhouse/java-dogstatsd-client/blob/jason/threadLocalRandom/.travis.yml That would bring the PR into a mergable state. |
|
Thanks, all green now! Now we just need a maintainer to merge this. |
|
I am wondering if there actually is a maintainer.... |
|
@ctoestreich there's a maintainer indeed, problem is he's not maintaining - that's me :( I owe an apologies to all the contributors (whether it's code or feedback), will work on the opened PRs next week and try to make a release. Thanks everyone for your patience! |
There is an atomic compare and set loop in
Math.randomthat can cause performance issues.You can find many postings on the performance aspects of this change
http://blog.anvard.org/articles/2014/01/12/concurrent-random.html
This change requires a minimum version change from Java 1.6 to 1.7