RFR: Add JVMTI raw monitor lock/unlock mechanisms

Man Cao manc at google.com
Wed May 1 01:39:58 UTC 2019


Looks good.

On Tue, Apr 30, 2019, 15:25 Jean Christophe Beyler <jcbeyler at google.com>
wrote:

> Fixed on my local branch:)
> Jc
>
> On Tue, Apr 30, 2019 at 3:23 PM Arthur Eubanks <aeubanks at google.com>
> wrote:
>
>> The comment in src/hotspot/share/prims/jvmtiEnv.cpp still says "below".
>>
>> Otherwise LGTM.
>>
>> On Tue, Apr 30, 2019 at 3:19 PM Jean Christophe Beyler <
>> jcbeyler at google.com> wrote:
>>
>>> Fair enough :)
>>>
>>> Done here:
>>> http://cr.openjdk.java.net/~jcbeyler/jvmti/webrev.07/
>>>
>>> (I left the jvmtiTagMap ones using TSAN_ONLY, as if TSAN is defined,
>>> they call the macros which then use TSAN_RUNTIME_ONLY so that works too)
>>>
>>> Tested that the test still passes :), let me know what you think,
>>> Jc
>>>
>>>
>>> On Tue, Apr 30, 2019 at 2:34 PM Man Cao <manc at google.com> wrote:
>>>
>>>> I'd prefer to revive the TSAN_RUNTIME_ONLY, and change the 4 macros in
>>>> tsan.hpp to use it. I like your TSAN_RUNTIME_ONLY and it could be used to
>>>> simplify other code.
>>>> The current webrev if left unchanged could cause issues with
>>>> -XX:-ThreadSanitizer.
>>>>
>>>> -Man
>>>>
>>>>
>>>> On Tue, Apr 30, 2019 at 1:28 PM Jean Christophe Beyler <
>>>> jcbeyler at google.com> wrote:
>>>>
>>>>> Good point, what do you prefer here? Just leave like this for now
>>>>> since it is a noop? I do :)
>>>>>
>>>>> But I'm biased at preferring forward progress on this webrev :)
>>>>> Jc
>>>>>
>>>>> On Tue, Apr 30, 2019 at 11:05 AM Man Cao <manc at google.com> wrote:
>>>>>
>>>>>> The issue now is the code is not guarded by the -XX:+ThreadSanitizer
>>>>>> flag any more.
>>>>>>
>>>>>> Perhaps we can add "if (ThreadSanitizer)" to the 4 macros in
>>>>>> tsan.hpp? E.g.:
>>>>>> #define TSAN_RAW_LOCK_CREATE(lock) \
>>>>>>    TSAN_ONLY(if (ThreadSanitizer) { TsanRawLockCreate(__FILE__,
>>>>>> __LINE__, lock); })
>>>>>>
>>>>>> Or maybe we can revive the TSAN_RUNTIME_ONLY macro, and use the
>>>>>> following?
>>>>>> #define TSAN_RAW_LOCK_CREATE(lock) \
>>>>>>   TSAN_RUNTIME_ONLY(TsanRawLockCreate(__FILE__, __LINE__, lock))
>>>>>>
>>>>>> In any case, it seems that there is no clean way to guard the
>>>>>> creation of TsanMutexScope objects with the +ThreadSanitizer flag, but it
>>>>>> is probably OK as it is no-op with -ThreadSanitizer.
>>>>>>
>>>>>> -Man
>>>>>>
>>>>>>
>>>>>> On Mon, Apr 29, 2019 at 6:58 PM Jean Christophe Beyler <
>>>>>> jcbeyler at google.com> wrote:
>>>>>>
>>>>>>> Fair enough, I forgot you had added it there while I was adding it
>>>>>>> in mine.
>>>>>>>
>>>>>>> I merged it to use yours, and because you don't do the fancy
>>>>>>> do/while in the macro, it works as intended then and we can leave it at
>>>>>>> that then for now :)
>>>>>>>
>>>>>>> New webrev is here:
>>>>>>> http://cr.openjdk.java.net/~jcbeyler/jvmti/webrev.06/
>>>>>>>
>>>>>>> I'll add in a future webrev the tests for the other VM operations
>>>>>>> and try to see how we can fix the VM operation also in a future webrev; as
>>>>>>> soon as symbolization is up, we should be able to get testing to be a bit
>>>>>>> more reliable :)
>>>>>>>
>>>>>>> Thanks for the reviews!
>>>>>>> Jc
>>>>>>>
>>>>>>> On Mon, Apr 29, 2019 at 3:17 PM Arthur Eubanks <aeubanks at google.com>
>>>>>>> wrote:
>>>>>>>
>>>>>>>> There is TSAN_ONLY in hotspot/share/utilities/macros.hpp, maybe
>>>>>>>> that's helpful?
>>>>>>>>
>>>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Jc
>>>>>>>
>>>>>>
>>>>>
>>>>> --
>>>>>
>>>>> Thanks,
>>>>> Jc
>>>>>
>>>>
>>>
>>> --
>>>
>>> Thanks,
>>> Jc
>>>
>>
>
> --
>
> Thanks,
> Jc
>


More information about the tsan-dev mailing list