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

[Vladimir Kozlov]

What about C1 in Tiered?

Thanks,
Vladimir

On 1/31/14 9:03 AM, Vladimir Ivanov wrote:
> 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.
>>>>>>>>>
>>>>>>>
>>>>>
>>>