On 20/08/2011 8:12 AM, 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/
Looks good. Just need someone from AWT to chime in.
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.
My point was that in a Swing app the constructor is only ever called once
and so there can not be a missed update. Anyway I was just musing :)
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.
And of course I knew that because the intrinsics that underlie the atomics
are the same as those for volatiles. :) Thanks for verifying.
Cheers,
David
Thanks, Clemens
