RFR: 8299125: UnifiedOopRef in JFR leakprofiler should treat thread local oops correctly [v2]

[Markus Grönlund]

On Mon, 2 Jan 2023 08:38:12 GMT, Axel Boldt-Christmas <aboldtch at openjdk.org> wrote:

>> In a similar vain to [JDK-8299089](https://bugs.openjdk.org/browse/JDK-8299089) there are different semantics for OopStorage and thread local oops. UnifiedOopRef in the jfr leakprofiler must be able to distinguish these and treat them appropriately.
>> 
>> Testing: Running test at the moment. Has been running on the generational branch of ZGC for over three months. Not many tests exercise the leakprofiler. x86_32 has mostly been tested with GHA. 
>> 
>> Note: The accessBackend changes are only there to facilitate comparing OopLoadProxy objects directly with nullptr instead of creating and comparing with an `oop()` / `oop(NULL)`. Can be backed out of this PR and put in a different RFE instead.
>
> Axel Boldt-Christmas has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Use more suitible nomenclature: raw

Changes requested by mgronlun (Reviewer).

src/hotspot/share/jfr/leakprofiler/chains/rootSetClosure.cpp line 51:

> 49:   assert(ref != NULL, "invariant");
> 50:   assert(is_aligned(ref, HeapWordSize), "invariant");
> 51:   if (NativeAccess<>::oop_load(ref) != nullptr) {

Why is this needed now?

src/hotspot/share/jfr/leakprofiler/utilities/unifiedOopRef.hpp line 34:

> 32:   static const uintptr_t tag_mask   = LP64_ONLY(0b111) NOT_LP64(0b011);
> 33:   static const uintptr_t native_tag = 0b001;
> 34:   static const uintptr_t raw_tag    = 0b010;

What is "raw", compared to "native"?

src/hotspot/share/jfr/leakprofiler/utilities/unifiedOopRef.inline.hpp line 45:

> 43: template<>
> 44: inline uintptr_t UnifiedOopRef::addr<uintptr_t>() const {
> 45:   return (_value & ~tag_mask) LP64_ONLY(>> 1);

Why a shift now as well?

src/hotspot/share/jfr/leakprofiler/utilities/unifiedOopRef.inline.hpp line 70:

> 68:   LP64_ONLY(assert(((reinterpret_cast<uintptr_t>(ref)) & (1ull << 63)) == 0, "Unexpected high-order bit"));
> 69:   UnifiedOopRef result = { (reinterpret_cast<uintptr_t>(ref) LP64_ONLY(<< 1)) | tag };
> 70:   assert(result.addr<T>() == ref, "sanity");

This is crazy hard to read.

src/hotspot/share/oops/accessBackend.hpp line 1260:

> 1258:     }
> 1259: 
> 1260:     inline bool operator ==(std::nullptr_t) const {

why std::nullptr_t?

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

PR: https://git.openjdk.org/jdk/pull/11741