Brian,
Peter is absolutely right - I realized my mistake lying in bed last night.
If the sentinel values were the default zero values there would be no
issue but they are initialized to a different value. If the instances
are then shared without a safe publication mechanism then it is possible
that threads will see the zero default value and mistake that for an
actual field value. So the fields have to be volatile to be guaranteed
that their initialized value will be seen.
In practice, because there are also final fields in these instances
implementations will most likely perform a storestore barrier after
construction and prior to setting the reference to the created object.
That would make non-volatile appear to work, but it would not be
guaranteed to do so and the final-field "freeze" action only guarantees
that unsafe publication will see the correct value of the final fields
(and for final references anything reachable via that reference).
Peter:
> * For various implementation reasons, JVMs arrange that
> volatile fields are publication safe anyway, at least in
> cases we know about.
Note that this part of the discussion was shown to be false - it is
certainly false in hotspot (except for the new AIX/PPC64 port).
Thanks,
David
On 21/02/2014 3:55 AM, Peter Levart wrote:
On 02/20/2014 11:20 AM, David Holmes wrote:
Hi Brian,
On 20/02/2014 7:44 AM, Brian Burkhalter wrote:
This patch concerns cleaning up some ugly internal deprecations.
Issue: https://bugs.openjdk.java.net/browse/JDK-8035279
Webrev: http://cr.openjdk.java.net/~bpb/8035279/webrev.00/
All JTREG BigInteger tests pass, and the serialized form has been
unaltered as verified by bidirectional read-write testing between
Java 7 and this version.
I would appreciate scrutiny of the arithmetic to make sure that I've
made no silly errors. Also I would be interested in opinions as to
whether the "volatile" keyword should in fact be used for the
affected instance variables.
re: volatile
The basic approach being taken with these primitive fields is lazy
initialization to a value that will always be the same if recomputed.
If the variables are not volatile then in the worst-case (this is
theoretical only) they will be recomputed by each thread that accesses
them. If they are volatile then they will only be recomputed by
threads that truly race on the initialization logic, but you will
thereafter pay the price of a volatile read on each access.
Either way you will be functionally correct so it all comes down to
performance. Practically speaking I would expect you to be better off
overall without the volatile.
As Paul notes the powerCache array does not need to be volatile as it
is initialized as part of class initialization.
Cheers,
David
Hi Brian,
I would also be concerned about correctness here. You changed fields to
be volatile and assigned them with different that default (zero) values
in field initializers. Take for example the field bitCount:
| 156 private volatile int bitCount = Integer.MIN_VALUE;|
In code you check this initial value and decide whether to lazily
compute the real value or not based on this check:
3298 public int bitCount() {
3299 // This computation has a known, acceptable non-critical race
condition.
3300 if (bitCount == Integer.MIN_VALUE) { // bitCount not initialized
yet
There has recently been a discussion on concurrency-dev mailing list
about whether this is safe:
http://cs.oswego.edu/pipermail/concurrency-interest/2013-November/011951.html
...in cases where the instance of such constructed object escapes to
non-constructing thread via data-race. I think the conclusion was that
such semantics is desirable and might be guaranteed in a future updated
JMM, but currently it is not, although most JVMs are implemented in such
a way that this is safe. Citing Doug Lea:
"
Right. To summarize:
* Programmers do not expect that even though final fields are specifically
publication-safe, volatile fields are not always so.
* For various implementation reasons, JVMs arrange that
volatile fields are publication safe anyway, at least in
cases we know about.
* Actually updating the JMM/JLS to mandate this is not easy
(no small tweak that I know applies). But now is a good time
to be considering a full revision for JDK9.
* In the mean time, it would make sense to further test
and validate JVMs as meeting this likely future spec.
-Doug
"
Is there a special reason you changed the "uninitialized" value from
default (zero) to non-zero?
Regards, Peter
