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

[Vladimir Ivanov]

Vladimir K,

At that moment, we targeted @Stable for C2 only. That's why there's no 
@Stable support in C1 & interpreter. Regarding interpreter, the only 
thing we discussed was verification mode for @Stable values. I filed a 
RFE for that [1].

Aleksey,

Can you also test with -XX:+FoldStableValues 
-XX:+UseImplicitStableValues since you changed relevant code and run HS 
regression tests (and CTW preferably).

Best regards,
Vladimir Ivanov

[1] https://bugs.openjdk.java.net/browse/JDK-8024042

On 1/31/14 8:33 PM, Vladimir Kozlov wrote:
> 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.
>>>>>>>>
>>>>>>
>>>>
>>