RFR(S): 8153267: nmethod's exception cache not multi-thread safe

[Vladimir Kozlov]

Thank you, Jamsheed. Testing results looks fine so far. I am pushing it.

Thanks,
Vladimir

On 4/15/16 12:44 AM, Jamsheed C m wrote:
> Hi Vladimir,
>
> PIT testing is in progress, link is available  in bug report.
>
> Best Regards,
> Jamsheed
>
> On 4/15/2016 7:14 AM, Vladimir Kozlov wrote:
>> Looks fine to me. Jamsheed, please, run our PIT testing with these changes and analyze results.
>>
>> Thanks,
>> Vladimir
>>
>> On 4/12/16 2:45 AM, Doerr, Martin wrote:
>>> Hi,
>>>
>>> I think we have come to a common understanding and there was no complaint about my latest webrev:
>>> http://cr.openjdk.java.net/~mdoerr/8153267_exception_cache/webrev.02/
>>>
>>> Can I consider it reviewed?
>>> Can somebody sponsor, please?
>>>
>>> Thanks and best regards,
>>> Martin
>>>
>>>
>>> -----Original Message-----
>>> From: hotspot-compiler-dev [mailto:hotspot-compiler-dev-bounces at openjdk.java.net] On Behalf Of Doerr, Martin
>>> Sent: Donnerstag, 7. April 2016 12:52
>>> To: Andrew Haley <aph at redhat.com>; Jamsheed C m <jamsheed.c.m at oracle.com>; hotspot-compiler-dev at openjdk.java.net
>>> Subject: RE: RFR(S): 8153267: nmethod's exception cache not multi-thread safe
>>>
>>> Hi Andrew, Jamsheed and all,
>>>
>>> thank you very much for your input.
>>>
>>> As Andrew, Jamsheed and I think, it's better to have a releasing store in increment_count().
>>> Therefore, I have replaced the storestore barrier introduced with JDK-8143897  (even though this barrier was also
>>> correct).
>>>
>>> My change still contains a releasing store for newly created ExceptionCache instances.
>>> As Jamsheed has pointed out, this should not be strictly required as we have the other barrier. It may only produce
>>> additional false negatives on weak memory model platforms.
>>> I think having the release doesn't hurt too much and makes the design a little cleaner.
>>>
>>> I also added comments based on your input.
>>>
>>> The new webrev is here:
>>> http://cr.openjdk.java.net/~mdoerr/8153267_exception_cache/webrev.02/
>>>
>>> Please review. I will also need a sponsor from Oracle, please.
>>>
>>> Thanks again and best regards,
>>> Martin
>>>
>>>
>>> -----Original Message-----
>>> From: Andrew Haley [mailto:aph at redhat.com]
>>> Sent: Donnerstag, 7. April 2016 12:14
>>> To: Doerr, Martin <martin.doerr at sap.com>; Jamsheed C m <jamsheed.c.m at oracle.com>; hotspot-compiler-dev at openjdk.java.net
>>> Subject: Re: RFR(S): 8153267: nmethod's exception cache not multi-thread safe
>>>
>>> On 07/04/16 10:08, Doerr, Martin wrote:
>>>
>>>> atomic update for the _count would only be required if there were
>>>> multiply threads which attempt to increment it
>>>> concurrently. However, updates are under lock, so we only have
>>>> concurrent readers which is ok.
>>>>
>>>> I still think "volatile" does what we need here. Especially the xlC
>>>> compiler on AIX tends to reload variables from memory. Exactly this
>>>> can be prevented by making the field volatile.
>>>
>>> I think your latest patch is OK.  Whether volatile is really good
>>> enough, I don't know.  The new(ish) C++ memory model treats this as a
>>> race, and therefore undefined behaviour.  Old C++ didn't have a memory
>>> model, so the best we can do with racy code is guess about what our
>>> compilers might do.
>>>
>>> I certainly much prefer a release_store to the storestore fence used
>>> in the fix for 8143897.
>>>
>>> Andrew.
>>>
>