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

[Vladimir Kozlov]

Thanks!

Vladimir

On 1/31/14 9:59 AM, Vladimir Ivanov wrote:
> There's no problem if @Stable doesn't work in C1. It means C1 doesn't
> use this bit of information and produces less optimal code - always
> loads the value.
>
> Best regards,
> Vladimir Ivanov
>
> On 1/31/14 9:51 PM, Vladimir Kozlov wrote:
>> 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.
>>>>>>>>>>>
>>>>>>>>>
>>>>>>>
>>>>>