RFR: 8316813: NMT: Using WhiteBox API, virtual memory tracking should also be stressed in JMH tests [v2]

[Afshin Zafari]

On Mon, 12 Feb 2024 17:52:33 GMT, Gerard Ziemski <gziemski at openjdk.org> wrote:

>> Afshin Zafari has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   better names for variables used.
>
> test/micro/org/openjdk/bench/vm/runtime/NMTBenchmark_wb.java line 51:
> 
>> 49: @State(Scope.Benchmark)
>> 50: @Warmup(iterations = 1, time = 1, timeUnit = TimeUnit.SECONDS)
>> 51: @Measurement(iterations = 5, time = 1, timeUnit = TimeUnit.SECONDS)
> 
> How did you select those particular iteration values?

Smaller numbers give very large errors in the measurements. 
If these numbers still give large errors, the benchmark user can override them at command line to get measurements with smaller variance/error.

> test/micro/org/openjdk/bench/vm/runtime/NMTBenchmark_wb.java line 63:
> 
>> 61:     public int SUB_REGIONS;
>> 62: 
>> 63:     private static final int PageSize = 1024 * 4;
> 
> I think `PAGE_SIZE` should be the capitalization used for `final` constants in Java?

True. 
Done.

> test/micro/org/openjdk/bench/vm/runtime/NMTBenchmark_wb.java line 84:
> 
>> 82:             WB = wb;
>> 83:         }
>> 84:     }
> 
> Is that a pattern of code you think we will be needed in future tests? This looks like something that could perhaps get factored out and pushed as a utility code up into JMH itself?

I got help from @cl4es for this part, when I have had problems using WB in JMH. 
It cannot be part of JMH, since not all benchmarks use WB. It should only be used for benchmarks that needs and use WB.
Ping @cl4es.

> test/micro/org/openjdk/bench/vm/runtime/NMTBenchmark_wb.java line 91:
> 
>> 89:     private static void release (long base, long size) {        WhiteBoxHolder.WB.NMTReleaseMemory (base                 , size    ); }
>> 90: 
>> 91:     public static void doAllMemoryOps(int nR, int region_count) {
> 
> Does `nR` stand for `numberOfReservations`? Can we perhaps use `reservations_count` instead, or something better?

Done.

> test/micro/org/openjdk/bench/vm/runtime/NMTBenchmark_wb.java line 97:
> 
>> 95:           base_array[i] = reserve(region_size);
>> 96: 
>> 97:         for (int R = 0; R < nR; R++) {
> 
> The capitol `R` doesn't look quite right to me here, maybe use `reservation` or even a simple plain `r` would look better, imho.

Done.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17582#discussion_r1487541965
PR Review Comment: https://git.openjdk.org/jdk/pull/17582#discussion_r1487545094
PR Review Comment: https://git.openjdk.org/jdk/pull/17582#discussion_r1487544585
PR Review Comment: https://git.openjdk.org/jdk/pull/17582#discussion_r1487545525
PR Review Comment: https://git.openjdk.org/jdk/pull/17582#discussion_r1487545698