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

Justin Sampson jsampson at guidewire.com
Thu Jul 6 12:01:36 EDT 2017


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
    



More information about the Concurrency-interest mailing list