[RFR] Instrumentation for Unsafe CAS
Arthur Eubanks
aeubanks at google.com
Mon Sep 23 22:10:34 UTC 2019
On Mon, Sep 23, 2019 at 2:58 PM Man Cao <manc at google.com> wrote:
> Looks good.
> Last nit, no need for webrev:
>
> ScopedReleaseAcquire(oop obj, ptrdiff_t offset) {
>
> This could use "jlong offset" now.
>
> Done.
> -Man
>
>
>
> On Mon, Sep 23, 2019 at 2:50 PM Arthur Eubanks <aeubanks at google.com>
> wrote:
>
>> New webrev: http://cr.openjdk.java.net/~aeubanks/tsancas/webrev.02
>>
>> On Mon, Sep 23, 2019 at 2:30 PM Man Cao <manc at google.com> wrote:
>>
>>> My main comment is:
>>>
>>> + ScopedReleaseAcquire(oop obj, ptrdiff_t offset): addr_(AccessInternal::field_addr(obj, offset)) {
>>>
>>> This could use "index_oop_from_field_offset_long(obj, offset)", right?
>>>
>>> Went back to the original version but used
>> index_oop_from_field_offset_long() instead of AccessInternal::field_addr().
>>
>>> Also discussed with Arthur that when TSAN is disabled at build-time, the code would still create the ScopedReleaseAcquire object, but hopefully the C++ compiler will optimize out no-op objects.
>>>
>>> Minor comments:
>>>
>>> +class ScopedReleaseAcquire {+private:+ void* addr_;
>>>
>>> This class should inherit from "public StackObj".
>>>
>>> Done
>>
>>> HotSpot style guide <https://wiki.openjdk.java.net/display/HotSpot/StyleGuide#StyleGuide-Names> says instance variable name should start with _, so "_addr".
>>>
>>> Done
>>
>>> -Man
>>>
>>>
>>>
>>> On Mon, Sep 23, 2019 at 12:52 PM Arthur Eubanks <aeubanks at google.com>
>>> wrote:
>>>
>>>> webrev:
>>>> http://cr.openjdk.java.net/~aeubanks/tsancas/webrev.00/index.html
>>>>
>>>> This is fairly self explanatory. I went down the route of using a C++
>>>> class's constructor/destructor for the release/acquire so I didn't have
>>>> to
>>>> touch the existing code too much.
>>>>
>>>
More information about the tsan-dev
mailing list