Re: Review CR #6611830: UUID thread-safety and performance improvements

2011-02-24 Thread Peter Jones
This change seems good all around. In addition to fixing the noted race, it removes a substantial amount of redundant instance footprint from a class whose instances are not infrequently collected in significant numbers-- fields most of which just served methods that are essentially never used in

Re: Review CR #6611830: UUID thread-safety and performance improvements

2011-02-23 Thread Brian Goetz
Ignore my comment -- I was reading the diffs backwards :( On 2/22/2011 11:53 PM, Brian Goetz wrote: I think you have a potential visibility problem here. You use -1 as the initial value, but observing threads might see instead the default value if the initializing write is not visible, and mista

Re: Review CR #6611830: UUID thread-safety and performance improvements

2011-02-23 Thread Alan Bateman
Mike Duigou wrote: Daniel Aioanei reported via Josh Bloch a data race issue with the UUID accessor and hashCode() methods. I've prepared a webrev with the suggested changes: http://cr.openjdk.java.net/~mduigou/6611830/webrev.0/webrev/ I've tested the change against the standard UUID tests and

Re: Review CR #6611830: UUID thread-safety and performance improvements

2011-02-23 Thread Vitaly Davidovich
resending as I just realized I replied only to Brian. On Wed, Feb 23, 2011 at 12:19 AM, Vitaly Davidovich wrote: > Hi David/Brian, > > Yes, I meant whether the "entire" string.hashcode technique can be used, > including the default zero values. I agree on the initial -1 assignment > possibly not

Re: Review CR #6611830: UUID thread-safety and performance improvements

2011-02-22 Thread Brian Goetz
I think you have a potential visibility problem here. You use -1 as the initial value, but observing threads might see instead the default value if the initializing write is not visible, and mistakenly think that the zero default value represents a computed value. (This is different from the

Re: Review CR #6611830: UUID thread-safety and performance improvements

2011-02-22 Thread David Holmes
Hi Vitaly, Vitaly Davidovich said the following on 02/23/11 11:46: Unless i missed something, seems like the race can be fixed via same mechanics as String.hashcode; ie since all of the cached values are based on least and most significant bits which are final and the long members are volatile,

Re: Review CR #6611830: UUID thread-safety and performance improvements

2011-02-22 Thread Vitaly Davidovich
Hi Mike, Unless i missed something, seems like the race can be fixed via same mechanics as String.hashcode; ie since all of the cached values are based on least and most significant bits which are final and the long members are volatile, using lazy evaluation with use of temps should yield the sam

Review CR #6611830: UUID thread-safety and performance improvements

2011-02-22 Thread Mike Duigou
Daniel Aioanei reported via Josh Bloch a data race issue with the UUID accessor and hashCode() methods. I've prepared a webrev with the suggested changes: http://cr.openjdk.java.net/~mduigou/6611830/webrev.0/webrev/ I've tested the change against the standard UUID tests and did a microbenchmark