RFR (S) 8031818: Experimental VM flag for enforcing safe object construction

Vladimir Kozlov vladimir.kozlov at oracle.com
Fri Jan 31 08:33:11 PST 2014


Thanks, Aleksey

Looks good to me.

One thing I don't see is Stable fields processing in C1. I also looked on original @Stable changes and don't see change 
in Interpreter too. I don't know why I did not asked about it when reviewed those changes. Please, talk to Vladimir 
Ivanov and file a bug if it is needed and missed. Your current fix is fine, no need additional changes for Stable.

Thanks,
Vladimir

On 1/31/14 5:35 AM, Aleksey Shipilev wrote:
> Ok, here you go:
>   http://cr.openjdk.java.net/~shade/8031818/webrev.02/
>
> Testing:
>   - full JPRT cycle
>   - microbenchmarks with -XX:(+|-)AlwaysSafeConstructors
>
> Thanks,
> -Aleksey
>
> On 01/31/2014 03:19 AM, Vladimir Kozlov wrote:
>> On 1/30/14 2:54 PM, Aleksey Shipilev wrote:
>>> Yes we can.
>>> Do you want me to do this in this CR?
>>
>> Yes, please.
>>
>> Thanks,
>> Vladimir
>>
>>>
>>> -Aleksey.
>>>
>>> On 01/31/2014 02:47 AM, Vladimir Kozlov wrote:
>>>> Can we simple separate stable stores to wrote_stable() with own code in
>>>> do_exit() and comments?
>>>>
>>>> Thanks,
>>>> Vladimir K
>>>>
>>>> On 1/30/14 2:14 PM, Aleksey Shipilev wrote:
>>>>> On 01/31/2014 01:53 AM, Vladimir Kozlov wrote:
>>>>>> hs-comp is closed for almost 2 weeks until SQE done all testing. Only
>>>>>> ppc merge related fixes are allowed.
>>>>>
>>>>> Understood, no pressure.
>>>>>
>>>>>> You check is_initializer() for all types of fields in do_exits(). But
>>>>>> is_final() is also set for @Stable field which could any methods based
>>>>>> on the comment in do_put_xxx().
>>>>>
>>>>> Well, that's disturbing.
>>>>>
>>>>> In pure spec-induced sense, the block emitting the barrier in
>>>>> do_exits()
>>>>> is only valid for initializers. That means, @Stable can indeed mimic
>>>>> its
>>>>> constructor store as the final field store. However, piggybacking on
>>>>> the
>>>>> same code to produce the barrier for an arbitrary method seems very
>>>>> error-prone (and even contradicting the comment in do_exits() which
>>>>> assumes [wrote_final == true] => [method.is_initializer() == true]).
>>>>>
>>>>> If @Stable writes out the value in an arbitrary method, then it
>>>>> requires
>>>>> release barrier on it's own. It is a pure luck finals _also_ have
>>>>> MemBar_Release, while they could have more relaxed form... especially
>>>>> when/if JMM 9 effort would allow this relaxation. Conflating memory
>>>>> semantics for finals and @Stable seems wrong.
>>>>>
>>>>> Hence, I think the change is sound. It keeps @Stable semantics for
>>>>> constructor stores. @Stable needs more work to emit release barriers
>>>>> for
>>>>> the arbitrary methods if required (why?), and I think given the failure
>>>>> scenario affects only non-TSO platforms, it can be done separately.
>>>>>
>>>>> Thoughts? (Vladimir Ivanov, please report in!)
>>>>>
>>>>> -Aleksey.
>>>>>
>>>>>> Thanks,
>>>>>> Vladimir
>>>>>>
>>>>>> On 1/30/14 12:15 PM, Aleksey Shipilev wrote:
>>>>>>> On 01/22/2014 11:13 PM, Aleksey Shipilev wrote:
>>>>>>>> On 01/22/2014 11:07 PM, Vladimir Kozlov wrote:
>>>>>>>>> Question first: can you wait about 2 weeks when we merge ppc64
>>>>>>>>> changes?
>>>>>>>>
>>>>>>>> I think we can wait. I don't want to collide with PPC merge
>>>>>>>> either. In
>>>>>>>> fact, it would be even more convenient for us to grab PPC C2 in the
>>>>>>>> experiments from the mainline.
>>>>>>>
>>>>>>> Are we open already? I see PPC changes had collided with my patch.
>>>>>>> Even
>>>>>>> if it's too early to push the patch, I merged the upstream changes
>>>>>>> into
>>>>>>> the updated webrev:
>>>>>>>       http://cr.openjdk.java.net/~shade/8031818/webrev.01/
>>>>>>>
>>>>>>> Instead of introducing a new method, I rewired the predicate and
>>>>>>> rearranged the comments to make its structure clear. I think
>>>>>>> extracting
>>>>>>> method is not required now. Also, a few typos corrected.
>>>>>>>
>>>>>>> Testing:
>>>>>>>      - full cycle JPRT
>>>>>>>
>>>>>>> -Aleksey.
>>>>>>>
>>>>>
>>>
>


More information about the hotspot-compiler-dev mailing list