On Feb 25, 2014, at 1:32 AM, Brian Burkhalter <brian.burkhal...@oracle.com>
wrote:
>
> On Feb 20, 2014, at 6:39 PM, David Holmes wrote:
>
>> Not clear what you mean by this.
>
> This is more or less my reaction to this entire thread, so to speak. ;-)
> Anyway, thanks for all the comments.
>
:-)
> Note that I am ignoring the powerCache field comments for the moment.
>
> To distill the discussion down to just the proposed changes I posted, my
> reading is that there is more or less consensus on two points:
>
> 1) the instance fields in question *should* be volatile for this proposed
> change set
Yes. Sorry, i mislead you earlier, i got confused about what hotspot does (and
realistically most other platforms) compared to what is currently specified
(and hopefully rectified in the JMM update).
> 2) non-zero initial values should be avoided in case of instance leaking to
> non-constructing threads
>
If say, in your patch, the bitCount field was not volatile then there is a
possibility on some platforms that a non-constructing thread may observe
default values:
class X {
static BigInteger x;
static void threadOne() {
x = new BigInteger("11111111", 2);
// A compiler is allowed to move code such that the put to x can occur
before the put to bitCount
// If bitCount is volatile then a memory barrier or fence is placed around
the put of bitCount
// such that (in general) code above cannot be moved below and code below
cannot be moved above
// (in addition to other things going with the caches on the processors to
ensure visibility)
}
static void threadTwo() {
if (x != null) {
int bc = x.bitCount();
assert bc = 8; // may fail, could return 0
}
}
}
I tweaked the example from
http://www.cs.umd.edu/~pugh/java/memoryModel/jsr-133-faq.html (although 10
years old that article is one of the best around at explaining things).
If you leak the "this" reference to another thread during construction then all
bets are off w.r.t. the visibility of any final fields.
Note the use of putVolatile on the final fields when deserializing, this
ensures fences are in place so that the visibility of the final fields is
preserved [*].
> Is this accurate?
>
> On second thought it occurred to me that instead of any complicated or
> contentious changes, as it were, the ugliness I was trying to remove from the
> code could just as well be addressed by simply changing the names of the
> affected instance variables to indicate what their respective values really
> represent, e.g., "bitCount" becomes "bitCountPlusOne" and we remove the
> @Deprecated and @deprecated. Yeah this is still ugly ...
>
Might as well just remove the @Deprecated stuff. I think it is fine under the
circumstances to have offsets and getter methods that return the correct values.
Paul.
[*] Strictly speaking it should only be necessary to do a put*Volatile on the
last final field set, others can use put*. And, unfortunately this is one
use-case where developers need to use Unsafe. Even if we have fence method on
Objects the final field still needs to be set and reflection is currently
slower, although MethodHandles could be an alternative when using a trusted
Lookup instance.
