RFR: Tsan oop map table hash fix [v2]

[Man Cao]

On Mon, 19 Aug 2024 21:43:35 GMT, Jiangli Zhou <jiangli at openjdk.org> wrote:

>> src/hotspot/share/gc/shared/weakProcessor.inline.hpp line 100:
>> 
>>> 98: 
>>> 99:   TSAN_RUNTIME_ONLY(
>>> 100:     MutexLocker mu(TsanOopMap_lock, Mutex::_no_safepoint_check_flag);
>> 
>> I think we can completely get rid of this lock during GC, by switching to Atomic on `_nworkers_completed`:
>> 
>> 
>> TSAN_RUNTIME_ONLY(
>> uint completed = Atomic::add(&_nworkers_completed, 1);
>> assert(_nworkers >= completed, "must be");
>> if (_nworkers == completed) {
>>   TsanOopMap::notify_tsan_for_freed_and_moved_objects();
>> }
>> )
>> 
>> 
>> We need to declared `_nworkers_completed` as `volatile uint` for using Atomic.
>
> I also considered with `Atomic::add` when making the change and decided to go with `TsanOopMap_lock`. The performance was not a concern for tsan. Using `Atomic::add` for tracking the completed worker count without a lock seemed also be a little simpler however. Since we already have the `TsanOopMap_lock` and there could be some potential cases that we uncover where we want to put the operations under the lock protection, I think it's better to keep using `TsanOopMap_lock` for now. We could optimize this later after more testing and fully stabilizing the system for tsan/JDK 21.

Do this later SGTM.

-------------

PR Review Comment: https://git.openjdk.org/tsan/pull/23#discussion_r1722410990