Java6 fix

[felixbarny]

Sorry guys, I screwed that one up :(

I'm not sure why but the JVM seems to load all classes which are used in a (static) method. That makes it not safe to just check the threadLocalRandomPresent flag and access ThreadLocalRandom right away. Maybe it has to do with some JIT magic? BTW making the flag final did not help either.

I noticed that in the integration tests for stagemonitor: https://travis-ci.org/stagemonitor/stagemonitor-integration-tests/jobs/222501692#L700. Tested the fix locally with Java 6.

[codefromthecrypt]

agreed. this is the way to do this :)

[codecov-io]

Codecov Report

Merging #138 into master will increase coverage by 0.09%.
The diff coverage is 66.66%.

@@             Coverage Diff              @@
##             master     #138      +/-   ##
============================================
+ Coverage     36.17%   36.27%   +0.09%     
  Complexity      598      598              
============================================
  Files            94       94              
  Lines          6341     6343       +2     
  Branches       1052     1052              
============================================
+ Hits           2294     2301       +7     
+ Misses         3865     3860       -5     
  Partials        182      182

Impacted Files	Coverage Δ	Complexity Δ
...jaeger/utils/Java6CompatibleThreadLocalRandom.java	71.42% <66.66%> (-3.58%)	3 <0> (ø)
...java/com/uber/jaeger/reporters/RemoteReporter.java	84.12% <0%> (+3.17%)	5% <0%> (ø)	⬇️
...uber/jaeger/reporters/protocols/TUDPTransport.java	86.44% <0%> (+3.38%)	16% <0%> (ø)	⬇️
...c/main/java/com/uber/jaeger/senders/UDPSender.java	71.42% <0%> (+4.08%)	10% <0%> (ø)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e340486...8f871ec. Read the comment docs.

[felixbarny]

Hey @raphw, could you explain us the magic?

[raphw]

The JVM is really free to load a class referenced within an executed method at any time it wants to. The concept of optional types is not know to the runtime. Reflection would probably be the best way to guard against eager loading but there is a runtime impact. As for your solution, it should work with HotSpot but it would generally be safer to move code to a dispatcher class which is discarded in case Java 7 is not available.

[felixbarny]

Thanks a lot for you quick answer!

it would generally be safer to move code to a dispatcher class which is discarded in case Java 7 is not available.

How would that look like? Do you have an example for this approach which is used in some OS project?

[codefromthecrypt]

it would generally be safer to move code to a dispatcher class which is discarded in case Java 7 is not available. How would that look like? Do you have an example for this approach which is used in some OS project?

I've used a Platform class (idea stolen from okhttp), which has a Java6 variant once via reflection you know which to keep.

[raphw]

Have a look at these dispatchers which do the same job in Byte Buddy: https://github.com/raphw/byte-buddy/blob/master/byte-buddy-dep/src/main/java/net/bytebuddy/dynamic/loading/ByteArrayClassLoader.java

[felixbarny]

@yurishkuro I think this should be good enough. Or should I reiterate?

[felixbarny]

Nice! Are you planning to do a bugfix release? Otherwise, when is 0.19.0 scheduled?