5

When I see some framework source code like dubbo, I often see code similar to the following:

public class Person {
   int age;
   String name;
   List<Person> persons = new ArrayList<Person>();

   public Person findPerson(int lowAge,int highAge){
         List<Person> localPersons = persons;
         for(Person p : localPersons){
              if( p.age >=lowAge && p.age <highAge){
                   return p;
              } 
         }
         return null;
   }

}

I just do not understand why not using the member variables persons directly. Local variable localPersons looks redundant to me.

Mark Rotteveel
  • 100,966
  • 191
  • 140
  • 197
aworker
  • 341
  • 1
  • 2
  • 6
  • 2
    It *is* redundant unless they actually make it `final` - in this case, it doesn't really add anything of value though – EpicPandaForce Mar 26 '19 at 10:15
  • 9
    It's a performance microoptimization. See https://stackoverflow.com/questions/21613098/java-local-vs-instance-variable-access-speed – Alex Shesterov Mar 26 '19 at 10:15
  • 7
    ^ with emphasis on micro. The gains are so abysmal you should just not do this. – Tim Mar 26 '19 at 10:17
  • 1
    What about multithreading safety? Thread 1 accesses `persons` and does some modifications to the object, while Thread 2 does `persons = new LinkedList<>()`, the result is undefined. But with this construct the `findPerson()` method will always modify the "old" `persons` object, because it "copied" the reference – Lino Mar 26 '19 at 10:17
  • Possible duplicate of [Java local vs instance variable access speed](https://stackoverflow.com/questions/21613098/java-local-vs-instance-variable-access-speed) – ave4496 Mar 26 '19 at 10:24
  • @Lino I seriously doubt that. Only the reference to the list is copied, not the list itself. That means that when `persons` is modified, `localPersons` is still modified as well. It would be another thing if one wrote `localPersons = new ArrayList<>(persons)`. – MC Emperor Mar 26 '19 at 10:56
  • @MCEmperor Not entirely what I meant (It's hard for me to explain it). I meant to say that *if* Thread 2 overwrites the old reference of `Person.persons` with e.g. a `new ArrayList<>()` and Thread 1 has copied the reference of `Person.persons` earlier into `localPersons`. Then Thread 1 will not notice, that `persons` has changed, because it is working with a different reference, it's changes would be lost though, as `localPersons != persons` – Lino Mar 26 '19 at 11:02
  • 1
    @Lino Ah, I see what you mean. That indeed *could* be a reason to declare a local variable. – MC Emperor Mar 26 '19 at 12:28

1 Answers1

5

It is possibly a micro-optimization, though I doubt that it would make any difference with a modern JVM. (If that was a worthwhile optimization, the JIT compiler's optimizer will probably do the equivalent transformation at the native code level.)

It is also possibly an (incorrect) attempt to make the code work if there are multiple threads. I say "incorrect" because the programmer is ignoring the requirements of the Java Memory Model. Since there are no clear happens before relationships between two threads accessing the persons variable or the list that it refers to, threads are liable to see stale data, leading to unpredictable behavior.

There could be other reasons for doing this that are not apparent in your example. (For example, if persons was declared as volatile and certain other preconditions were met, this could be thread-safe, and the supposedly redundant local variable could have a valid purpose.)

Stephen C
  • 698,415
  • 94
  • 811
  • 1,216