Hi Clemens,
On 08/20/11 02:12, Clemens Eisserer wrote:
The original code is rather strange. The static field will be
default initialized to zero, the AppContext constructor will
increment it to 1, then the static block explicitly sets it to one.
It would be cleaner/clearer if the field were declared and
initialized before the static block.
Agreed. Please find the modified patch at:
http://cr.openjdk.java.net/~ceisserer/7080700/webrev.04/
The fix looks fine to me.
--
best regards,
Anthony
That's an uninteresting case from a data-race perspective though
because the counter is only ever written once.
In the constructor the non-atomic increment can cause a missed update
and therefor cause numAppContexts to become 1 or even 0, which could
trigger that logic unintentionally.
However I now wonder about how often this gets called in a Swing
app? If this is a frequent call then the overhead of the atomic
might be significant. Perhaps the Swing folk should be evaluating
this change as well?
Did some benchmarks, and at least on x86 AtomicInteger.get() performs
identical to a volatile field read.
Thanks, Clemens
