RFR: 8293972: runtime/NMT/NMTInitializationTest.java#default_long-off failed with "Suspiciously long bucket chains in lookup table." [v4]

[Gerard Ziemski]

On Thu, 20 Jul 2023 07:08:32 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Gerard Ziemski has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains seven additional commits since the last revision:
>> 
>>  - init NMT earlier to decrease the reliance on NMTPreInit lookup table, which lets us simplify the code
>>  - Merge remote-tracking branch 'upstream/master' into JDK-8293972
>>  - do not clip bits to 32, we need full 64 bits
>>  - ^ is XOR, not pow
>>  - remove tabs
>>  - mark output with PID to help identify where it comes from
>>  - use Mersenne prime and mod for uniform bit distribution of hash
>
> src/hotspot/share/runtime/arguments.cpp line 337:
> 
>> 335: }
>> 336: 
>> 337: ccstr Arguments::process_nmt_property(JavaVMInitArgs* args) {
> 
> Please don't use "property" here as "property" has a very specific meaning when it comes to command-line arguments. I think "setting" would suffice here.

Fixed.

> src/hotspot/share/services/memTracker.cpp line 60:
> 
>> 58: 
>> 59:   NMT_TrackingLevel level = NMTUtil::parse_tracking_level(value);
>> 60:   // Should have been validated before in arguments.cpp
> 
> Why don't we save the value parsed in arguments.cpp so we don't need to repeat the parsing?

Fixed.

I moved the relevant NMT related argument parsing into one place, hence we only need to parse the setting once.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/14607#discussion_r1270020182
PR Review Comment: https://git.openjdk.org/jdk/pull/14607#discussion_r1270023970