RFR: 8313708: NMT: cleanup _mst_marker

[Thomas Stuefe]

On Thu, 3 Aug 2023 18:44:43 GMT, Gerard Ziemski <gziemski at openjdk.org> wrote:

> While learning the NMT code I came across _mst_marker and I don't really like how it combines 16bit index and 16bit position into single 32bit mst_marker using bit sizzling.
> 
> Consequently, right now we need the following 3 APIs: build_marker(), bucket_idx_from_marker(), pos_idx_from_marker() to support this. They are really not adding any value, in my opinion, and in fact obfuscate the code.
> 
> I'd like to propose that we simplify the code and pass a struct value (as suggested by Thomas) which hides away the 2 individual fields that are implementation detail.

I don't like much the increased complexity of `MallocSiteTable::lookup_or_add`. It now needs two args to let the caller track the MST bucket, and exposes implementation details: "marker" had been a neutral term, "bucketXX" implies an open hash table and therefore exposes details that may change in the future.

But it's a minor gripe, and the rest is good. Let's ship it.

src/hotspot/share/services/mallocHeader.hpp line 120:

> 118:     const uint16_t position;
> 119:     inline BucketInfo(uint16_t idx, uint16_t pos) : index(idx), position(pos) { }
> 120:   };

Curious, why the alignas? Internally, 16-bit alignment would be sufficient, but these live in MallocHeaders anyway which are placed by us manually.

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

Marked as reviewed by stuefe (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/15145#pullrequestreview-1591416510
PR Review Comment: https://git.openjdk.org/jdk/pull/15145#discussion_r1303863776