Mutant World

Friday, July 15, 2005

A weird particular in java.util.concurrent

I am using J2SE 5, and so far I am quite happy. I got used to the generics syntax, and in general I can write less code to do the same things.

While browsing java.util.concurrent source code, I found this weird pattern:

public class LinkedBlockingQueue extends ...
{
...
private final ReentrantLock takeLock = new ReentrantLock();
...

private void signalNotEmpty()
{
final ReentrantLock takeLock = this.takeLock;
takeLock.lock();
try
{
notEmpty.signal();
}
finally
{
takeLock.unlock();
}
}
...
}

The more I looked at the code, the more it seemed to me unnecessary to declare that final local variable in the signalNotEmpty() method.
The data member is already final, so it's not a question of visibility in multithread environments.

I decided to investigate more, and went back to Bill Pugh's site, which contains a lot of information about the java memory model and its revision through JSR 133, but I had no luck.

Finally I discovered that JSR 166 implementation is also under public domain, here.

The CVS log that added the weird pattern above is marked in this cryptic comment:

"cache finals across volatiles"

Now, if you have some information about, please drop a comment.

This reminds me what JSR 133 experts were saying about the memory model: that not even the JLS writers understood it.
Now I am certain that few more people understand it (e.g. Doug Lea, Bill Pugh), but - alas - I guess they can be counted with just one hand.

I'll let you informed if I found a solution to this arcane.

UPDATE: Doug Lea had the courtesy to answer to my request for clarification. His answer:

"This was a hopefully temporary performance hack due to a limitation in JVMs that don't realize that final fields are an exception to the rule that you must re-read fields after reading a volatile.
These JVMs, including 1.5.0 hotspot are correct wrt the memory model but inefficient unless helped out in this ugly/weird way. Someday these can disappear. It's
only worth bothering (and even then only sometimes, and even then only marginally so)
for performance-critical code like that inside j.u.c.
It's not a recommended practice."

It happens that notEmpty.signal() reads volatile fields and the JVM implementation inefficiently re-reads takeLock to execute takeLock.unlock() in the finally block.