RFR: 8165808: Add release barriers when allocating objects with concurrent collection
David Holmes
david.holmes at oracle.com
Thu Sep 15 09:49:01 UTC 2016
You can count me a reviewer #2.
Thanks,
David
On 15/09/2016 9:45 AM, Kim Barrett wrote:
>> On Sep 12, 2016, at 7:22 PM, David Holmes <david.holmes at oracle.com> wrote:
>> On 13/09/2016 4:08 AM, Kim Barrett wrote:
>>>> On Sep 11, 2016, at 9:38 PM, David Holmes <david.holmes at oracle.com> wrote:
>>>>> Webrev:
>>>>> http://cr.openjdk.java.net/~kbarrett/8165808/webrev.00/
>>>>
>>>> src/share/vm/gc/shared/collectedHeap.inline.hpp
>>>>
>>>> Based on experiences with RMO architectures (ref JDK-8033920 - sorry it is a non-open bug report) I would expect the use of release_set_klass to be unconditional, not dependent on the nature of the GC. Publication of the object reference also has to be safe for other mutator threads. I'm not convinced by the brief argument in JDK-8033920 that thread-state transitions will always ensure visibility and ordering.
>>>
>>> There were two different issues discussed in (non-open) JDK-8033920.
>>>
>>> (1) Ensure setting the klass happens after some other parts of the
>>> object initialization, to ensure (some) concurrent GCs can parse the
>>> heap.
>>>
>>> (2) Ensure the klass is visible to other (mutator) threads if the
>>> object is accessible to those threads, e.g. if the object escapes from
>>> the allocating thread.
>>>
>>> JDK-8160369 is only about (1), and this change (for JDK-8165808) is
>>> part of fixing (1). I'm not trying to address (2) here, even assuming
>>> there are problems in that area. (I don't know that there *are*
>>
>> I have my doubts about treating these two issues separately, but will not press the issue. For the general mutator path the devil is in the detail, and as Derek indicated if there is a problem there the solution would likely need the matching load-acquire.
>
> I'm pretty sure they are different, leading to barriers in different
> places for different reasons.
>
> I'm investigating a hypothesis about the general mutator path, but not
> yet ready to say anything more because I may be talking out of my hat.
>
> So can I get a second review of this change?
>
More information about the hotspot-dev
mailing list