RFR: Add JVMTI raw monitor lock/unlock mechanisms

Jean Christophe Beyler jcbeyler at google.com
Tue Apr 30 22:19:19 UTC 2019


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


More information about the tsan-dev mailing list