RFR: Add JVMTI raw monitor lock/unlock mechanisms

[Jean Christophe Beyler]

Done, thanks both for the reviews :)
Jc

On Tue, Apr 30, 2019 at 6:40 PM Man Cao <manc at google.com> wrote:

> 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
>>
>

-- 

Thanks,
Jc