[RFR] Instrumentation for Unsafe CAS

Man Cao manc at google.com
Mon Sep 23 21:58:43 UTC 2019


Looks good.
Last nit, no need for webrev:

ScopedReleaseAcquire(oop obj, ptrdiff_t offset) {

This could use "jlong offset" now.

-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