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 am not really happy about bitwise ORing the two comparison results as it seems to reduce readability at no real gain. Thanks, Thomas