JDK 8 Reviewers: The proposed patch is here:
http://cr.openjdk.java.net/~bpb/6470700/ JTREG tests pass and JMH benchmarking shows a very slight performance improvement versus the extant code. Thanks, Brian On Aug 21, 2013, at 7:23 PM, Mike Duigou wrote: > On Aug 21 2013, at 17:01 , David M. Lloyd wrote: > >> On 8/21/13 5:37 PM, Brian Burkhalter wrote: >>> With respect to this issue >>> >>> http://bugs.sun.com/view_bug.do?bug_id=6470700 >>> >>> the code of concern from java.lang.Math is >>> >>> 701 private static Random randomNumberGenerator; >>> 702 >>> 703 private static synchronized Random initRNG() { >>> 704 Random rnd = randomNumberGenerator; >>> 705 return (rnd == null) ? (randomNumberGenerator = new Random()) : >>> rnd; >>> 706 } >>> >>> 731 public static double random() { >>> 732 Random rnd = randomNumberGenerator; >>> 733 if (rnd == null) rnd = initRNG(); >>> 734 return rnd.nextDouble(); >>> 735 } >>> >>> where the class variable "randomNumberGenerator" is not used anywhere else >>> in the class. The complaint is that this is an instance of the broken >>> double-checked locking pattern. While at first glance this might appear to >>> be the case, it does not seem so to me. It looks more like an attempt to >>> avoid calling a synchronized method if "randomNumberGenerator" has already >>> been initialized. >>> >>> A more typical pattern would be >>> >>> private static Random randomNumberGenerator; >>> >>> private static synchronized Random getRNG() { >>> if (randomNumberGenerator == null) { >>> randomNumberGenerator = new Random(); >>> } >>> return randomNumberGenerator; >>> } >>> >>> public static double random() { >>> return getRNG().nextDouble(); >>> } >>> >>> Comments, please. >> >> I don't think you'd want to introduce the overhead of synchronization here. >> It may be better in this case to use this kind of lazy init pattern: >> >> static final class Holder { >> static final Random RNG = new Random(); >> } >> >> public static double random() { >> return Holder.RNG.nextDouble(); >> } >> >> -- >> - DML > > > David's suggestion seems very reasonable. > > > Mike
