RFR: Merge jdk-21-ga
Jiangli Zhou
jiangli at openjdk.org
Wed Apr 17 23:14:18 UTC 2024
On Wed, 17 Apr 2024 22:50:12 GMT, Jiangli Zhou <jiangli at openjdk.org> wrote:
> > In javaClasses.hpp, the empty block of #if INCLUDE_TSAN with a FIXME line above can be removed.
>
> Done.
>
> > In fieldInfo.hpp, the removal of empty line above the following context is not needed:
>
> Added empty line back.
>
> > In instanceKlass.cpp, InstanceKlass::initialize, the new sentence in comment is a bit hard to understand:
> > // This is to read/write of natives related to class static initializer.
> > How about "Native memory accesses in JNI code invoked from class static initializer could rely on this happens-> > before edge to avoid data races."?
> > Alternatively, we can leave the old comment as-is, and create a separate PR later to add the new sentence, to make this change more conspicuous.
>
> I reworded your suggested comment as the following:
>
> ```
> // Memory accesses from JNI native code
> // invoked from class static initializer may rely on this happens-before
> // edge to avoid reporting false positive data races.
> ```
>
> > In InstanceKlass::set_initialization_state_and_notify(), it might be better to wrap the if (state == fully_initialized) check inside TSAN_RUNTIME_ONLY macro. Also, the indention seems off inside the if block.
>
> Done.
>
> > Change to jvmtiTagMap.hpp in unnecessary (added an empty line).
>
> Removed empty line at line# 60.
>
> > In unsafe.cpp, in Unsafe_CompareAndExchangeInt, it should be ScopedReleaseAcquire releaseAcquire(addr) instead of ScopedReleaseAcquire releaseAcquire(p, offset).
>
> Fixed.
>
> > In tsanOopMap.cpp, I can see it is difficult to get GCTraceCPUTime working, so it is fine to remove it. But for GCTraceTime, do they trigger a build error if they are left as-is? It is better to remove GCTraceTime in the later change that fixes the issues with tsanOopMap.
>
> Right. As mentioned in offline conversion with you yesterday, the GCTraceCPUTime usages in TsanOopMap was causing build failures. That's why I removed them as part of the current merge change. It's not worth making GCTraceCPUTime code working in the current version than later remove it when making TsanOopMap concurrent.
@caoman and I just had additional offline discussion for clarifications. Both GCTraceTime and GCTraceCPUTime usages were removed from the current version of TsanOopMap of part of my merge. TsanOopMap will need to be reworked.
-------------
PR Comment: https://git.openjdk.org/tsan/pull/18#issuecomment-2062631076
More information about the tsan-dev
mailing list