Is extracting to static final necessary for Java optimization?

Question

Consider this method:

private void iterate(List<Worker> workers) {
    SortedSet<Worker> set = new TreeSet<>(new Comparator<Worker>() {
        @Override
        public int compare(Worker w0, Worker w1) {
            return Double.compare(w0.average, w1.average);
        }
     });

     // ... 
}

As you can see the set is creating a new TreeSet with a custom comparator.

I was wondering if it makes any difference from a performance/memory/garbage collection/whatever point of view, if I were to do this and instead having polluted the outer space:

static final Comparator<Worker> COMPARATOR = new Comparator<Worker>() {
    @Override
    public int compare(Worker w0, Worker w1) {
        return Double.compare(w0.average, w1.average);
    }
};

private void iterate(List<Worker> workers) {
    SortedSet<Worker> set = new TreeSet<>(COMPARATOR);
    // ... 
}

The reason I am asking, is that I feel the compiler should already figure this out and optimize it for me, so I shouldn't have to extract it, right?

The same thing goes for Strings or other temporary, immutable objects declared within a method.

Would it make any difference to extract it a final variable?

Note: I am aware of the little impact of performance boost this might give. The question is whether there is any difference, howsoever negligible.

Answer 1

There will be a difference yes.

• CPU affect: assigning to a static reduces the amount of work necessary to allocate a new comparator every time
• GC effect: assigning a new object every time and then immediately discarding it will have no young GC cost; however assigning it to a variable will increase GC times (very very marginally) as it is an extra set of references that will need to be walked. Dead objects cost nothing, live objects do.
• Memory effect: assigning the comparator to a constant will reduce how much memory each invocation of the method will require, in exchange for a low constant overhead that will be moved into tenured GC space.
• Risk of reference escapes: Inner classes contain pointers to the class that constructed it. If the inner class (Comparator) was ever returned out of the method that created it, then a strong reference to the parent object could escape and prevent GC of the parent. Purely a gotcha that can creep into code, it is not a problem in this example.

Hotspot is very good at inlining but it is unlikely to recognise that the comparator can be allocated on heap or moved to a constant. But that will depend on the contents of TreeSet. If the implementation of TreeSet was very simple (and small) then it could get inlined, however we all know that it is not. Also TreeSet is coded to be generic, if it was only ever used with one type of object (Worker) then there are some optimisations that the JVM can apply however we should assume that TreeSet will get used by other types too and so TreeSet will not be able to make any assumptions about the Comparator that is being past into it.

Thus the difference between the two versions is primarily an object allocation. The use of the final keyword is unlikely to improve performance as Hotspot mostly ignores the final keyword anyway.

Java 8 has a very interesting behaviour here when using lambdas. Consider the following variant of your example:

import java.util.*;

public class T {
    public void iterate(List<String> workers) {
        SortedSet<Double> set = new TreeSet<>( Double::compare );
    }
}

Run 'javap -c T.class', and you will see the following jvm code:

  public void iterate(java.util.List<java.lang.String>);
    Code:
       0: new           #2                  // class java/util/TreeSet
       3: dup
       4: invokedynamic #3,  0              // InvokeDynamic #0:compare:()Ljava/util/Comparator;
       9: invokespecial #4                  // Method java/util/TreeSet."<init>":(Ljava/util/Comparator;)V
      12: astore_2
      13: return

The cool thing to note here is that there is no object construction for the lambda. invokedynamic will have a higher cost the first time that it is called and then it gets effectively cached.

Answer 2

One big difference is caused by the "hidden" implication that anonymous classes hold an implicit reference to the containing class, so if you pass that TreeSet to another process, a reference to your class instance is held via the TreeSet via the anonymous Comparator by another piece of code, so your instance ca't be garbage collected.

That can cause a memory leak.

Option 2 however doesn't suffer from that problem.

Otherwise, it's a matter of style.

---

In java 8, you can use a lambda expression instead, which is the best of both worlds.