[concurrency-interest] Fwd: Should I avoid compareAndSet with value-based classes?

Gil Tene gil at azul.com
Thu Jul 6 15:12:09 EDT 2017


The "it's not as horrifying as you might think" crowd (which I'm not sure I'm part of or not) would argue that the worries are overblown. Yes, one "instance" can be freely replaced by another at any point, without you knowing about it. But why would you care? Howe would you be able to tell? If you properly avoid using identity operations (which you were told not to), where is the problem? The objects are immutable, so proper equality testing will never get you the "wrong" logical behavior.

The answer to this is generally "But… concurrency!". Which is why this is an ok place to chat about it probably. With no identity, how do you achieve atomic updates? Michael's MutableClock example (scroll down) of wanting to (potentially concurrently) add duration amounts to some notion of a point in time, while avoiding the potential for "losing" an add racy situations, requires some sort of synchronization. Since Instant is not mutable, and we are necessarily trying to atomically replace one Instant value with another, we have a problem with the notion that these values have no identity (and therefore no way to do a proper CAS on). We are also specifically admonished to not do any synchronization on "instances" of such classes, which should serious dissuade one from trying. Boxing is *an* answer (put the value in an instance that has a proper identity, and then CAS the identity), but a seemingly unsatisfying one...

— Gil.

> On Jul 6, 2017, at 11:47 AM, Justin Sampson <jsampson at guidewire.com> wrote:
> 
> That is absolutely horrifying, and leads me to believe we should not use such "value-based" classes at all in our code. Off I go to find a list of them and forbid them in my codebase. Somehow I thought that I was relatively up-to-date on my Java knowledge and this is the very first time I'm hearing about them. Surely the vast majority of Java programmers are similarly unaware, and the idea that an object instance can actually be replaced by another object instance at runtime is beyond surprising.
> 
> -Justin
> 
> 
> On 7/6/17, 10:49 AM, "Gil Tene" <gil at azul.com> wrote:
> 
>    We are not discussing the upcoming value types. We are discussing the value-based classes as currently spec'ed in Java SE 8. Instant is a good example of such a class.
> 
>    To give you a feel for a currently valid (and may soon occur) optimization in Java 8: The current spec'ed behavior for Instant makes it possible to "inline" instances of Instant into their containing objects (which refer to them with mutable fields):
>    - There are no valid identity operations on Instant, so assignment can be validly done by copying contents onto the inlined version. A "get" of an inlined instance (where it can't be escape analyzed away) can be performed by returning a new (and different) instance of Instant.
>    - Word-tearing on value assignment [e.g. myClock.setInstsnt(Instant.now()))] can be avoided at least on some platforms, e.g. with 16-byte atomic operations, where the state of Instant instances can be represented in 16 bytes or less.
> 
>    This can then result in an elimination of a dereferencing operation when e.g. accessing someClock.getInstant().getNano(). (and no new instance will be created).
> 
>    Now the specific generics based containment below [AtomicReference<Instant>] may seem a bit more "challenging" (and likely will happen much later than in the non generic cases), but given the fact that the AtomicReference instance use is local to the MutableClock class and private, and no instances of that AtomicReference are exposed elsewhere, it would be valid to transform this code to use some custom subclass of AtomicReference that had  an Instant instance inlined in it. At that point, the updateAndGet operation comes into play. It may have "interesting" behavior because it is identity based, or (more likely) its existence could just prevent the whole optimization from occurring.
> 
>    In any case, to keep to well specified behavior, I'd box the Instant when putting it in an AtomicReference. That will provide you with a safe concurrent add() that doesn't use identity on Instant. You can probably do some simple caching on the get for speed (hold a cachedInstant field and a valid flag, re-read from boxed contents whenever invalid) with an invalidation (set invalid after each modification of the boxed contents) in set() and add().
> 
>    Oh, and BTW, you should probably check for null and enforce non-null for all incoming Instance arguments, e.g. in the constructor and in setInstance(). 
> 
>    Sent from my iPad
> 
>> On Jul 6, 2017, at 9:01 AM, Justin Sampson <jsampson at guidewire.com> wrote:
>> 
>> Howdy,
>> 
>> This thread got sidetracked with discussion of the upcoming "value types" in some later version of the JDK, which don't exist yet in 8, and really aren't relevant at all to Michael's question. He's asking about the existing Instant class, which is simply an immutable object with an equals() that compares its contents.
>> 
>> It's absolutely silly to suggest that AtomicReference "doesn't work" for things like Instant. It simply uses identity instead of equals() in its operations, which is entirely well-defined.
>> 
>> Michael's example MutableClock will work just fine. It uses updateAndGet(), which CAS's the result of the update. The point of the CAS is merely to check whether another thread has updated the reference concurrently, for which identity works correctly. The update is retried if the CAS fails, so there's no problem introduced by the fact that equals() for the value is different from equality.
>> 
>> Cheers,
>> Justin
>> 
>> 
>> On 7/6/17, 7:31 AM, "Concurrency-interest on behalf of Michael Hixson" <concurrency-interest-bounces at cs.oswego.edu on behalf of michael.hixson at gmail.com> wrote:
>> 
>>   Forwarding this reply of mine because I forgot to CC the mailing list.
>> 
>> 
>>   ---------- Forwarded message ----------
>>   From: Michael Hixson <michael.hixson at gmail.com>
>>   Date: Thu, Jul 6, 2017 at 7:24 AM
>>   Subject: Re: [concurrency-interest] Should I avoid compareAndSet with
>>   value-based classes?
>>   To: Andrew Haley <aph at redhat.com>
>> 
>> 
>>>>>   On Thu, Jul 6, 2017 at 4:20 AM, Andrew Haley <aph at redhat.com> wrote:
>>>>>> On 06/07/17 11:41, Alex Otenko wrote:
>>>>> 
>>>>> On 6 Jul 2017, at 10:13, Andrew Haley <aph at redhat.com> wrote:
>>>>> 
>>>>> On 06/07/17 04:59, Michael Hixson wrote:
>>>>>> AtomicReference and VarHandle are specified to use == in compareAndSet
>>>>>> (and related) operations [1].  Using == to compare instances of
>>>>>> value-based classes may lead to "unpredictable results" [2].  Does
>>>>>> this mean I should avoid using compareAndSet with arguments that are
>>>>>> instances of value-based classes?
>>>>>> 
>>>>>> It seems like the documentation clearly tells me "yes, avoid doing
>>>>>> that" but I'm hoping I misunderstood, or maybe AtomicReference and
>>>>>> VarHandle are exempt somehow.  Otherwise, how do I implement
>>>>>> non-broken compareAndSet and updateAndGet for a java.time.Instant
>>>>>> value for example?
>>>>> 
>>>>> java.time.Instant stores times that are longer than a JVM word, so
>>>>> they cannot be CAS'd in a lock-free way unless a factory guarantees
>>>>> that instances which compare equal also have the property of
>>>>> reference equality.  j.t.Instant factories in the Java library are
>>>>> explicitly documented *not* to have this property, so that doesn't
>>>>> help.
>>>> 
>>>> That’s not entirely clear.
>>>> 
>>>> Wouldn’t this loop work:
>>>> 
>>>> volatile j.t.Instant curr = ...
>>>> 
>>>> j.t.Instant next = …
>>>> j.t.Instant tmp = ...
>>>> do {
>>>> tmp = curr;
>>>> if (tmp.equal(next)) break;
>>>> } while(!curr.CAS(tmp, next));
>>>> 
>>>> // assume curr is equal to next
>>> 
>>> Something like that, yes.  But it's going to be evil if there are
>>> several high-frequency writers.  If you're doing all that work in
>>> order to CAS a timestamp, why not use a synchronized block?  It would
>>> at least be less prone to the thundering herd, and we'd generate
>>> pretty decent code for that.  It'd be interesting to compare and
>>> contrast the two approaches for contended and non-contended cases.
>> 
>>   The main reason I reached for AtomicReference is that I thought, "I
>>   want thread-safe updates to this read-heavy value where writes won't
>>   get lost if there's contention -- this sounds like the sort of problem
>>   that java.util.concurrent.atomic solves."
>> 
>>   As a minor point, I wanted synchronization on reads to be as
>>   minor/invisible as possible, to affect the readers' behavior as little
>>   as possible (in comparison to their behavior when the value they're
>>   reading a constant value with no synchronization).
>> 
>>   But if AtomicReference is simply the wrong tool to use here, I
>>   shouldn't use it.  That's fine.
>> 
>>> 
>>>>> If you want to be able to CAS a reference to a j.t.Instant, you're
>>>>> going to have to wrap accesses to it in a synchronized block.  This is
>>>>> a direct consequence of the JVM's inability to CAS multi-word objects.
>>>> 
>>>> This is confusing.
>>>> 
>>>> Surely this isn’t talking about CASing a reference? The contents of
>>>> the object can’t be assumed to have any atomicity properties,
>>>> whether it is j.t.Instant or not.
>>> 
>>> I agree.  I'm trying to look at what the OP actually wants to do: I
>>> assume this is some kind of atomic timestamp, and the OP wants to be
>>> able to CAS an instance of j.u.Instant.
>> 
>>   It was more of a general worry and question.  Should I ever use
>>   AtomicReference with value-based classes?  It sounds like the answer
>>   is no.
>> 
>>   But here's some code that illustrates the specific problem I was
>>   trying to solve.  I think it "works" right now, but that it's in
>>   violation of the spec for value-based classes and so possibly not
>>   future-proof, and I think I understand how to fix it now.  Thanks for
>>   the tips.
>> 
>>       class MutableClock {
>> 
>>           static MutableClock create(Instant instant, ZoneId zone) {
>>               return new MutableClock(
>>                       new AtomicReference<>(instant),
>>                       zone);
>>           }
>> 
>>           private final AtomicReference<Instant> instantHolder;
>>           private final ZoneId zone;
>> 
>>           private MutableClock(
>>                   AtomicReference<Instant> instantHolder,
>>                   ZoneId zone) {
>>               this.instantHolder = instantHolder;
>>               this.zone = zone;
>>           }
>> 
>>           Instant instant() {
>>               return instantHolder.get();
>>           }
>> 
>>           ZoneId getZone() {
>>               return zone;
>>           }
>> 
>>           void setInstant(Instant newInstant) {
>>               instantHolder.set(newInstant);
>>           }
>> 
>>           void add(Duration amountToAdd) {
>>               // this is the part that uses == and CAS
>>               instantHolder.updateAndGet(
>>                     instant -> instant.plus(amountToAdd));
>>           }
>> 
>>           MutableClock withZone(ZoneId newZone) {
>>               // conveniently, AtomicReference also acts as a
>>               // vehicle for "shared updates" between instances
>>               // of my class
>>               return new MutableClock(instantHolder, newZone);
>>           }
>>       }
>> 
>>   -Michael
>>   _______________________________________________
>>   Concurrency-interest mailing list
>>   Concurrency-interest at cs.oswego.edu
>>   http://cs.oswego.edu/mailman/listinfo/concurrency-interest
>> 
>> 
>> _______________________________________________
>> Concurrency-interest mailing list
>> Concurrency-interest at cs.oswego.edu
>> http://cs.oswego.edu/mailman/listinfo/concurrency-interest
> 
> 



More information about the Concurrency-interest mailing list