with colleagues, we were discussing the following idiom found in the code of PriorityBlockingQueue (Oracle JDK 8, v1.8.0_131). At almost every use of the field private final ReentrantLock lock, the developer has chosen to first store it in a local variable (defined as final too). The pattern looks like this:
...
private final ReentrantLock lock; // initialized as 'new ReentrantLock()' in constructors
...
public E poll() {
final ReentrantLock lock = this.lock; // <--
lock.lock();
This choice is difficult to explain, and none of us can see an advantage of this instead of using directly the field. Since the original field is final, this is not needed to prevent or reduce the risk of dirty reads in a multithreaded context. Since the field is not volatile, this is not an optimisation to access a volatile field only once. This could be a very tiny-micro-optimisation where a field reference lives in the heap and a local variable in the stack, and reading from the stack is (or could be?) a bit more efficient. But we are talking about at most picoseconds here. And this idiom is not used for others fields access - only the lock is managed this way...
So we are stuck at "this is useless and could (should) be rewritten as direct field access". But I'm a bit unconfortable with our conclusion. I know for sure that most of the code in the JDK is quite smart, and passed with success a few code reviews with top level engineering people. Are we missing some tricky implementation details here?
