6

This question is somewhat continuation and expansion of this one, as I think perfect question: How does assigning to a local variable help here?

This question based on Item 71 of Effective Java, where it is suggested to speed up performance by introducing local variable in purpose of volatile field access:

private volatile FieldType field;  
FieldType getField() {  
    FieldType result = field;  
    if (result == null) { // First check (no locking)  
        synchronized(this) {   
        result = field;  
        if (result == null) // Second check (with locking)  
            field = result = computeFieldValue();  
        }  
    }  
    return result;  
}

So, my question is more common:

should we always access to volatile fields through assigning their values to local variables? (in order to archive best performance).

I.e. some idiom:

  1. we have some volatile field, call it just volatileField;

  2. if we want to read its value in multi-thread method, we should:

    1. create local variable with same type: localVolatileVariable
    2. assign value of volatile field: localVolatileVariable = volatileField
    3. read value from this local copy, e.g.:

      if (localVolatileVariable != null) { ... }
      
Community
  • 1
  • 1
Andremoniy
  • 34,031
  • 20
  • 135
  • 241
  • How is that question different than the one you link to? It seems you want to have an answer to a situation broader than the "initialize lazy-loaded field" scenario of that particular piece of code shown there (and here). But then the answer cannot be more than "it depends", I am afraid. – Thilo Apr 19 '13 at 09:23
  • It is different, that I just want to understand: is this optimization applicable only for the particular case, described in `Item 71`, or it is common approach for `volatile` fields reading? – Andremoniy Apr 19 '13 at 09:25

3 Answers3

4

You must assign volatile variables to local fields if you plan on doing any sort of multi-step logic (assuming of course, that the field is mutable).

for instance:

volatile String _field;

public int getFieldLength() {
  String tmp = _field;
  if(tmp != null) {
    return tmp.length();
  }
  return 0;
}

if you did not use a local copy of _field, then the value could change between the "if" test and the "length()" method call, potentially resulting in an NPE.

this is besides the obvious benefit of a speed improvement by not doing more than one volatile read.

jtahlborn
  • 52,909
  • 5
  • 76
  • 118
1

There are two sides of a coin.

On the one hand assignment to a volatile works like a memory barrier and it's very unlikely that JIT will reorder assignment with computeFieldValue invocation.

On the other hand in theory this code breaks JMM. Because for some reasons some JVM is allowed to reorder computeFieldValue with assignment and you see partially initialized object. This is possible as long as variable read is not order with variable write.

field = result = computeFieldValue();  

does not happen before

if (result == null) { // First check (no locking) 

As long as java code supposed to be "write once run everywhere" DCL is a bad practice and should be avoided. This code is broken and is not a point of consideration.

If you have multiple reads of a volatile variable in a method, by assigning it to a local variable first you minimize such reads, which are more expensive. But I don't think, that you get a performance boost. This is likely to be a theoretical improvement. Such optimization should be left to JIT and is not a point of developer's considerations. I agree with this.

Community
  • 1
  • 1
Mikhail
  • 4,175
  • 15
  • 31
  • On the one hand assignment to a volatile works -> he is not writing to a volatile, he is reading one. – Eugene Apr 19 '13 at 12:29
  • "field = result = computeFieldValue()" here he writes. This is a variation of a classic DCL anti pattern. – Mikhail Apr 19 '13 at 13:11
  • fyi, the DCL pattern shown in the OP is _not_ broken (it is the "fixed" version). not sure from your post whether you are implying that it is correct or not. – jtahlborn Apr 19 '13 at 16:35
  • There are some things in Java, which are working, but you should avoid them. DCL is one of those. – Mikhail Apr 20 '13 at 11:06
  • @Mikhail http://docs.oracle.com/javase/specs/jls/se7/html/jls-17.html#jls-17.4.5 "If x and y are actions of the same thread and x comes before y in program order, then hb(x, y)." That means "if (result == null)" does happen before "field = result = computeFieldValue(); " – Vitaly Nov 05 '13 at 11:10
0

Instead of potentially doing TWO reads of a volatile variables, he does just one. Reading volatile is probably a bit slower the a usual variable. But even if so, we are talking about nano seconds here.

Eugene
  • 117,005
  • 15
  • 201
  • 306
  • Well, but Joshua Bloch is speaking about 25% better performance on his machine :) – Andremoniy Apr 19 '13 at 13:09
  • 1
    @Andremoniy that was some time ago he wrote that. And 25% from 1 nano seconds si still very little. Too little to me. – Eugene Apr 19 '13 at 13:11