[RFR]: Lock instrumentation

[Jean Christophe Beyler]

Yeah for the test, I'm actually adding that macro now in the JVMTI patch so
we can see what it looks and decide :), then I can refactor the other
changes to the same style in a later webrev,
Jc

On Wed, Apr 24, 2019 at 10:37 AM Arthur Eubanks <aeubanks at google.com> wrote:

> With JC's fix to the tests and
> the ReleaseJavaMonitorsClosure::do_monitor() fix, now all the tests pass!
>
> I'm mixed on the TSAN_RUNTIME_ONLY() suggestion, but either way it can
> come in a later change.
>
> On Tue, Apr 23, 2019 at 1:23 PM Man Cao <manc at google.com> wrote:
>
>> In synchronizer.cpp, in ReleaseJavaMonitorsClosure::do_monitor(), the
>> TSAN instrumentation should be inside the code block of the conditional "if
>> (mid->owner() == THREAD) {".
>> Otherwise looks good. No need for a new webrev after fixing this.
>>
>> Regarding the "#if INCLUDE_TSAN" macro, I think we could define a macro
>> in macros.hpp like the following:
>> #if INCLUDE_TSAN
>> #define TSAN_RUNTIME_ONLY(code) if (ThreadSanitizer) { code }
>> #else
>> #define TSAN_RUNTIME_ONLY(code)
>> #endif
>>
>> Then it could simplify some (but not all) of the instrumentation.
>>
>> -Man
>>
>>
>> On Tue, Apr 23, 2019 at 9:49 AM Jean Christophe Beyler <
>> jcbeyler at google.com> wrote:
>>
>>> Hi Arthur,
>>>
>>> Looks good to me, I was looking at if we could factorize a bit the
>>> assertions and what-not but for now it's fine.
>>>
>>> (I did notice that we let creep in the #endif // INCLUDE_TSAN which does
>>> not seem to be that much the style of the general code base so we might
>>> want to remove them at some point)
>>>
>>> But that's a detail, LGTM,
>>> Jc
>>>
>>>
>>> On Tue, Apr 23, 2019 at 9:29 AM Arthur Eubanks <aeubanks at google.com>
>>> wrote:
>>>
>>> > This changes adds instrumentation for various locks in the JVM. (not
>>> > including JVMTI)
>>> > http://cr.openjdk.java.net/~aeubanks/tsanlock/webrev.00
>>> >
>>> > Note the NonRacy tests still don't pass for some reason. I believe
>>> it's due
>>> > to dynamic linking of libtsan.so.
>>> >
>>>
>>>
>>> --
>>>
>>> Thanks,
>>> Jc
>>>
>>

-- 

Thanks,
Jc