On 07/01/2013 03:51 PM, Thomas Schatzl wrote: > Hi all, > > On Mon, 2013-07-01 at 15:44 +0400, Aleksey Shipilev wrote: >> On 07/01/2013 03:37 PM, David Holmes wrote: >>> On 1/07/2013 8:14 PM, Aleksey Shipilev wrote: >>>> The same "thou shalt not do multiple volatile reads" applies to >>>> "(r.queue == NULL) || (r.queue == ENQUEUED)" now. >>> >>> Doesn't that just reduce to "r.queue != this" ? (The assert suggests >>> so :) ) >> >> Thomas' original patch had this in form of "r.queue != this". I argue it >> is more future-proof to distinguish the concrete cases. > > :) > > I also thought it was more understandable if the cases were explicitly > enumerated, in addition to the assert. > > I changed this in > http://cr.openjdk.java.net/~tschatzl/8014890/webrev.refactor to that > version now, using a temporary variable that stores r.queue before the > checks to avoid the double volatile reads. > > However for me either version is fine, just tell me what you favor.
I have no opinion on this. Fine with both versions. -Aleksey.
