Is synchronization needed while reading if no contention could occur

Question

Consider code sniper below:

package sync;

public class LockQuestion {
    private String mutable;

    public synchronized void setMutable(String mutable) {
        this.mutable = mutable;
    }

    public String getMutable() {
        return mutable;
    }   
}

At time Time1 thread Thread1 will update ‘mutable’ variable. Synchronization is needed in setter in order to flush memory from local cache to main memory. At time Time2 ( Time2 > Time1, no thread contention) thread Thread2 will read value of mutable.

Question is – do I need to put synchronized before getter? Looks like this won’t cause any issues - memory should be up to date and Thread2’s local cache memory should be invalidated&updated by Thread1, but I’m not sure.

Fcc

Threading in Java is defined in terms of happens-before relationships. Don't try to think in terms of flushing caches, because you will be wrong. For one thing, compiler optimisations are important. Even if flushing caches was an accurate model, if the referred object is mutable, the order of updates wouldn't be guaranteed. (Up until the 1.5 spec, the spec did use a flushing-caches model, but was unimplementable.) – Appetency

Answer 1

Rather than wonder, why not just use the atomic references in java.util.concurrent?

(and for what it's worth, my reading of happens-before does not guarantee that Thread2 will see changes to mutable unless it also uses synchronized ... but I always get a headache from that part of the JLS, so use the atomic references)

Vita

While Atomic classes are highly useful, they are inevitably have tiny performance penalty. I prefer to use 'simple' types unless atomicity of update is required. In case above only visibility of update is required. – Fcc

@Petro Semeniuk Without one of the atomic objects, synchronization, and/or volatile, there is no way to guarantee "visibility" of update and the read in relation to eachother. This may or may not matter. – Ehling

Note that the synchronized/volatile needs to be on the same object. – Appetency

Petro, I wish I had your performance problems if you are truly concerned about the performance of memory barriers. – Housebreak

Answer 2

It will be fine if you make mutable volatile, details in the "cheap read-write lock"

Chadwick

Thanks. My thoughts about volatile are similar as about Atomic* classes - they are cheap but they are not free. Ideally I want to force 'happens-before'but without any penalty/lock on reads. – Fcc

@Petro Semeniuk It's not about being "free", it's about "always having the desired semantics" (e.g. not letting a strange synchronization bug enter). However, your program might function fine in such a way without an "expensive lock" (which, face it, is very damn cheap unless highly contended) -- but that's only for you to analyze in the larger picture. Better to just do "the right thing" and not worry about silly micro-optimizations. – Ehling

Although in practice volatile is rarely useful. – Appetency

@Tom: Why? I've been using volatile successfully for synchronization. Since I know its semantics exactly, what should be the problem? It's a great light-weight synchronization mechanism. And – by the way – would you consider the ConcurrentHashMap "rarely useful"? – Belenbelesprit

@Roland Illig What has the usefulness of ConcurrentHashMap got to do with the usefulness of volatile? – Appetency

The former uses the latter a lot to achieve proper synchronization. – Belenbelesprit

volatile simply introduces a happens-before relationship such that any thread reading the value will see the "latest" value. As there is nothing here apart from a write to a single reference, making the reference volatile removes the need for the lock which adds nothing. – Housebreak

@Petro Semeniuk - which platform do you run on? volatile reads on x86 are pretty cheap. Ultimately if performance matters this much then you need to start coding with the architecture you run on explicitly in mind. What is best for sparc might not be best for x86, e.g. infoq.com/articles/memory_barriers_jvm_concurrency – Chadwick

Answer 3

Are you absolutely sure that the getter will be called only after the setter is called? If so, you don't need the getter to be synchronized, since concurrent reads do not need to synchronized.

If there is a chance that get and set can be called concurrently then you definitely need to synchronize the two.

Heartache

This is not true. Without a synchronized lock in the getter, there is no happens-before guarantee. The setter could fire, finish, and then the getter fires in a different thread -- but reads the old value. This may or may not matter. For a single-access, volatile would "be equivalent" here, but misses out on the larger synchronization implications. – Ehling

OP wrote that the the setter is called at Time1, getter is called at Time2, and Time2 > Time1, so it is possible that thread 2 is only started after thread 1 completes which gives a "happens-before" guarantee (e.g. thread1.Start(), thread1.Join(), thread2.Start()) – Heartache

I don't see anything relating to threads being stopped or joined. T2 > T1 is not sufficient for happens-before -- caching throws this assumption out the window. – Ehling

OP wrote "no thread contention" so the threads must be synchronized somehow for this to be guaranteed, but I agree not enough information was given on how this was done which is why I gave both answers in my post. – Heartache

I didn't give enough information because I don't have it. @pst you are right about happens-before. T2 > T1 doesn't guarantee this, while accessing/releasing the same lock (either through volatile/synchronized) or using Atomic classes could guarantee that. – Fcc

@pst :) - if it was my code I would synchronize them, its safer, and if there is no contention there shouldn't be any major performance hit (in .net at least) – Heartache

Answer 4

If you worry so much about the performance in the reading thread, then what you do is read the value once using proper synchronization or volatile or atomic references. Then you assign the value to a plain old variable.

The assign to the plain variable is guaranteed to happen after the atomic read (because how else could it get the value?) and if the value will never be written to by another thread again you are all set.

Truckload

Answer 5

I think you should start with something which is correct and optimise later when you know you have an issue. I would just use AtomicReference unless a few nano-seconds is too long. ;)

public static void main(String... args) {
    AtomicReference<String> ars = new AtomicReference<String>();
    ars.set("hello");
    long start = System.nanoTime();
    int runs = 1000* 1000 * 1000;
    int length = test(ars, runs);
    long time = System.nanoTime() - start;
    System.out.printf("get() costs " + 1000*time / runs + " ps.");
}

private static int test(AtomicReference<String> ars, int runs) {
    int len = 0;
    for (int i = 0; i < runs; i++)
        len = ars.get().length();
    return len;
}

Prints

get() costs 1219 ps.

ps is a pico-second, with is 1 millionth of a micro-second.

Wisp

Thanks for providing perf metrics. Btw, if you'll do the same with plain String you'll see that constant access to AtomicReference doubles cost of retrieval. Probably the most efficient way to do it is to assign value to local variable(inside test method) – Fcc

The get is just a volatile read, declaring it volatile will have the same effect. – Cagliostro

Answer 6

This probably will never result in incorrect behavior, but unless you also guarantee the order that the threads startup in, you cannot necessarily guarantee that the compiler didn't reorder the read in Thread2 before the write in Thread1. More specifically, the entire Java runtime only has to guarantee that threads execute as if they were run in serial. So, as long as the thread has the same output running serially under optimizations, the entire language stack (compiler, hardware, language runtime) can do pretty much whatever it wants. Including allowing Thread2 to cache the the result of LockQuestion.getMutable().

In practice, I would be very surprised if that ever happened. If you want to guarantee that this doesn't happen, have LockQuestion.mutable be declared as final and get initialized in the constructor. Or use the following idiom:

private static class LazySomethingHolder {
  public static Something something = new Something();
}

public static Something getInstance() {
  return LazySomethingHolder.something;
}

Hyla