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

[Gerard Ziemski]

On Thu, 1 Feb 2024 18:26:16 GMT, Afshin Zafari <azafari at openjdk.org> wrote:

>> After WhiteBox (`wb.jar`) is added to the lib-test, it is possible to use it in Java benchmarks. This PR uses WhiteBox to access and call directly NMT API and measure their performance in stressed circumstances.
>
> Afshin Zafari has updated the pull request incrementally with one additional commit since the last revision:
> 
>   better names for variables used.

Good test, thank you for adding it.

I have a few small feedback items, but most of those have to do with syntax, not the logic, which looks fine to me.

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?

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?

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?

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?

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.

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

Changes requested by gziemski (Committer).

PR Review: https://git.openjdk.org/jdk/pull/17582#pullrequestreview-1875844859
PR Review Comment: https://git.openjdk.org/jdk/pull/17582#discussion_r1486559641
PR Review Comment: https://git.openjdk.org/jdk/pull/17582#discussion_r1486565962
PR Review Comment: https://git.openjdk.org/jdk/pull/17582#discussion_r1486561959
PR Review Comment: https://git.openjdk.org/jdk/pull/17582#discussion_r1486571107
PR Review Comment: https://git.openjdk.org/jdk/pull/17582#discussion_r1486573351