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.

Reply via email to